From ca6dfb81d9a1b0ea6300fd734c0bb1f030c485d1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Jan 2016 21:18:41 -0800 Subject: [PATCH 01/18] Run valgrind with full leak check --- script/test | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/script/test b/script/test index 0655bbfb..675281bd 100755 --- a/script/test +++ b/script/test @@ -66,10 +66,11 @@ fi case ${mode} in valgrind) - valgrind \ + valgrind \ --suppressions=./script/util/valgrind.supp \ - --dsymutil=yes \ - $cmd "${args[@]}" 2>&1 | \ + --dsymutil=yes \ + --leak-check=full \ + $cmd "${args[@]}" 2>&1 | \ grep --color -E '\w+_specs?.cc:\d+|$' ;; From 5f27550a7a778729a1ab7b063bb8265c0ec4ac87 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Jan 2016 21:18:57 -0800 Subject: [PATCH 02/18] Fix leaked nodes in stack --- spec/runtime/stack_spec.cc | 125 ++++++++++++++++++++++++++----------- src/runtime/stack.c | 55 +++++++++------- src/runtime/vector.h | 6 +- 3 files changed, 129 insertions(+), 57 deletions(-) diff --git a/spec/runtime/stack_spec.cc b/spec/runtime/stack_spec.cc index 3ed40bc6..566f916e 100644 --- a/spec/runtime/stack_spec.cc +++ b/spec/runtime/stack_spec.cc @@ -1,8 +1,12 @@ +#define TREE_SITTER_WRAP_MALLOC #include "spec_helper.h" #include "helpers/tree_helpers.h" +#include "helpers/record_alloc.h" +#include "helpers/stream_methods.h" #include "runtime/stack.h" #include "runtime/tree.h" #include "runtime/length.h" +#include "runtime/alloc.h" enum { stateA, stateB, stateC, stateD, stateE, stateF, stateG, stateH, stateI, stateJ @@ -31,8 +35,28 @@ TSTree * tree_selection_spy_callback(void *data, TSTree *left, TSTree *right) { return spy->tree_to_return; } -START_TEST +void free_pop_results(Vector *pop_results) { + for (size_t i = 0; i < pop_results->size; i++) { + StackPopResult *pop_result = (StackPopResult *)vector_get(pop_results, i); + bool matches_prior_trees = false; + for (size_t j = 0; j < i; j++) { + StackPopResult *prior_result = (StackPopResult *)vector_get(pop_results, j); + if (pop_result->trees == prior_result->trees) { + matches_prior_trees = true; + break; + } + } + + if (!matches_prior_trees) { + for (size_t j = 0; j < pop_result->tree_count; j++) + ts_tree_release(pop_result->trees[j]); + ts_free(pop_result->trees); + } + } +} + +START_TEST describe("Stack", [&]() { Stack *stack; @@ -43,6 +67,8 @@ describe("Stack", [&]() { TSSymbolMetadata metadata = {true, true, true, true}; before_each([&]() { + record_alloc::start(); + stack = ts_stack_new(); ts_stack_set_tree_selection_callback(stack, @@ -58,6 +84,9 @@ describe("Stack", [&]() { ts_stack_delete(stack); for (size_t i = 0; i < tree_count; i++) ts_tree_release(trees[i]); + + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); describe("pushing entries to the stack", [&]() { @@ -108,47 +137,57 @@ describe("Stack", [&]() { /* * A0. */ - Vector pop = ts_stack_pop(stack, 0, 2, false); - StackPopResult pop1 = *(StackPopResult *)vector_get(&pop, 0); - AssertThat(pop.size, Equals(1)); - AssertThat(pop1.tree_count, Equals(2)); - AssertThat(pop1.trees[0], Equals(trees[1])); - AssertThat(pop1.trees[1], Equals(trees[2])); + Vector results = ts_stack_pop(stack, 0, 2, false); + AssertThat(results.size, Equals(1)); + + StackPopResult result = *(StackPopResult *)vector_get(&results, 0); + AssertThat(result.tree_count, Equals(2)); + AssertThat(result.trees[0], Equals(trees[1])); + AssertThat(result.trees[1], Equals(trees[2])); AssertThat(*ts_stack_head(stack, 0), Equals({trees[0], stateA, tree_len})); + free_pop_results(&results); /* * . */ - pop = ts_stack_pop(stack, 0, 1, false); - pop1 = *(StackPopResult *)vector_get(&pop, 0); - AssertThat(pop.size, Equals(1)); - AssertThat(pop1.tree_count, Equals(1)); - AssertThat(pop1.trees[0], Equals(trees[0])); + results = ts_stack_pop(stack, 0, 1, false); + AssertThat(results.size, Equals(1)); + + result = *(StackPopResult *)vector_get(&results, 0); + AssertThat(result.tree_count, Equals(1)); + AssertThat(result.trees[0], Equals(trees[0])); AssertThat(ts_stack_head(stack, 0), Equals(nullptr)); + + free_pop_results(&results); }); it("does not count 'extra' trees toward the count", [&]() { trees[1]->extra = true; - Vector pop = ts_stack_pop(stack, 0, 2, false); - StackPopResult pop1 = *(StackPopResult *)vector_get(&pop, 0); - AssertThat(pop.size, Equals(1)); - AssertThat(pop1.tree_count, Equals(3)); - AssertThat(pop1.trees[0], Equals(trees[0])); - AssertThat(pop1.trees[1], Equals(trees[1])); - AssertThat(pop1.trees[2], Equals(trees[2])); + Vector results = ts_stack_pop(stack, 0, 2, false); + AssertThat(results.size, Equals(1)); + + StackPopResult result = *(StackPopResult *)vector_get(&results, 0); + AssertThat(result.tree_count, Equals(3)); + AssertThat(result.trees[0], Equals(trees[0])); + AssertThat(result.trees[1], Equals(trees[1])); + AssertThat(result.trees[2], Equals(trees[2])); AssertThat(ts_stack_head(stack, 0), Equals(nullptr)); + + free_pop_results(&results); }); it("pops the entire stack when given a negative count", [&]() { - Vector pop = ts_stack_pop(stack, 0, -1, false); + Vector results = ts_stack_pop(stack, 0, -1, false); + AssertThat(results.size, Equals(1)); - AssertThat(pop.size, Equals(1)); - StackPopResult pop1 = *(StackPopResult *)vector_get(&pop, 0); - AssertThat(pop1.tree_count, Equals(3)); - AssertThat(pop1.trees[0], Equals(trees[0])); - AssertThat(pop1.trees[1], Equals(trees[1])); - AssertThat(pop1.trees[2], Equals(trees[2])); + StackPopResult result = *(StackPopResult *)vector_get(&results, 0); + AssertThat(result.tree_count, Equals(3)); + AssertThat(result.trees[0], Equals(trees[0])); + AssertThat(result.trees[1], Equals(trees[1])); + AssertThat(result.trees[2], Equals(trees[2])); + + free_pop_results(&results); }); }); @@ -170,11 +209,15 @@ describe("Stack", [&]() { * \. */ ts_stack_push(stack, 0, stateD, trees[3]); - ts_stack_pop(stack, 1, 1, false); + Vector pop_results = ts_stack_pop(stack, 1, 1, false); AssertThat(ts_stack_head_count(stack), Equals(2)); AssertThat(*ts_stack_head(stack, 0), Equals({trees[3], stateD, tree_len * 4})); AssertThat(*ts_stack_head(stack, 1), Equals({trees[1], stateB, tree_len * 2})); + AssertThat(pop_results.size, Equals(1)); + StackPopResult *pop_result = (StackPopResult *)vector_get(&pop_results, 0); + AssertThat(pop_result->tree_count, Equals(1)); + free_pop_results(&pop_results); /* * A0__B1__C2__D3. @@ -276,20 +319,18 @@ describe("Stack", [&]() { }); describe("when the first head is only one node deep", [&]() { - it("adds it as an additional successor node to The Null node", [&]() { + it("creates a node with one null successor and one non-null successor", [&]() { + TSTree *parent = ts_tree_make_node(5, 2, tree_array({ trees[2], trees[3] }), metadata); + tree_selection_spy.tree_to_return = parent; + tree_selection_spy.call_count = 0; + /* * .__C5. * B2.__/ */ ts_stack_clear(stack); ts_stack_split(stack, 0); - TSTree *parent = ts_tree_make_node(5, 2, tree_array({ trees[2], trees[3] }), metadata); - - ts_stack_push(stack, 0, stateC, parent); - - tree_selection_spy.tree_to_return = parent; - tree_selection_spy.call_count = 0; - + AssertThat(ts_stack_push(stack, 0, stateC, parent), Equals(StackPushResultContinued)); AssertThat(ts_stack_push(stack, 1, stateB, trees[2]), Equals(StackPushResultContinued)); AssertThat(ts_stack_push(stack, 1, stateC, trees[3]), Equals(StackPushResultMerged)); AssertThat(tree_selection_spy.call_count, Equals(1)); @@ -301,6 +342,8 @@ describe("Stack", [&]() { AssertThat(ts_stack_entry_next_count(head), Equals(2)); AssertThat(ts_stack_entry_next(head, 0), Equals(nullptr)); AssertThat(*ts_stack_entry_next(head, 1), Equals({trees[2], stateB, tree_len})); + + ts_tree_release(parent); }); }); }); @@ -349,6 +392,8 @@ describe("Stack", [&]() { AssertThat(ts_stack_head_count(stack), Equals(2)); AssertThat(*ts_stack_head(stack, 0), Equals({trees[2], stateC, tree_len * 3})); AssertThat(*ts_stack_head(stack, 1), Equals({trees[4], stateE, tree_len * 3})); + + free_pop_results(&pop); }); }); @@ -369,6 +414,8 @@ describe("Stack", [&]() { AssertThat(pop.size, Equals(1)); AssertThat(ts_stack_head_count(stack), Equals(1)); + + free_pop_results(&pop); }); }); @@ -400,6 +447,8 @@ describe("Stack", [&]() { AssertThat(pop2.tree_count, Equals(2)); AssertThat(pop2.trees[0], Equals(trees[6])); AssertThat(pop2.trees[1], Equals(trees[7])); + + free_pop_results(&pop); }); }); @@ -422,6 +471,8 @@ describe("Stack", [&]() { AssertThat(pop2.tree_count, Equals(3)); AssertThat(pop2.head_index, Equals(0)); AssertThat(pop2.trees[0], Equals(trees[4])); + + free_pop_results(&pop); }); }); }); @@ -485,6 +536,8 @@ describe("Stack", [&]() { AssertThat(pop3.head_index, Equals(2)); AssertThat(pop3.tree_count, Equals(2)); AssertThat(pop3.trees, Equals(pop1.trees)); + + free_pop_results(&pop); }); }); @@ -523,6 +576,8 @@ describe("Stack", [&]() { AssertThat(pop3.trees[0], Equals(trees[7])); AssertThat(pop3.trees[1], Equals(trees[8])); AssertThat(pop3.trees[2], Equals(trees[9])); + + free_pop_results(&pop); }); }); }); diff --git a/src/runtime/stack.c b/src/runtime/stack.c index a1b28244..8e4c63f7 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -5,6 +5,7 @@ #include "runtime/stack.h" #include "runtime/length.h" #include +#include #define MAX_SUCCESSOR_COUNT 8 #define INITIAL_HEAD_CAPACITY 3 @@ -78,13 +79,6 @@ error: return NULL; } -void ts_stack_delete(Stack *self) { - vector_delete(&self->pop_results); - vector_delete(&self->pop_paths); - ts_free(self->heads); - ts_free(self); -} - /* * Section: Reading from the stack */ @@ -169,6 +163,21 @@ static StackNode *stack_node_new(StackNode *next, TSStateId state, TSTree *tree) return self; } +static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, TSTree *tree) { + if (tree != node->entry.tree) { + TSTree *new_tree = self->tree_selection_function( + self->tree_selection_payload, + node->entry.tree, + tree); + + if (new_tree != node->entry.tree) { + ts_tree_retain(new_tree); + ts_tree_release(node->entry.tree); + node->entry.tree = new_tree; + } + } +} + static void ts_stack__add_node_successor(Stack *self, StackNode *node, StackNode *new_successor) { for (int i = 0; i < node->successor_count; i++) { @@ -179,12 +188,7 @@ static void ts_stack__add_node_successor(Stack *self, StackNode *node, return; if (successor->entry.state == new_successor->entry.state) { - if (successor->entry.tree != new_successor->entry.tree) { - successor->entry.tree = self->tree_selection_function( - self->tree_selection_payload, successor->entry.tree, - new_successor->entry.tree); - ts_tree_retain(successor->entry.tree); - } + ts_stack__add_alternative_tree(self, successor, new_successor->entry.tree); for (int j = 0; j < new_successor->successor_count; j++) ts_stack__add_node_successor(self, successor, new_successor->successors[j]); @@ -234,11 +238,7 @@ static bool ts_stack__merge_head(Stack *self, int head_index, TSStateId state, StackNode *head = self->heads[i]; if (head->entry.state == state && ts_length_eq(head->entry.position, position)) { - if (head->entry.tree != tree) { - head->entry.tree = self->tree_selection_function( - self->tree_selection_payload, head->entry.tree, tree); - ts_tree_retain(head->entry.tree); - } + ts_stack__add_alternative_tree(self, head, tree); ts_stack__add_node_successor(self, head, self->heads[head_index]); ts_stack_remove_head(self, head_index); return true; @@ -267,15 +267,15 @@ StackPushResult ts_stack_push(Stack *self, int head_index, TSStateId state, if (!new_head) return StackPushResultFailed; + stack_node_release(self->heads[head_index]); self->heads[head_index] = new_head; return StackPushResultContinued; } void ts_stack_add_alternative(Stack *self, int head_index, TSTree *tree) { assert(head_index < self->head_count); - StackEntry *entry = &self->heads[head_index]->entry; - entry->tree = self->tree_selection_function(self->tree_selection_payload, - entry->tree, tree); + StackNode *node = self->heads[head_index]; + ts_stack__add_alternative_tree(self, node, tree); } int ts_stack_split(Stack *self, int head_index) { @@ -332,6 +332,11 @@ Vector ts_stack_pop(Stack *self, int head_index, int child_count, */ if (path->is_shared) { path->trees = vector_copy(&path->trees); + for (size_t j = 0; j < path->trees.size; j++) { + TSTree **tree = vector_get(&path->trees, j); + ts_tree_retain(*tree); + } + path->is_shared = false; } @@ -410,3 +415,11 @@ void ts_stack_set_tree_selection_callback(Stack *self, void *payload, self->tree_selection_payload = payload; self->tree_selection_function = function; } + +void ts_stack_delete(Stack *self) { + vector_delete(&self->pop_results); + vector_delete(&self->pop_paths); + ts_stack_clear(self); + ts_free(self->heads); + ts_free(self); +} diff --git a/src/runtime/vector.h b/src/runtime/vector.h index 94faebe4..ef36bb16 100644 --- a/src/runtime/vector.h +++ b/src/runtime/vector.h @@ -8,6 +8,7 @@ extern "C" { #include #include #include +#include #include "runtime/alloc.h" typedef struct { @@ -37,7 +38,10 @@ static inline bool vector_valid(Vector *self) { } static inline void vector_delete(Vector *self) { - ts_free(self->contents); + if (self->contents) { + ts_free(self->contents); + self->contents = NULL; + } } static inline void *vector_get(Vector *self, size_t index) { From 0cf59913ae49d8988687cd6a62548ed80f32ef4a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Jan 2016 23:15:22 -0800 Subject: [PATCH 03/18] Fix double retain of child trees --- spec/runtime/stack_spec.cc | 3 +++ spec/runtime/tree_spec.cc | 18 +++++++++++++++--- src/runtime/tree.c | 1 - 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/spec/runtime/stack_spec.cc b/spec/runtime/stack_spec.cc index 566f916e..f9694445 100644 --- a/spec/runtime/stack_spec.cc +++ b/spec/runtime/stack_spec.cc @@ -320,7 +320,10 @@ describe("Stack", [&]() { describe("when the first head is only one node deep", [&]() { it("creates a node with one null successor and one non-null successor", [&]() { + ts_tree_retain(trees[2]); + ts_tree_retain(trees[3]); TSTree *parent = ts_tree_make_node(5, 2, tree_array({ trees[2], trees[3] }), metadata); + tree_selection_spy.tree_to_return = parent; tree_selection_spy.call_count = 0; diff --git a/spec/runtime/tree_spec.cc b/spec/runtime/tree_spec.cc index e75d767e..dcec0a0f 100644 --- a/spec/runtime/tree_spec.cc +++ b/spec/runtime/tree_spec.cc @@ -33,6 +33,9 @@ describe("Tree", []() { before_each([&]() { tree1 = ts_tree_make_leaf(cat, {2, 1, 0, 1}, {5, 4, 0, 4}, visible); tree2 = ts_tree_make_leaf(cat, {1, 1, 0, 1}, {3, 3, 0, 3}, visible); + + ts_tree_retain(tree1); + ts_tree_retain(tree2); parent1 = ts_tree_make_node(dog, 2, tree_array({ tree1, tree2, @@ -83,6 +86,9 @@ describe("Tree", []() { before_each([&]() { tree1->fragile_left = true; tree1->extra = true; + + ts_tree_retain(tree1); + ts_tree_retain(tree2); parent = ts_tree_make_node(eel, 2, tree_array({ tree1, tree2, @@ -104,6 +110,9 @@ describe("Tree", []() { before_each([&]() { tree2->fragile_right = true; tree2->extra = true; + + ts_tree_retain(tree1); + ts_tree_retain(tree2); parent = ts_tree_make_node(eel, 2, tree_array({ tree1, tree2, @@ -125,6 +134,9 @@ describe("Tree", []() { before_each([&]() { tree1->fragile_right = true; tree2->fragile_left = true; + + ts_tree_retain(tree1); + ts_tree_retain(tree2); parent = ts_tree_make_node(eel, 2, tree_array({ tree1, tree2, @@ -281,8 +293,6 @@ describe("Tree", []() { AssertThat(ts_tree_eq(parent1, parent2), IsTrue()); - ts_tree_release(tree1_copy); - ts_tree_release(tree2_copy); ts_tree_release(parent2); }); @@ -320,8 +330,10 @@ describe("Tree", []() { tree1->size, visible); + ts_tree_retain(different_tree); + ts_tree_retain(tree2); TSTree *different_parent = ts_tree_make_node(dog, 2, tree_array({ - different_tree, different_tree, + different_tree, tree2, }), visible); AssertThat(ts_tree_eq(different_parent, parent1), IsFalse()); diff --git a/src/runtime/tree.c b/src/runtime/tree.c index 52c022ed..5bcb8127 100644 --- a/src/runtime/tree.c +++ b/src/runtime/tree.c @@ -82,7 +82,6 @@ void ts_tree_set_children(TSTree *self, size_t child_count, TSTree **children) { self->visible_child_count = 0; for (size_t i = 0; i < child_count; i++) { TSTree *child = children[i]; - ts_tree_retain(child); if (i == 0) { self->padding = child->padding; From 95828f42a813c52aaa1534a3fcdbbfb0f9d850f5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 16:40:38 -0800 Subject: [PATCH 04/18] Fix leak of StringInput wrapper struct --- src/runtime/document.c | 6 +++++- src/runtime/document.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/runtime/document.c b/src/runtime/document.c index b5416e56..34c7b19c 100644 --- a/src/runtime/document.c +++ b/src/runtime/document.c @@ -23,6 +23,7 @@ void ts_document_free(TSDocument *self) { ts_parser_destroy(&self->parser); if (self->tree) ts_tree_release(self->tree); + ts_document_set_input(self, (TSInput){}); ts_free(self); } @@ -49,12 +50,16 @@ TSInput ts_document_input(TSDocument *self) { } void ts_document_set_input(TSDocument *self, TSInput input) { + if (self->owns_input) + ts_free(self->input.payload); self->input = input; + self->owns_input = false; } void ts_document_set_input_string(TSDocument *self, const char *text) { ts_document_invalidate(self); ts_document_set_input(self, ts_string_input_make(text)); + self->owns_input = true; } void ts_document_edit(TSDocument *self, TSInputEdit edit) { @@ -82,7 +87,6 @@ int ts_document_parse(TSDocument *self) { if (!tree) return -1; - ts_tree_retain(tree); if (self->tree) ts_tree_release(self->tree); self->tree = tree; diff --git a/src/runtime/document.h b/src/runtime/document.h index 79b307b2..e23aad1f 100644 --- a/src/runtime/document.h +++ b/src/runtime/document.h @@ -12,6 +12,7 @@ struct TSDocument { TSTree *tree; size_t parse_count; bool valid; + bool owns_input; }; #endif From b1f4b046f514f6a56b0e8dcee1473d576b22348e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 17:03:35 -0800 Subject: [PATCH 05/18] Test script - allow valgrind to be run w/o the leak check --- script/test | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/script/test b/script/test index 675281bd..d7897567 100755 --- a/script/test +++ b/script/test @@ -24,13 +24,14 @@ EOF } profile= +leak_check=no mode=normal args=() target=tests export BUILDTYPE=Test cmd="out/${BUILDTYPE}/${target}" -while getopts "df:s:ghpv" option; do +while getopts "df:s:gGhpv" option; do case ${option} in h) usage @@ -42,6 +43,10 @@ while getopts "df:s:ghpv" option; do g) mode=valgrind ;; + G) + mode=valgrind + leak_check=full + ;; p) profile=true ;; @@ -69,7 +74,7 @@ case ${mode} in valgrind \ --suppressions=./script/util/valgrind.supp \ --dsymutil=yes \ - --leak-check=full \ + --leak-check=${leak_check} \ $cmd "${args[@]}" 2>&1 | \ grep --color -E '\w+_specs?.cc:\d+|$' ;; From a74bf7ece13938882836842162b4bd6a6703e1e5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 17:25:07 -0800 Subject: [PATCH 06/18] Release tree when changing document's language --- src/runtime/document.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/document.c b/src/runtime/document.c index 34c7b19c..9f85f0f3 100644 --- a/src/runtime/document.c +++ b/src/runtime/document.c @@ -34,7 +34,10 @@ const TSLanguage *ts_document_language(TSDocument *self) { void ts_document_set_language(TSDocument *self, const TSLanguage *language) { ts_document_invalidate(self); self->parser.language = language; - self->tree = NULL; + if (self->tree) { + ts_tree_release(self->tree); + self->tree = NULL; + } } TSDebugger ts_document_debugger(const TSDocument *self) { From 7c44b0e387591546f73eb5d4e479afd087d42944 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 17:31:43 -0800 Subject: [PATCH 07/18] Fix leaked lookahead trees in normal parsing --- spec/runtime/document_spec.cc | 47 ++++++++++++++++++++++++---------- spec/runtime/node_spec.cc | 12 ++++++++- spec/runtime/stack_spec.cc | 1 - spec/spec_helper.h | 2 ++ src/runtime/parser.c | 48 +++++++++++++++++++++++------------ src/runtime/stack.c | 7 +++-- src/runtime/tree.c | 4 +++ 7 files changed, 85 insertions(+), 36 deletions(-) diff --git a/spec/runtime/document_spec.cc b/spec/runtime/document_spec.cc index 194beeac..85552fcf 100644 --- a/spec/runtime/document_spec.cc +++ b/spec/runtime/document_spec.cc @@ -1,5 +1,8 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "runtime/debugger.h" +#include "helpers/record_alloc.h" +#include "helpers/stream_methods.h" #include "helpers/tree_helpers.h" #include "helpers/spy_debugger.h" #include "helpers/spy_input.h" @@ -12,13 +15,22 @@ describe("Document", [&]() { TSNode root; before_each([&]() { + record_alloc::start(); doc = ts_document_make(); }); after_each([&]() { ts_document_free(doc); + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); + auto assert_node_string_equals = [&](TSNode node, const string &expected) { + char *actual = ts_node_string(node, doc); + AssertThat(actual, Equals(expected)); + ts_free(actual); + }; + describe("set_input(input)", [&]() { SpyInput *spy_input; @@ -30,8 +42,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(object (pair (string) (array (number) (number))))")); + assert_node_string_equals( + root, + "(object (pair (string) (array (number) (number))))"); }); after_each([&]() { @@ -48,8 +61,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(array (true) (false))")); + assert_node_string_equals( + root, + "(array (true) (false))"); }); it("allows the input to be retrieved later", [&]() { @@ -74,8 +88,9 @@ describe("Document", [&]() { ts_document_parse(doc); TSNode new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(object (pair (string) (array (null) (number))))")); + assert_node_string_equals( + new_root, + "(object (pair (string) (array (null) (number))))"); AssertThat(spy_input->strings_read, Equals(vector({" [null, 2", ""}))); }); @@ -83,14 +98,16 @@ describe("Document", [&]() { ts_document_set_input_string(doc, ""); ts_document_parse(doc); TSNode new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(ERROR (UNEXPECTED ))")); + assert_node_string_equals( + new_root, + "(ERROR (UNEXPECTED ))"); ts_document_set_input_string(doc, "1"); ts_document_parse(doc); new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(number)")); + assert_node_string_equals( + new_root, + "(number)"); }); }); @@ -104,8 +121,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(object (pair (string) (array (number) (number))))")); + assert_node_string_equals( + root, + "(object (pair (string) (array (number) (number))))"); }); it("clears out any previous tree", [&]() { @@ -117,9 +135,10 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( + assert_node_string_equals( + root, "(program (expression_statement " - "(object (pair (string) (array (number) (number))))))")); + "(object (pair (string) (array (number) (number))))))"); }); }); diff --git a/spec/runtime/node_spec.cc b/spec/runtime/node_spec.cc index 9b371f6e..cfd1bc1a 100644 --- a/spec/runtime/node_spec.cc +++ b/spec/runtime/node_spec.cc @@ -1,7 +1,10 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "helpers/tree_helpers.h" #include "helpers/point_helpers.h" #include "helpers/test_languages.h" +#include "helpers/record_alloc.h" +#include "helpers/stream_methods.h" START_TEST @@ -33,21 +36,28 @@ describe("Node", []() { size_t null_end_index = null_index + string("null").size(); before_each([&]() { + record_alloc::start(); + document = ts_document_make(); ts_document_set_language(document, get_test_language("json")); ts_document_set_input_string(document, input_string.c_str()); ts_document_parse(document); array_node = ts_document_root_node(document); - AssertThat(ts_node_string(array_node, document), Equals( + char *node_string = ts_node_string(array_node, document); + AssertThat(node_string, Equals( "(array " "(number) " "(false) " "(object (pair (string) (null))))")); + ts_free(node_string); }); after_each([&]() { ts_document_free(document); + + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); describe("named_child_count(), named_child(i)", [&]() { diff --git a/spec/runtime/stack_spec.cc b/spec/runtime/stack_spec.cc index f9694445..a183ebf4 100644 --- a/spec/runtime/stack_spec.cc +++ b/spec/runtime/stack_spec.cc @@ -1,4 +1,3 @@ -#define TREE_SITTER_WRAP_MALLOC #include "spec_helper.h" #include "helpers/tree_helpers.h" #include "helpers/record_alloc.h" diff --git a/spec/spec_helper.h b/spec/spec_helper.h index 03cc699e..7a3d5bce 100644 --- a/spec/spec_helper.h +++ b/spec/spec_helper.h @@ -14,4 +14,6 @@ using namespace tree_sitter; #define START_TEST go_bandit([]() { #define END_TEST }); +#define TREE_SITTER_WRAP_MALLOC + #endif // SPEC_HELPER_ diff --git a/src/runtime/parser.c b/src/runtime/parser.c index d12f5d17..b04288aa 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -221,6 +221,7 @@ static TSTree *ts_parser__get_next_lookahead(TSParser *self, int head) { LOG("reuse sym:%s size:%lu extra:%d", SYM_NAME(result->symbol), size.chars, result->extra); ts_parser__pop_reusable_subtree(state); + ts_tree_retain(result); return result; } @@ -305,7 +306,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, const TSSymbolMetadata *all_metadata = self->language->symbol_metadata; TSSymbolMetadata metadata = all_metadata[symbol]; Vector pop_results = ts_stack_pop(self->stack, head, child_count, count_extra); - if (!pop_results.element_size) + if (!vector_valid(&pop_results)) return FailedToUpdateStackHead; int last_head_index = -1; @@ -326,6 +327,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, if (pop_result->trees == prior_result->trees) { TSTree **existing_parent = vector_get(&self->reduce_parents, j); parent = *existing_parent; + ts_tree_retain(parent); trailing_extra_count = pop_result->tree_count - parent->child_count; break; } @@ -346,10 +348,10 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, parent = ts_tree_make_node(symbol, child_count, pop_result->trees, metadata); if (!parent) - return FailedToUpdateStackHead; + goto error; } if (!vector_push(&self->reduce_parents, &parent)) - return FailedToUpdateStackHead; + goto error; /* * If another path led to the same stack head, add this new parent tree @@ -370,6 +372,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, */ if (i > 0) { if (symbol == ts_builtin_sym_error) { + removed_heads++; ts_stack_remove_head(self->stack, new_head); continue; } @@ -378,7 +381,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, LookaheadState lookahead_state = *(LookaheadState *)vector_get(&self->lookahead_states, head); if (!vector_push(&self->lookahead_states, &lookahead_state)) - return FailedToUpdateStackHead; + goto error; } /* @@ -412,7 +415,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, */ switch (ts_stack_push(self->stack, new_head, state, parent)) { case StackPushResultFailed: - return FailedToUpdateStackHead; + goto error; case StackPushResultMerged: LOG("merge_during_reduce head:%d", new_head); vector_erase(&self->lookahead_states, new_head); @@ -449,17 +452,20 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, } } - if (fragile) { - for (size_t i = 0; i < self->reduce_parents.size; i++) { - TSTree **parent = vector_get(&self->reduce_parents, i); + for (size_t i = 0; i < self->reduce_parents.size; i++) { + TSTree **parent = vector_get(&self->reduce_parents, i); + if (fragile) (*parent)->fragile_left = (*parent)->fragile_right = true; - } + ts_tree_release(*parent); } if (removed_heads < revealed_heads) return UpdatedStackHead; else return RemovedStackHead; + +error: + return FailedToUpdateStackHead; } static ParseActionResult ts_parser__reduce_error(TSParser *self, int head, @@ -472,12 +478,12 @@ static ParseActionResult ts_parser__reduce_error(TSParser *self, int head, case RemovedStackHead: return RemovedStackHead; case UpdatedStackHead: { - TSTree **parent = vector_back(&self->reduce_parents); StackEntry *stack_entry = ts_stack_head(self->stack, head); + TSTree *parent = stack_entry->tree; stack_entry->position = ts_length_add(stack_entry->position, lookahead->padding); - (*parent)->size = ts_length_add((*parent)->size, lookahead->padding); - (*parent)->fragile_left = (*parent)->fragile_right = true; + parent->size = ts_length_add(parent->size, lookahead->padding); + parent->fragile_left = parent->fragile_right = true; lookahead->padding = ts_length_zero(); return UpdatedStackHead; } @@ -538,6 +544,7 @@ static ParseActionResult ts_parser__handle_error(TSParser *self, int head, if (lookahead->symbol == ts_builtin_sym_end) { LOG("fail_to_recover"); ts_parser__reduce_error(self, head, -1, lookahead); + ts_tree_release(lookahead); return RemovedStackHead; } } @@ -596,10 +603,10 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { size_t new_count = root->child_count + leading_extra_count + trailing_extra_count; ts_tree_set_children(root, new_count, new_children); - ts_tree_retain(root); ts_parser__remove_head(self, pop_result->head_index); self->finished_tree = ts_parser__select_tree(self, self->finished_tree, root); + ts_free(pop_result->trees); break; } } @@ -788,8 +795,11 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { ts_stack_head_count(self->stack), ts_stack_top_state(self->stack, head), position.chars); - if (position.chars != last_position.chars || - !ts_parser__can_reuse(self, head, lookahead)) { + if (position.chars == last_position.chars && + ts_parser__can_reuse(self, head, lookahead)) { + ts_tree_retain(lookahead); + } else { + ts_tree_release(lookahead); lookahead = ts_parser__get_next_lookahead(self, head); if (!lookahead) return NULL; @@ -800,7 +810,7 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { switch (ts_parser__consume_lookahead(self, head, lookahead)) { case FailedToUpdateStackHead: - return NULL; + goto error; case RemovedStackHead: removed = true; break; @@ -810,9 +820,15 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { } } + ts_tree_release(lookahead); + if (ts_stack_head_count(self->stack) == 0) { + ts_stack_clear(self->stack); ts_tree_assign_parents(self->finished_tree); return self->finished_tree; } } + +error: + return NULL; } diff --git a/src/runtime/stack.c b/src/runtime/stack.c index 8e4c63f7..685fc781 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -163,12 +163,11 @@ static StackNode *stack_node_new(StackNode *next, TSStateId state, TSTree *tree) return self; } -static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, TSTree *tree) { +static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, + TSTree *tree) { if (tree != node->entry.tree) { TSTree *new_tree = self->tree_selection_function( - self->tree_selection_payload, - node->entry.tree, - tree); + self->tree_selection_payload, node->entry.tree, tree); if (new_tree != node->entry.tree) { ts_tree_retain(new_tree); diff --git a/src/runtime/tree.c b/src/runtime/tree.c index 5bcb8127..6d15fb36 100644 --- a/src/runtime/tree.c +++ b/src/runtime/tree.c @@ -76,6 +76,8 @@ void ts_tree_assign_parents(TSTree *self) { } void ts_tree_set_children(TSTree *self, size_t child_count, TSTree **children) { + if (self->child_count > 0) + ts_free(self->children); self->children = children; self->child_count = child_count; self->named_child_count = 0; @@ -131,6 +133,8 @@ void ts_tree_retain(TSTree *self) { } void ts_tree_release(TSTree *self) { + if (!self) + return; assert(self->ref_count > 0); self->ref_count--; if (self->ref_count == 0) { From e3d65d5cbda48338e425b371b0bc4f3407785b5b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 17:33:15 -0800 Subject: [PATCH 08/18] Change name of generated parser files The previous name, parser.c made it harder to set breakpoints inside of src/runtime/parser.c in LLDB --- spec/helpers/load_language.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/load_language.cc b/spec/helpers/load_language.cc index 6d623b03..15d00f59 100644 --- a/spec/helpers/load_language.cc +++ b/spec/helpers/load_language.cc @@ -59,7 +59,7 @@ const TSLanguage *load_language(const string &name, const string &code) { return nullptr; } - string source_filename = string(temp_directory) + "/parser.c"; + string source_filename = string(temp_directory) + "/generated-parser.c"; string obj_filename = string(source_filename) + ".o"; string lib_filename = string(source_filename) + ".so"; string header_dir = string(getenv("PWD")) + "/include"; From 4a7312e51480184ee9341e20e6de95d47e04e524 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Feb 2016 12:03:11 -0800 Subject: [PATCH 09/18] Fix memory leaks when editing --- spec/runtime/parser_spec.cc | 133 ++++++++++++++++++++---------------- src/runtime/parser.c | 13 +++- 2 files changed, 84 insertions(+), 62 deletions(-) diff --git a/spec/runtime/parser_spec.cc b/spec/runtime/parser_spec.cc index 7b325dd9..7e8e45c2 100644 --- a/spec/runtime/parser_spec.cc +++ b/spec/runtime/parser_spec.cc @@ -1,4 +1,6 @@ #include "spec_helper.h" +#include "runtime/alloc.h" +#include "helpers/record_alloc.h" #include "helpers/spy_input.h" #include "helpers/test_languages.h" #include "helpers/log_debugger.h" @@ -13,6 +15,8 @@ describe("Parser", [&]() { size_t chunk_size; before_each([&]() { + record_alloc::start(); + chunk_size = 3; input = nullptr; @@ -20,11 +24,13 @@ describe("Parser", [&]() { }); after_each([&]() { - if (doc) - ts_document_free(doc); + ts_document_free(doc); if (input) delete input; + + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); auto set_text = [&](const char *text) { @@ -68,6 +74,13 @@ describe("Parser", [&]() { AssertThat(new_size, Equals(prev_size - length + new_text.size())); }; + auto assert_root_node = [&](const string &expected) { + TSNode node = ts_document_root_node(doc); + char *actual = ts_node_string(node, doc); + AssertThat(actual, Equals(expected)); + ts_free(actual); + }; + describe("handling errors", [&]() { before_each([&]() { ts_document_set_language(doc, get_test_language("json")); @@ -77,8 +90,8 @@ describe("Parser", [&]() { it("computes the error node's size and position correctly", [&]() { set_text(" [123, @@@@@, true]"); - AssertThat(ts_node_string(root, doc), Equals( - "(array (number) (ERROR (UNEXPECTED '@')) (true))")); + assert_root_node( + "(array (number) (ERROR (UNEXPECTED '@')) (true))"); TSNode error = ts_node_named_child(root, 1); TSNode last = ts_node_named_child(root, 2); @@ -96,8 +109,8 @@ describe("Parser", [&]() { it("computes the error node's size and position correctly", [&]() { set_text(" [123, faaaaalse, true]"); - AssertThat(ts_node_string(root, doc), Equals( - "(array (number) (ERROR (UNEXPECTED 'a')) (true))")); + assert_root_node( + "(array (number) (ERROR (UNEXPECTED 'a')) (true))"); TSNode error = ts_node_named_child(root, 1); TSNode last = ts_node_named_child(root, 2); @@ -117,8 +130,8 @@ describe("Parser", [&]() { it("computes the error node's size and position correctly", [&]() { set_text(" [123, true false, true]"); - AssertThat(ts_node_string(root, doc), Equals( - "(array (number) (ERROR (true) (UNEXPECTED 'f') (false)) (true))")); + assert_root_node( + "(array (number) (ERROR (true) (UNEXPECTED 'f') (false)) (true))"); TSNode error = ts_node_named_child(root, 1); TSNode last = ts_node_named_child(root, 2); @@ -136,8 +149,8 @@ describe("Parser", [&]() { it("computes the error node's size and position correctly", [&]() { set_text(" [123, , true]"); - AssertThat(ts_node_string(root, doc), Equals( - "(array (number) (ERROR (UNEXPECTED ',')) (true))")); + assert_root_node( + "(array (number) (ERROR (UNEXPECTED ',')) (true))"); TSNode error = ts_node_named_child(root, 1); TSNode last = ts_node_named_child(root, 2); @@ -163,8 +176,8 @@ describe("Parser", [&]() { it("is incorporated into the tree", [&]() { set_text("fn()\n"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (function_call (identifier))))")); + assert_root_node( + "(program (expression_statement (function_call (identifier))))"); }); }); @@ -174,9 +187,9 @@ describe("Parser", [&]() { "fn()\n" " .otherFn();"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (function_call " - "(member_access (function_call (identifier)) (identifier)))))")); + "(member_access (function_call (identifier)) (identifier)))))"); }); }); @@ -188,11 +201,11 @@ describe("Parser", [&]() { "\n\n" ".otherFn();"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (function_call " "(member_access (function_call (identifier)) " "(comment) " - "(identifier)))))")); + "(identifier)))))"); }); }); }); @@ -207,17 +220,17 @@ describe("Parser", [&]() { it("updates the parse tree and re-reads only the changed portion of the text", [&]() { set_text("x * (100 + abc);"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (math_op " "(identifier) " - "(math_op (number) (identifier)))))")); + "(math_op (number) (identifier)))))"); insert_text(strlen("x ^ (100 + abc"), ".d"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (math_op " "(identifier) " - "(math_op (number) (member_access (identifier) (identifier))))))")); + "(math_op (number) (member_access (identifier) (identifier))))))"); AssertThat(input->strings_read, Equals(vector({ " abc.d);", "" }))); }); @@ -229,19 +242,19 @@ describe("Parser", [&]() { set_text("123 + 456 * (10 + x);"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (math_op " "(number) " - "(math_op (number) (math_op (number) (identifier))))))")); + "(math_op (number) (math_op (number) (identifier))))))"); insert_text(strlen("123"), " || 5"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (bool_op " "(number) " "(math_op " "(number) " - "(math_op (number) (math_op (number) (identifier)))))))")); + "(math_op (number) (math_op (number) (identifier)))))))"); AssertThat(input->strings_read, Equals(vector({ "123 || 5 +", "" }))); }); @@ -253,20 +266,20 @@ describe("Parser", [&]() { set_text("var x = y;"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (var_declaration (var_assignment " - "(identifier) (identifier))))")); + "(identifier) (identifier))))"); insert_text(strlen("var x = y"), " *"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (var_declaration (ERROR (identifier) (identifier) (UNEXPECTED ';'))))")); + assert_root_node( + "(program (var_declaration (ERROR (identifier) (identifier) (UNEXPECTED ';'))))"); insert_text(strlen("var x = y *"), " z"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (var_declaration (var_assignment " - "(identifier) (math_op (identifier) (identifier)))))")); + "(identifier) (math_op (identifier) (identifier)))))"); }); }); @@ -274,13 +287,13 @@ describe("Parser", [&]() { it("updates the parse tree", [&]() { set_text("abc * 123;"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (identifier) (number))))")); + assert_root_node( + "(program (expression_statement (math_op (identifier) (number))))"); insert_text(strlen("ab"), "XYZ"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (identifier) (number))))")); + assert_root_node( + "(program (expression_statement (math_op (identifier) (number))))"); TSNode node = ts_node_named_descendant_for_range(root, 1, 1); AssertThat(ts_node_name(node, doc), Equals("identifier")); @@ -292,13 +305,13 @@ describe("Parser", [&]() { it("updates the parse tree", [&]() { set_text("abc * 123;"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (identifier) (number))))")); + assert_root_node( + "(program (expression_statement (math_op (identifier) (number))))"); insert_text(strlen("abc"), "XYZ"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (identifier) (number))))")); + assert_root_node( + "(program (expression_statement (math_op (identifier) (number))))"); TSNode node = ts_node_named_descendant_for_range(root, 1, 1); AssertThat(ts_node_name(node, doc), Equals("identifier")); @@ -311,14 +324,14 @@ describe("Parser", [&]() { // 'αβδ' + '1' set_text("'\u03b1\u03b2\u03b4' + '1';"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (string) (string))))")); + assert_root_node( + "(program (expression_statement (math_op (string) (string))))"); // 'αβδ' + 'ψ1' insert_text(strlen("'abd' + '"), "\u03c8"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (string) (string))))")); + assert_root_node( + "(program (expression_statement (math_op (string) (string))))"); }); }); @@ -328,11 +341,11 @@ describe("Parser", [&]() { "// a-comment\n" "abc;"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (math_op " "(number) " "(comment) " - "(identifier))))")); + "(identifier))))"); insert_text( strlen("123 *\n" @@ -340,11 +353,11 @@ describe("Parser", [&]() { "abc"), "XYZ"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (math_op " "(number) " "(comment) " - "(identifier))))")); + "(identifier))))"); }); }); }); @@ -354,13 +367,13 @@ describe("Parser", [&]() { it("updates the parse tree, creating an error", [&]() { set_text("123 * 456;"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (math_op (number) (number))))")); + assert_root_node( + "(program (expression_statement (math_op (number) (number))))"); delete_text(strlen("123 "), 2); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (ERROR (number) (UNEXPECTED '4') (number))))")); + assert_root_node( + "(program (expression_statement (ERROR (number) (UNEXPECTED '4') (number))))"); }); }); }); @@ -371,15 +384,15 @@ describe("Parser", [&]() { set_text("{ x: (b.c) };"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (object (pair " - "(identifier) (member_access (identifier) (identifier))))))")); + "(identifier) (member_access (identifier) (identifier))))))"); replace_text(strlen("{ x: "), strlen("(b.c)"), "b.c"); - AssertThat(ts_node_string(root, doc), Equals( + assert_root_node( "(program (expression_statement (object (pair " - "(identifier) (member_access (identifier) (identifier))))))")); + "(identifier) (member_access (identifier) (identifier))))))"); }); }); @@ -404,8 +417,8 @@ describe("Parser", [&]() { it("terminates them at the end of the document", [&]() { set_text("x; // this is a comment"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (identifier)) (comment))")); + assert_root_node( + "(program (expression_statement (identifier)) (comment))"); TSNode comment = ts_node_named_child(root, 1); @@ -418,8 +431,8 @@ describe("Parser", [&]() { // 'ΩΩΩ — ΔΔ'; set_text("'\u03A9\u03A9\u03A9 \u2014 \u0394\u0394';"); - AssertThat(ts_node_string(root, doc), Equals( - "(program (expression_statement (string)))")); + assert_root_node( + "(program (expression_statement (string)))"); AssertThat(ts_node_end_char(root), Equals(strlen("'OOO - DD';"))); AssertThat(ts_node_end_byte(root), Equals(strlen("'\u03A9\u03A9\u03A9 \u2014 \u0394\u0394';"))); @@ -484,8 +497,8 @@ describe("Parser", [&]() { char *node_string2 = ts_node_string(ts_document_root_node(doc), doc); AssertThat(string(node_string2), Equals(node_string)); - free(node_string2); - free(node_string); + ts_free(node_string2); + ts_free(node_string); }); }); }); diff --git a/src/runtime/parser.c b/src/runtime/parser.c index b04288aa..ec2926dd 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -100,7 +100,10 @@ static ParseActionResult ts_parser__breakdown_top_of_stack(TSParser *self, assert(last_push == StackPushResultMerged); } + for (size_t j = 0, count = first_result->tree_count; j < count; j++) + ts_tree_release(first_result->trees[j]); ts_free(removed_trees); + } while (last_child && last_child->child_count > 0); return UpdatedStackHead; @@ -327,8 +330,10 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, if (pop_result->trees == prior_result->trees) { TSTree **existing_parent = vector_get(&self->reduce_parents, j); parent = *existing_parent; - ts_tree_retain(parent); trailing_extra_count = pop_result->tree_count - parent->child_count; + ts_tree_retain(parent); + for (size_t k = parent->child_count; k < pop_result->tree_count; k++) + ts_tree_retain(pop_result->trees[k]); break; } } @@ -435,10 +440,11 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, case StackPushResultMerged: vector_erase(&self->lookahead_states, new_head); removed_heads++; - continue; + break; case StackPushResultContinued: break; } + ts_tree_release(tree); } } } @@ -494,6 +500,7 @@ static ParseActionResult ts_parser__handle_error(TSParser *self, int head, TSTree *lookahead) { size_t error_token_count = 1; StackEntry *entry_before_error = ts_stack_head(self->stack, head); + ts_tree_retain(lookahead); for (;;) { @@ -517,6 +524,7 @@ static ParseActionResult ts_parser__handle_error(TSParser *self, int head, LOG("recover state:%u, count:%lu", state_after_error, error_token_count + i); ts_parser__reduce_error(self, head, error_token_count + i, lookahead); + ts_tree_release(lookahead); return UpdatedStackHead; } } @@ -533,6 +541,7 @@ static ParseActionResult ts_parser__handle_error(TSParser *self, int head, TSStateId state = ts_stack_top_state(self->stack, head); if (ts_parser__shift(self, head, state, lookahead) == FailedToUpdateStackHead) return FailedToUpdateStackHead; + ts_tree_release(lookahead); lookahead = self->language->lex_fn(&self->lexer, 0, true); if (!lookahead) return FailedToUpdateStackHead; From 1c9dff6dad33573f75c7b495d6d90d96c6085eaf Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Feb 2016 12:10:28 -0800 Subject: [PATCH 10/18] Guard memcpy calls in ts_parser__accept --- src/runtime/parser.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/runtime/parser.c b/src/runtime/parser.c index ec2926dd..d002e47f 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -602,13 +602,16 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { if (!new_children) return FailedToUpdateStackHead; - memcpy(new_children, pop_result->trees, - leading_extra_count * sizeof(TSTree *)); - memcpy(new_children + leading_extra_count, root->children, - root->child_count * sizeof(TSTree *)); - memcpy(new_children + leading_extra_count + root->child_count, - pop_result->trees + leading_extra_count + 1, - trailing_extra_count * sizeof(TSTree *)); + if (leading_extra_count > 0) + memcpy(new_children, pop_result->trees, + leading_extra_count * sizeof(TSTree *)); + if (root->child_count > 0) + memcpy(new_children + leading_extra_count, root->children, + root->child_count * sizeof(TSTree *)); + if (trailing_extra_count > 0) + memcpy(new_children + leading_extra_count + root->child_count, + pop_result->trees + leading_extra_count + 1, + trailing_extra_count * sizeof(TSTree *)); size_t new_count = root->child_count + leading_extra_count + trailing_extra_count; ts_tree_set_children(root, new_count, new_children); From 2da716d659758eabf7751eab2929b2c0b8993c0e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Feb 2016 12:31:13 -0800 Subject: [PATCH 11/18] Update help message for test script --- script/test | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/script/test b/script/test index d7897567..2c6c50e5 100755 --- a/script/test +++ b/script/test @@ -6,7 +6,7 @@ function usage { cat <<-EOF USAGE - $0 [-dghv] [-f focus-string] + $0 [-dgGhv] [-f focus-string] OPTIONS @@ -14,7 +14,9 @@ OPTIONS -d run tests in a debugger (either lldb or gdb) - -g run tests with valgrind + -g run tests with valgrind's memcheck tool + + -G run tests with valgrind's memcheck tool, including a full leak check -v run tests with verbose output From 29c77c34efdc55cf38bc85ef60ff38b687990129 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Feb 2016 13:13:32 -0800 Subject: [PATCH 12/18] Avoid leaking copy of potentially-extra token --- src/runtime/parser.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/runtime/parser.c b/src/runtime/parser.c index d002e47f..8a008208 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -292,13 +292,17 @@ static ParseActionResult ts_parser__shift_extra(TSParser *self, int head, TSTree *lookahead) { TSSymbolMetadata metadata = self->language->symbol_metadata[lookahead->symbol]; if (metadata.structural && ts_stack_head_count(self->stack) > 1) { - lookahead = ts_tree_make_copy(lookahead); - if (!lookahead) + TSTree *copy = ts_tree_make_copy(lookahead); + if (!copy) return FailedToUpdateStackHead; + copy->extra = true; + ParseActionResult result = ts_parser__shift(self, head, state, copy); + ts_tree_release(copy); + return result; + } else { + lookahead->extra = true; + return ts_parser__shift(self, head, state, lookahead); } - - lookahead->extra = true; - return ts_parser__shift(self, head, state, lookahead); } static ParseActionResult ts_parser__reduce(TSParser *self, int head, From a302ee822ab3a2c360344c102ec015b855e3729c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Feb 2016 13:37:23 -0800 Subject: [PATCH 13/18] Add swift runtime memory memory leak to valgrind suppressions --- script/util/valgrind.supp | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/script/util/valgrind.supp b/script/util/valgrind.supp index 2770d1f9..090e58ae 100644 --- a/script/util/valgrind.supp +++ b/script/util/valgrind.supp @@ -1,4 +1,5 @@ -# Errors in bandit's options parser +# Errors + { Memcheck:Cond @@ -30,7 +31,6 @@ fun:main } -# Errors in bandit's assertions { Memcheck:Value8 @@ -65,8 +65,6 @@ fun:_ZN6bandit8describeEPKcNSt3__18functionIFvvEEE } -# CPP strings - { Memcheck:Cond @@ -237,3 +235,22 @@ fun:_ZN6bandit8describeEPKcNSt3__18functionIFvvEEE } +# Leaks + +{ + + Memcheck:Leak + match-leak-kinds: possible + fun:malloc_zone_malloc + fun:_objc_copyClassNamesForImage + fun:_ZL9protocolsv + fun:_Z9readClassP10objc_classbb + fun:gc_init + fun:_ZL33objc_initializeClassPair_internalP10objc_classPKcS0_S0_ + fun:layout_string_create + fun:_ZL12realizeClassP10objc_class + fun:_ZL22copySwiftV1MangledNamePKcb + fun:_ZL22copySwiftV1MangledNamePKcb + fun:_ZL22copySwiftV1MangledNamePKcb + fun:_ZL22copySwiftV1MangledNamePKcb +} From c96c4a08e67cf57ae8780d628f3aa405b36cf3a1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 4 Feb 2016 11:15:46 -0800 Subject: [PATCH 14/18] Use an object pool for stack nodes, to reduce allocations Also, fix some leaks in the case where memory allocation failed during parsing --- spec/runtime/parser_spec.cc | 69 ++++++++++++++++----------- src/runtime/document.c | 22 +++++---- src/runtime/lexer.c | 7 ++- src/runtime/lexer.h | 2 +- src/runtime/parser.c | 66 +++++++++++++++++--------- src/runtime/stack.c | 93 ++++++++++++++++++++++++------------- src/runtime/string_input.c | 6 +++ src/runtime/vector.h | 34 ++++++++++---- 8 files changed, 196 insertions(+), 103 deletions(-) diff --git a/spec/runtime/parser_spec.cc b/spec/runtime/parser_spec.cc index 7e8e45c2..4865960f 100644 --- a/spec/runtime/parser_spec.cc +++ b/spec/runtime/parser_spec.cc @@ -24,7 +24,8 @@ describe("Parser", [&]() { }); after_each([&]() { - ts_document_free(doc); + if (doc) + ts_document_free(doc); if (input) delete input; @@ -440,15 +441,9 @@ describe("Parser", [&]() { }); describe("handling allocation failures", [&]() { - before_each([&]() { - record_alloc::start(); - }); - - after_each([&]() { - record_alloc::stop(); - }); - it("handles failures when allocating documents", [&]() { + record_alloc::start(); + TSDocument *document = ts_document_make(); ts_document_free(document); AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); @@ -462,43 +457,61 @@ describe("Parser", [&]() { AssertThat(ts_document_make(), Equals(nullptr)); AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); } + + record_alloc::stop(); }); it("handles allocation failures during parsing", [&]() { - ts_document_set_language(doc, get_test_language("cpp")); + const TSLanguage *language = get_test_language("cpp"); + const char *input_string = "int main() { return vector().size(); }"; + string expected_node_string = + "(translation_unit (function_definition " + "(identifier) " + "(function_declarator (identifier)) " + "(compound_statement " + "(return_statement (call_expression (field_expression " + "(call_expression (template_call " + "(identifier) " + "(type_name (identifier) (abstract_pointer_declarator)))) " + "(identifier)))))))"; - set_text("int main() { return vector().size(); }"); + record_alloc::start(); + ts_document_set_language(doc, language); + ts_document_set_input_string(doc, input_string); + AssertThat(ts_document_parse(doc), Equals(0)); size_t allocation_count = record_alloc::allocation_count(); AssertThat(allocation_count, IsGreaterThan(1)); - char *node_string = ts_node_string(root, doc); - AssertThat(node_string, Equals("(translation_unit (function_definition " - "(identifier) " - "(function_declarator (identifier)) " - "(compound_statement " - "(return_statement (call_expression (field_expression " - "(call_expression (template_call " - "(identifier) " - "(type_name (identifier) (abstract_pointer_declarator)))) " - "(identifier)))))))")); + assert_root_node(expected_node_string); + ts_document_free(doc); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); for (size_t i = 0; i < allocation_count; i++) { + record_alloc::stop(); + doc = ts_document_make(); + record_alloc::start(); record_alloc::fail_at_allocation_index(i); - ts_document_invalidate(doc); + ts_document_set_language(doc, language); + ts_document_set_input_string(doc, input_string); AssertThat(ts_document_parse(doc), Equals(-1)); + AssertThat(ts_document_root_node(doc).data, Equals(nullptr)); + + ts_document_free(doc); + doc = nullptr; + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); } + record_alloc::stop(); + doc = ts_document_make(); + record_alloc::start(); record_alloc::fail_at_allocation_index(allocation_count + 1); - ts_document_invalidate(doc); + ts_document_set_language(doc, language); + ts_document_set_input_string(doc, input_string); AssertThat(ts_document_parse(doc), Equals(0)); - - char *node_string2 = ts_node_string(ts_document_root_node(doc), doc); - AssertThat(string(node_string2), Equals(node_string)); - ts_free(node_string2); - ts_free(node_string); + assert_root_node(expected_node_string); }); }); }); diff --git a/src/runtime/document.c b/src/runtime/document.c index 9f85f0f3..8eb0025d 100644 --- a/src/runtime/document.c +++ b/src/runtime/document.c @@ -9,14 +9,17 @@ TSDocument *ts_document_make() { TSDocument *self = ts_calloc(1, sizeof(TSDocument)); if (!self) - return NULL; + goto error; - if (!ts_parser_init(&self->parser)) { - ts_free(self); - return NULL; - } + if (!ts_parser_init(&self->parser)) + goto error; return self; + +error: + if (self) + ts_free(self); + return NULL; } void ts_document_free(TSDocument *self) { @@ -61,8 +64,11 @@ void ts_document_set_input(TSDocument *self, TSInput input) { void ts_document_set_input_string(TSDocument *self, const char *text) { ts_document_invalidate(self); - ts_document_set_input(self, ts_string_input_make(text)); - self->owns_input = true; + TSInput input = ts_string_input_make(text); + ts_document_set_input(self, input); + if (input.payload) { + self->owns_input = true; + } } void ts_document_edit(TSDocument *self, TSInputEdit edit) { @@ -80,7 +86,7 @@ void ts_document_edit(TSDocument *self, TSInputEdit edit) { int ts_document_parse(TSDocument *self) { if (!self->input.read_fn || !self->parser.language) - return 0; + return -1; TSTree *reusable_tree = self->valid ? self->tree : NULL; if (reusable_tree && !reusable_tree->has_changes) diff --git a/src/runtime/lexer.c b/src/runtime/lexer.c index 6dacfe0b..66c7dd66 100644 --- a/src/runtime/lexer.c +++ b/src/runtime/lexer.c @@ -122,8 +122,8 @@ static TSTree *ts_lexer__accept(TSLexer *self, TSSymbol symbol, * can call them without needing to be linked against this library. */ -TSLexer ts_lexer_make() { - TSLexer result = (TSLexer){ +void ts_lexer_init(TSLexer *self) { + *self = (TSLexer){ .start_fn = ts_lexer__start, .start_token_fn = ts_lexer__start_token, .advance_fn = ts_lexer__advance, @@ -132,8 +132,7 @@ TSLexer ts_lexer_make() { .chunk_start = 0, .debugger = ts_debugger_null(), }; - ts_lexer_reset(&result, ts_length_zero()); - return result; + ts_lexer_reset(self, ts_length_zero()); } static inline void ts_lexer__reset(TSLexer *self, TSLength position) { diff --git a/src/runtime/lexer.h b/src/runtime/lexer.h index c5957421..8141375d 100644 --- a/src/runtime/lexer.h +++ b/src/runtime/lexer.h @@ -7,7 +7,7 @@ extern "C" { #include "tree_sitter/parser.h" -TSLexer ts_lexer_make(); +void ts_lexer_init(TSLexer *); void ts_lexer_set_input(TSLexer *, TSInput); void ts_lexer_reset(TSLexer *, TSLength); diff --git a/src/runtime/parser.c b/src/runtime/parser.c index 8a008208..10124325 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -356,8 +356,12 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, size_t child_count = pop_result->tree_count - trailing_extra_count; parent = ts_tree_make_node(symbol, child_count, pop_result->trees, metadata); - if (!parent) + if (!parent) { + for (size_t i = 0; i < pop_result->tree_count; i++) + ts_tree_release(pop_result->trees[i]); + ts_free(pop_result->trees); goto error; + } } if (!vector_push(&self->reduce_parents, &parent)) goto error; @@ -424,6 +428,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, */ switch (ts_stack_push(self->stack, new_head, state, parent)) { case StackPushResultFailed: + ts_tree_release(parent); goto error; case StackPushResultMerged: LOG("merge_during_reduce head:%d", new_head); @@ -590,7 +595,7 @@ static ParseActionResult ts_parser__start(TSParser *self, TSInput input, static ParseActionResult ts_parser__accept(TSParser *self, int head) { Vector pop_results = ts_stack_pop(self->stack, head, -1, true); if (!pop_results.size) - return FailedToUpdateStackHead; + goto error; for (size_t j = 0; j < pop_results.size; j++) { StackPopResult *pop_result = vector_get(&pop_results, j); @@ -604,7 +609,7 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { root->child_count + leading_extra_count + trailing_extra_count, sizeof(TSTree *)); if (!new_children) - return FailedToUpdateStackHead; + goto error; if (leading_extra_count > 0) memcpy(new_children, pop_result->trees, @@ -629,6 +634,15 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { } return RemovedStackHead; + +error: + if (pop_results.size) { + StackPopResult *pop_result = vector_get(&pop_results, 0); + for (size_t i = 0; i < pop_result->tree_count; i++) + ts_tree_release(pop_result->trees[i]); + ts_free(pop_result->trees); + } + return FailedToUpdateStackHead; } /* @@ -743,34 +757,43 @@ static ParseActionResult ts_parser__consume_lookahead(TSParser *self, int head, */ bool ts_parser_init(TSParser *self) { + ts_lexer_init(&self->lexer); self->finished_tree = NULL; - self->lexer = ts_lexer_make(); + self->stack = NULL; + self->lookahead_states = vector_new(sizeof(LookaheadState)); + self->reduce_parents = vector_new(sizeof(TSTree *)); self->stack = ts_stack_new(); - if (!self->stack) { - return false; - } + if (!self->stack) + goto error; - self->lookahead_states = vector_new(sizeof(LookaheadState), 4); - if (!self->lookahead_states.contents) { - ts_stack_delete(self->stack); - return false; - } + if (!vector_grow(&self->lookahead_states, 4)) + goto error; - self->reduce_parents = vector_new(sizeof(TSTree *), 4); - if (!self->reduce_parents.contents) { - ts_stack_delete(self->stack); - vector_delete(&self->lookahead_states); - return false; - } + if (!vector_grow(&self->reduce_parents, 4)) + goto error; return true; + +error: + if (self->stack) { + ts_stack_delete(self->stack); + self->stack = NULL; + } + if (self->lookahead_states.contents) + vector_delete(&self->lookahead_states); + if (self->reduce_parents.contents) + vector_delete(&self->reduce_parents); + return false; } void ts_parser_destroy(TSParser *self) { - ts_stack_delete(self->stack); - vector_delete(&self->lookahead_states); - vector_delete(&self->reduce_parents); + if (self->stack) + ts_stack_delete(self->stack); + if (self->lookahead_states.contents) + vector_delete(&self->lookahead_states); + if (self->reduce_parents.contents) + vector_delete(&self->reduce_parents); } TSDebugger ts_parser_debugger(const TSParser *self) { @@ -826,6 +849,7 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { switch (ts_parser__consume_lookahead(self, head, lookahead)) { case FailedToUpdateStackHead: + ts_tree_release(lookahead); goto error; case RemovedStackHead: removed = true; diff --git a/src/runtime/stack.c b/src/runtime/stack.c index 685fc781..7a5cc42d 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -10,6 +10,7 @@ #define MAX_SUCCESSOR_COUNT 8 #define INITIAL_HEAD_CAPACITY 3 #define STARTING_TREE_CAPACITY 10 +#define MAX_NODE_POOL_SIZE 50 typedef struct StackNode { StackEntry entry; @@ -24,6 +25,7 @@ struct Stack { int head_capacity; Vector pop_results; Vector pop_paths; + Vector node_pool; void *tree_selection_payload; TreeSelectionFunction tree_selection_function; }; @@ -50,20 +52,26 @@ Stack *ts_stack_new() { self->head_count = 1; self->head_capacity = INITIAL_HEAD_CAPACITY; + self->heads = NULL; + self->pop_results = vector_new(sizeof(StackPopResult)); + self->pop_paths = vector_new(sizeof(PopPath)); + self->node_pool = vector_new(sizeof(StackNode *)); + self->tree_selection_payload = NULL; + self->tree_selection_function = ts_stack__default_tree_selection; + self->heads = ts_calloc(INITIAL_HEAD_CAPACITY, sizeof(StackNode *)); if (!self->heads) goto error; - self->pop_results = vector_new(sizeof(StackPopResult), 4); - if (!vector_valid(&self->pop_results)) + if (!vector_grow(&self->pop_results, 4)) goto error; - self->pop_paths = vector_new(sizeof(PopPath), 4); - if (!vector_valid(&self->pop_paths)) + if (!vector_grow(&self->pop_paths, 4)) + goto error; + + if (!vector_grow(&self->node_pool, 20)) goto error; - self->tree_selection_payload = NULL; - self->tree_selection_function = ts_stack__default_tree_selection; return self; error: @@ -74,6 +82,8 @@ error: vector_delete(&self->pop_results); if (self->pop_paths.contents) vector_delete(&self->pop_paths); + if (self->node_pool.contents) + vector_delete(&self->node_pool); ts_free(self); } return NULL; @@ -127,40 +137,50 @@ static void stack_node_retain(StackNode *self) { self->ref_count++; } -static bool stack_node_release(StackNode *self) { - if (!self) +static bool stack_node_release(Stack *self, StackNode *node) { + if (!node) return false; - assert(self->ref_count != 0); - self->ref_count--; - if (self->ref_count == 0) { - for (int i = 0; i < self->successor_count; i++) - stack_node_release(self->successors[i]); - ts_tree_release(self->entry.tree); - ts_free(self); + assert(node->ref_count != 0); + node->ref_count--; + if (node->ref_count == 0) { + for (int i = 0; i < node->successor_count; i++) + stack_node_release(self, node->successors[i]); + ts_tree_release(node->entry.tree); + + if (self->node_pool.size >= MAX_NODE_POOL_SIZE) + ts_free(node); + else + vector_push(&self->node_pool, &node); + return true; } else { return false; } } -static StackNode *stack_node_new(StackNode *next, TSStateId state, TSTree *tree) { +static StackNode *stack_node_new(Stack *self, StackNode *next, TSStateId state, TSTree *tree) { assert(tree->ref_count > 0); - StackNode *self = ts_malloc(sizeof(StackNode)); - if (!self) - return NULL; + StackNode *node; + if (self->node_pool.size == 0) { + node = ts_malloc(sizeof(StackNode)); + if (!node) + return NULL; + } else { + node = *(StackNode **)vector_pop(&self->node_pool); + } ts_tree_retain(tree); stack_node_retain(next); TSLength position = ts_tree_total_size(tree); if (next) position = ts_length_add(next->entry.position, position); - *self = (StackNode){ + *node = (StackNode){ .ref_count = 1, .successor_count = 1, .successors = { next, NULL, NULL }, .entry = {.state = state, .tree = tree, .position = position }, }; - return self; + return node; } static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, @@ -225,7 +245,7 @@ static int ts_stack__find_head(Stack *self, StackNode *node) { } void ts_stack_remove_head(Stack *self, int head_index) { - stack_node_release(self->heads[head_index]); + stack_node_release(self, self->heads[head_index]); for (int i = head_index; i < self->head_count - 1; i++) self->heads[i] = self->heads[i + 1]; self->head_count--; @@ -262,11 +282,11 @@ StackPushResult ts_stack_push(Stack *self, int head_index, TSStateId state, if (ts_stack__merge_head(self, head_index, state, tree, position)) return StackPushResultMerged; - StackNode *new_head = stack_node_new(self->heads[head_index], state, tree); + StackNode *new_head = stack_node_new(self, self->heads[head_index], state, tree); if (!new_head) return StackPushResultFailed; - stack_node_release(self->heads[head_index]); + stack_node_release(self, self->heads[head_index]); self->heads[head_index] = new_head; return StackPushResultContinued; } @@ -292,11 +312,11 @@ Vector ts_stack_pop(Stack *self, int head_index, int child_count, PopPath initial_path = { .goal_tree_count = child_count, .node = previous_head, - .trees = vector_new(sizeof(TSTree *), capacity), + .trees = vector_new(sizeof(TSTree *)), .is_shared = false, }; - if (!vector_valid(&initial_path.trees)) + if (!vector_grow(&initial_path.trees, capacity)) goto error; if (!vector_push(&self->pop_paths, &initial_path)) @@ -382,11 +402,12 @@ Vector ts_stack_pop(Stack *self, int head_index, int child_count, goto error; } - stack_node_release(previous_head); + stack_node_release(self, previous_head); return self->pop_results; error: - return vector_new(0, 0); + vector_delete(&initial_path.trees); + return vector_new(0); } void ts_stack_shrink(Stack *self, int head_index, int count) { @@ -398,13 +419,13 @@ void ts_stack_shrink(Stack *self, int head_index, int count) { new_head = new_head->successors[0]; } stack_node_retain(new_head); - stack_node_release(head); + stack_node_release(self, head); self->heads[head_index] = new_head; } void ts_stack_clear(Stack *self) { for (int i = 0; i < self->head_count; i++) - stack_node_release(self->heads[i]); + stack_node_release(self, self->heads[i]); self->head_count = 1; self->heads[0] = NULL; } @@ -416,9 +437,17 @@ void ts_stack_set_tree_selection_callback(Stack *self, void *payload, } void ts_stack_delete(Stack *self) { - vector_delete(&self->pop_results); - vector_delete(&self->pop_paths); + if (self->pop_paths.contents) + vector_delete(&self->pop_results); + if (self->pop_paths.contents) + vector_delete(&self->pop_paths); ts_stack_clear(self); + for (size_t i = 0; i < self->node_pool.size; i++) { + StackNode **node = vector_get(&self->node_pool, i); + ts_free(*node); + } + if (self->node_pool.contents) + vector_delete(&self->node_pool); ts_free(self->heads); ts_free(self); } diff --git a/src/runtime/string_input.c b/src/runtime/string_input.c index 2ebd9c00..b7bf47b3 100644 --- a/src/runtime/string_input.c +++ b/src/runtime/string_input.c @@ -28,6 +28,9 @@ int ts_string_input_seek(void *payload, size_t character, size_t byte) { TSInput ts_string_input_make(const char *string) { TSStringInput *input = ts_malloc(sizeof(TSStringInput)); + if (!input) + goto error; + input->string = string; input->position = 0; input->length = strlen(string); @@ -36,4 +39,7 @@ TSInput ts_string_input_make(const char *string) { .read_fn = ts_string_input_read, .seek_fn = ts_string_input_seek, }; + +error: + return (TSInput){NULL, NULL, NULL}; } diff --git a/src/runtime/vector.h b/src/runtime/vector.h index ef36bb16..6bc2ed20 100644 --- a/src/runtime/vector.h +++ b/src/runtime/vector.h @@ -18,21 +18,29 @@ typedef struct { size_t element_size; } Vector; -static inline Vector vector_new(size_t element_size, size_t capacity) { +static inline Vector vector_new(size_t element_size) { Vector result; + result.contents = NULL; result.size = 0; - result.capacity = capacity; + result.capacity = 0; result.element_size = element_size; - - if (capacity > 0) { - result.contents = ts_calloc(capacity, element_size); - if (!result.contents) - result.element_size = 0; - } - return result; } +static inline bool vector_grow(Vector *self, size_t capacity) { + void *new_contents; + if (self->contents) + new_contents = ts_realloc(self->contents, capacity * self->element_size); + else + new_contents = ts_calloc(capacity, self->element_size); + + if (!new_contents) + return false; + self->capacity = capacity; + self->contents = new_contents; + return true; +} + static inline bool vector_valid(Vector *self) { return self->element_size > 0; } @@ -41,6 +49,8 @@ static inline void vector_delete(Vector *self) { if (self->contents) { ts_free(self->contents); self->contents = NULL; + self->size = 0; + self->capacity = 0; } } @@ -54,6 +64,12 @@ static inline void *vector_back(Vector *self) { return vector_get(self, self->size - 1); } +static inline void *vector_pop(Vector *self) { + void *result = vector_back(self); + self->size--; + return result; +} + static inline void vector_clear(Vector *self) { self->size = 0; } From f6f02182c15fac1828188dab3a18af7351088a48 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 4 Feb 2016 12:59:44 -0800 Subject: [PATCH 15/18] Tail-call-optimize recursive functions Refs https://github.com/maxbrunsfeld/node-tree-sitter-compiler/pull/7 --- src/runtime/tree.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/runtime/tree.c b/src/runtime/tree.c index 6d15fb36..f631ad30 100644 --- a/src/runtime/tree.c +++ b/src/runtime/tree.c @@ -62,13 +62,21 @@ TSTree *ts_tree_make_copy(TSTree *self) { } void ts_tree_assign_parents(TSTree *self) { - TSLength offset = ts_length_zero(); + TSLength offset; + +recur: + offset = ts_length_zero(); for (size_t i = 0; i < self->child_count; i++) { TSTree *child = self->children[i]; if (child->context.parent != self) { child->context.parent = self; child->context.index = i; child->context.offset = offset; + if (i == self->child_count - 1) { + self = child; + goto recur; + } + ts_tree_assign_parents(child); } offset = ts_length_add(offset, ts_tree_total_size(child)); @@ -135,13 +143,23 @@ void ts_tree_retain(TSTree *self) { void ts_tree_release(TSTree *self) { if (!self) return; + +recur: assert(self->ref_count > 0); self->ref_count--; + if (self->ref_count == 0) { - for (size_t i = 0; i < self->child_count; i++) - ts_tree_release(self->children[i]); - if (self->child_count > 0) + if (self->child_count > 0) { + for (size_t i = 0; i < self->child_count - 1; i++) + ts_tree_release(self->children[i]); + TSTree *last_child = self->children[self->child_count - 1]; ts_free(self->children); + ts_free(self); + + self = last_child; + goto recur; + } + ts_free(self); } } From 2f2ca401be189556918ba86ec5e70a074186b879 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 5 Feb 2016 11:18:22 -0800 Subject: [PATCH 16/18] Fix last few leaks, add leak checking to all randomized specs --- spec/integration/corpus_specs.cc | 6 +++- src/runtime/parser.c | 51 ++++++++++++++++++-------------- src/runtime/vector.h | 3 ++ 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/spec/integration/corpus_specs.cc b/spec/integration/corpus_specs.cc index a986a2d3..9d9cd191 100644 --- a/spec/integration/corpus_specs.cc +++ b/spec/integration/corpus_specs.cc @@ -1,16 +1,18 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "helpers/test_languages.h" #include "helpers/read_test_entries.h" #include "helpers/spy_input.h" #include "helpers/log_debugger.h" #include "helpers/point_helpers.h" #include "helpers/encoding_helpers.h" +#include "helpers/record_alloc.h" #include static void expect_the_correct_tree(TSNode node, TSDocument *document, string tree_string) { const char *node_string = ts_node_string(node, document); AssertThat(node_string, Equals(tree_string)); - free((void *)node_string); + ts_free((void *)node_string); } static void expect_a_consistent_tree(TSNode node, TSDocument *document) { @@ -105,6 +107,7 @@ describe("The Corpus", []() { TSDocument *document; before_each([&]() { + record_alloc::start(); document = ts_document_make(); ts_document_set_language(document, get_test_language(language_name)); // ts_document_set_debugger(document, log_debugger_make(true)); @@ -112,6 +115,7 @@ describe("The Corpus", []() { after_each([&]() { ts_document_free(document); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); for (auto &entry : read_corpus_entries(language_dir + "/grammar_test")) { diff --git a/src/runtime/parser.c b/src/runtime/parser.c index 10124325..76507e78 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -605,28 +605,35 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { TSTree *root = pop_result->trees[i]; size_t leading_extra_count = i; size_t trailing_extra_count = pop_result->tree_count - 1 - i; - TSTree **new_children = ts_calloc( - root->child_count + leading_extra_count + trailing_extra_count, - sizeof(TSTree *)); - if (!new_children) - goto error; - - if (leading_extra_count > 0) - memcpy(new_children, pop_result->trees, - leading_extra_count * sizeof(TSTree *)); - if (root->child_count > 0) - memcpy(new_children + leading_extra_count, root->children, - root->child_count * sizeof(TSTree *)); - if (trailing_extra_count > 0) - memcpy(new_children + leading_extra_count + root->child_count, - pop_result->trees + leading_extra_count + 1, - trailing_extra_count * sizeof(TSTree *)); size_t new_count = root->child_count + leading_extra_count + trailing_extra_count; - ts_tree_set_children(root, new_count, new_children); + + if (new_count > 0) { + TSTree **new_children = ts_calloc(new_count, sizeof(TSTree *)); + if (!new_children) + goto error; + if (leading_extra_count > 0) + memcpy(new_children, pop_result->trees, + leading_extra_count * sizeof(TSTree *)); + if (root->child_count > 0) + memcpy(new_children + leading_extra_count, root->children, + root->child_count * sizeof(TSTree *)); + if (trailing_extra_count > 0) + memcpy(new_children + leading_extra_count + root->child_count, + pop_result->trees + leading_extra_count + 1, + trailing_extra_count * sizeof(TSTree *)); + ts_tree_set_children(root, new_count, new_children); + } + ts_parser__remove_head(self, pop_result->head_index); - self->finished_tree = - ts_parser__select_tree(self, self->finished_tree, root); + TSTree *tree = ts_parser__select_tree(self, self->finished_tree, root); + if (tree == root) { + ts_tree_release(self->finished_tree); + self->finished_tree = root; + } else { + ts_tree_release(root); + } + ts_free(pop_result->trees); break; } @@ -834,10 +841,8 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { ts_stack_head_count(self->stack), ts_stack_top_state(self->stack, head), position.chars); - if (position.chars == last_position.chars && - ts_parser__can_reuse(self, head, lookahead)) { - ts_tree_retain(lookahead); - } else { + if (position.chars != last_position.chars || + !ts_parser__can_reuse(self, head, lookahead)) { ts_tree_release(lookahead); lookahead = ts_parser__get_next_lookahead(self, head); if (!lookahead) diff --git a/src/runtime/vector.h b/src/runtime/vector.h index 6bc2ed20..4c33c9d8 100644 --- a/src/runtime/vector.h +++ b/src/runtime/vector.h @@ -28,6 +28,9 @@ static inline Vector vector_new(size_t element_size) { } static inline bool vector_grow(Vector *self, size_t capacity) { + if (capacity == 0) + return true; + void *new_contents; if (self->contents) new_contents = ts_realloc(self->contents, capacity * self->element_size); From b1c8b74e9c57c5dbc3d329472bd384878e7591c8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 5 Feb 2016 11:35:40 -0800 Subject: [PATCH 17/18] Avoid leak caused by earlier corpus spec failures --- spec/integration/corpus_specs.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/integration/corpus_specs.cc b/spec/integration/corpus_specs.cc index 9d9cd191..8f264e8f 100644 --- a/spec/integration/corpus_specs.cc +++ b/spec/integration/corpus_specs.cc @@ -11,8 +11,9 @@ static void expect_the_correct_tree(TSNode node, TSDocument *document, string tree_string) { const char *node_string = ts_node_string(node, document); - AssertThat(node_string, Equals(tree_string)); + string result(node_string); ts_free((void *)node_string); + AssertThat(result, Equals(tree_string)); } static void expect_a_consistent_tree(TSNode node, TSDocument *document) { From b80a330a74a38f772ba60634088b7c0de1d040f0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 5 Feb 2016 12:23:54 -0800 Subject: [PATCH 18/18] Fix assorted memory leaks in test code --- include/tree_sitter/compiler.h | 4 ++-- spec/helpers/load_language.cc | 6 +++-- spec/integration/compile_grammar_spec.cc | 28 +++++++++++++----------- spec/runtime/document_spec.cc | 4 ++++ spec/runtime/tree_spec.cc | 2 ++ src/compiler/compile.cc | 8 +++---- src/compiler/parse_grammar.cc | 1 + 7 files changed, 32 insertions(+), 21 deletions(-) diff --git a/include/tree_sitter/compiler.h b/include/tree_sitter/compiler.h index 4439b617..e3878f33 100644 --- a/include/tree_sitter/compiler.h +++ b/include/tree_sitter/compiler.h @@ -16,8 +16,8 @@ typedef enum { } TSCompileErrorType; typedef struct { - const char *code; - const char *error_message; + char *code; + char *error_message; TSCompileErrorType error_type; } TSCompileResult; diff --git a/spec/helpers/load_language.cc b/spec/helpers/load_language.cc index 15d00f59..70f17639 100644 --- a/spec/helpers/load_language.cc +++ b/spec/helpers/load_language.cc @@ -40,11 +40,13 @@ static std::string run_cmd(const char *cmd, const char *args[]) { const TSLanguage *load_language(const string &name, const TSCompileResult &compile_result) { if (compile_result.error_type != TSCompileErrorTypeNone) { - AssertThat(string(compile_result.error_message), IsEmpty()); + Assert::Failure(string("Compilation failed ") + compile_result.error_message); return nullptr; } - return load_language(name, compile_result.code); + const TSLanguage *language = load_language(name, compile_result.code); + free(compile_result.code); + return language; } const TSLanguage *load_language(const string &name, const string &code) { diff --git a/spec/integration/compile_grammar_spec.cc b/spec/integration/compile_grammar_spec.cc index 0e69d998..58458f58 100644 --- a/spec/integration/compile_grammar_spec.cc +++ b/spec/integration/compile_grammar_spec.cc @@ -1,4 +1,5 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "helpers/load_language.h" START_TEST @@ -14,6 +15,13 @@ describe("compile_grammar", []() { ts_document_free(document); }); + auto assert_root_node = [&](const string &expected_string) { + TSNode root_node = ts_document_root_node(document); + char *node_string = ts_node_string(root_node, document); + AssertThat(node_string, Equals(expected_string)); + ts_free(node_string); + }; + describe("when the grammar's start symbol is a token", [&]() { it("parses the token", [&]() { TSCompileResult result = ts_compile_grammar(R"JSON( @@ -29,8 +37,7 @@ describe("compile_grammar", []() { ts_document_set_input_string(document, "the-value"); ts_document_parse(document); - TSNode root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals("(first_rule)")); + assert_root_node("(first_rule)"); }); }); @@ -49,8 +56,7 @@ describe("compile_grammar", []() { ts_document_set_input_string(document, ""); ts_document_parse(document); - TSNode root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals("(first_rule)")); + assert_root_node("(first_rule)"); }); }); @@ -77,18 +83,15 @@ describe("compile_grammar", []() { ts_document_set_input_string(document, "1234"); ts_document_parse(document); - TSNode root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals("(first_rule)")); + assert_root_node("(first_rule)"); ts_document_set_input_string(document, "\n"); ts_document_parse(document); - root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals("(first_rule)")); + assert_root_node("(first_rule)"); ts_document_set_input_string(document, "'hello'"); ts_document_parse(document); - root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals("(first_rule)")); + assert_root_node("(first_rule)"); }); }); @@ -177,13 +180,12 @@ describe("compile_grammar", []() { ts_document_set_input_string(document, "a + b * c"); ts_document_parse(document); - TSNode root_node = ts_document_root_node(document); - AssertThat(ts_node_string(root_node, document), Equals( + assert_root_node( "(expression (sum " "(expression (variable)) " "(expression (product " "(expression (variable)) " - "(expression (variable))))))")); + "(expression (variable))))))"); }); }); }); diff --git a/spec/runtime/document_spec.cc b/spec/runtime/document_spec.cc index 85552fcf..5da23b4f 100644 --- a/spec/runtime/document_spec.cc +++ b/spec/runtime/document_spec.cc @@ -151,6 +151,10 @@ describe("Document", [&]() { ts_document_set_input_string(doc, "[1, 2]"); }); + after_each([&]() { + delete debugger; + }); + it("calls the debugger with a message for each lex action", [&]() { ts_document_set_debugger(doc, debugger->debugger()); ts_document_parse(doc); diff --git a/spec/runtime/tree_spec.cc b/spec/runtime/tree_spec.cc index dcec0a0f..a0761c37 100644 --- a/spec/runtime/tree_spec.cc +++ b/spec/runtime/tree_spec.cc @@ -64,6 +64,8 @@ describe("Tree", []() { AssertThat(error_tree->fragile_left, IsTrue()); AssertThat(error_tree->fragile_right, IsTrue()); + + ts_tree_release(error_tree); }); }); diff --git a/src/compiler/compile.cc b/src/compiler/compile.cc index fdd6af3c..cc9d8155 100644 --- a/src/compiler/compile.cc +++ b/src/compiler/compile.cc @@ -18,7 +18,7 @@ using std::make_tuple; extern "C" TSCompileResult ts_compile_grammar(const char *input) { ParseGrammarResult parse_result = parse_grammar(string(input)); if (!parse_result.error_message.empty()) { - return { "", strdup(parse_result.error_message.c_str()), + return { nullptr, strdup(parse_result.error_message.c_str()), TSCompileErrorTypeInvalidGrammar }; } @@ -28,7 +28,7 @@ extern "C" TSCompileResult ts_compile_grammar(const char *input) { const LexicalGrammar &lexical_grammar = get<1>(prepare_grammar_result); CompileError error = get<2>(prepare_grammar_result); if (error.type) { - return { "", strdup(error.message.c_str()), error.type }; + return { nullptr, strdup(error.message.c_str()), error.type }; } auto table_build_result = @@ -37,13 +37,13 @@ extern "C" TSCompileResult ts_compile_grammar(const char *input) { const LexTable &lex_table = get<1>(table_build_result); error = get<2>(table_build_result); if (error.type) { - return { "", strdup(error.message.c_str()), error.type }; + return { nullptr, strdup(error.message.c_str()), error.type }; } string code = generate_code::c_code(parse_result.name, parse_table, lex_table, syntax_grammar, lexical_grammar); - return { strdup(code.c_str()), "", TSCompileErrorTypeNone }; + return { strdup(code.c_str()), nullptr, TSCompileErrorTypeNone }; } pair compile(const Grammar &grammar, diff --git a/src/compiler/parse_grammar.cc b/src/compiler/parse_grammar.cc index 263c68ae..653e9cca 100644 --- a/src/compiler/parse_grammar.cc +++ b/src/compiler/parse_grammar.cc @@ -313,6 +313,7 @@ ParseGrammarResult parse_grammar(const string &input) { } } + json_value_free(grammar_json); return { name, grammar, "" }; error: