Hi Vincent, Gnulib's list are certainly very convenient, but their lack of templating also makes things harder to follow: typing is weak, everything is a gl_list_t.
I think we should at least use typedef to clarify what different types of list exist (e.g., parse_state_list_t). I'm of course well aware that the compiler will not check we use these types consistently, but it should definitely help us understanding what is going on. I have installed the appended commit. The memory leaks are clearly coming from the parse simulator, but I don't understand enough of its architecture to fix them. It seems clear for instance that we are leaking new_root, but adding `free_parse_state (new_root)` where it seems to make sense results in crashes. I'm attaching testsuite.log for 243 with line numbers. This was the piece of information I was missing last time. If you want them too, here's how I got them on a mac (I should add this to README-hacking). 1. I used gcc 10 (coming from MacPorts FWIW, sudo port install gcc10) 2. I configured with $ ~bison/configure '-C' 'CC=gcc-mp-10 -fsanitize=address' 'CXX=g++-mp-10.0 -fsanitize=address' 'LDFLAGS=-L/opt/local/lib' '--enable-gcc-warnings' 'CFLAGS= -ggdb' 'CPPFLAGS=-isystem /opt/local/include' 'CXXFLAGS=-ggdb' 3. compile 4. Force the generation of debug symbols: $ dsymutil src/bison 5. Run the cex tests with leak detection enabled $ make check-local TESTSUITEFLAGS='-j5 -k cex' ASAN_OPTIONS=detect_leaks=1 I couldn't get clang to generate file/line diagnostics. I believe all the "plain" compilers (no sanitizers) of the CI will properly pass this time, I fixed the last minor portability issue: https://travis-ci.org/github/akimd/bison/builds/688241839 Vincent, if there's a way I can be of use here, please tell me. I do not understand how memory management is dealt with here, so I feel useless. It should be documented somewhere. Is it? Maybe I overlooked that. Cheers!
testsuite.log
Description: Binary data
commit 556a01cb123e1d75a53d588f8a73454b319d9ad2 Author: Akim Demaille <[email protected]> Date: Mon May 18 08:00:37 2020 +0200 cex: style changes in parse-simulation * src/parse-simulation.c: Formatting changes. (parse_state_list_new): New. Use it. diff --git a/src/parse-simulation.c b/src/parse-simulation.c index 1320efaa..0c399e64 100644 --- a/src/parse-simulation.c +++ b/src/parse-simulation.c @@ -37,7 +37,6 @@ typedef struct size_t total_size; } ps_chunk; -struct parse_state; typedef struct parse_state { // path of state-items the parser has traversed @@ -86,11 +85,11 @@ static int frees = 0; static parse_state * empty_parse_state (void) { - parse_state *res = xcalloc (1, sizeof (parse_state)); - res->state_items.contents = gl_list_create_empty (GL_LINKED_LIST, NULL, - NULL, NULL, true); - res->derivs.contents = gl_list_create_empty (GL_LINKED_LIST, NULL, - NULL, NULL, true); + parse_state *res = xcalloc (1, sizeof *res); + res->state_items.contents + = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); + res->derivs.contents + = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); ++allocs; return res; } @@ -109,10 +108,10 @@ copy_parse_state (bool prepend, parse_state *parent) { parse_state *res = xmalloc (sizeof *res); memcpy (res, parent, sizeof *res); - res->state_items.contents = gl_list_create_empty (GL_LINKED_LIST, NULL, - NULL, NULL, true); - res->derivs.contents = gl_list_create_empty (GL_LINKED_LIST, NULL, - NULL, NULL, true); + res->state_items.contents + = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); + res->derivs.contents + = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); res->parent = parent; res->prepend = prepend; res->reference_count = 0; @@ -216,8 +215,9 @@ parse_state_comparator (const parse_state *ps1, const parse_state *ps2) { const ps_chunk *sis1 = &ps1->state_items; const ps_chunk *sis2 = &ps2->state_items; - return sis1->head_elt == sis2->head_elt && - sis1->tail_elt == sis2->tail_elt && sis1->total_size == sis2->total_size; + return sis1->head_elt == sis2->head_elt + && sis1->tail_elt == sis2->tail_elt + && sis1->total_size == sis2->total_size; } @@ -273,6 +273,14 @@ list_flatten_and_split (gl_list_t *list, gl_list_t *rets, int split, int n) } } +static gl_list_t +parse_state_list_new (void) +{ + return gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, + (gl_listelement_dispose_fn)free_parse_state, + true); +} + // Emulates a reduction on a parse state by popping some amount of // derivations and state_items off of the parse_state and returning // the result in ret. Returns the derivation of what's popped. @@ -283,7 +291,7 @@ parser_pop (parse_state *ps, int deriv_index, // prepend sis, append sis, prepend derivs, append derivs gl_list_t chunks[4]; for (int i = 0; i < 4; ++i) - chunks[i] = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, 1); + chunks[i] = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); for (parse_state *pn = ps; pn != NULL; pn = pn->parent) { if (pn->prepend) @@ -298,7 +306,7 @@ parser_pop (parse_state *ps, int deriv_index, } } gl_list_t popped_derivs = - gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, 1); + gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, NULL, true); gl_list_t ret_chunks[4] = { ret->state_items.contents, NULL, ret->derivs.contents, popped_derivs }; @@ -385,10 +393,7 @@ simulate_transition (parse_state *ps) symbol_number sym = item_number_as_symbol_number (*si->item); // Transition on the same next symbol, taking nullable // symbols into account. - gl_list_t result = - gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn)free_parse_state, - true); + gl_list_t result = parse_state_list_new (); state_item_number si_next = si_trans[si - state_items]; // check for disabled transition, shouldn't happen // as any state_items that lead to these should be @@ -427,10 +432,7 @@ compatible (symbol_number sym1, symbol_number sym2) gl_list_t simulate_production (parse_state *ps, symbol_number compat_sym) { - gl_list_t result = - gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn)free_parse_state, - true); + gl_list_t result = parse_state_list_new (); const state_item *si = parse_state_tail (ps); bitset prod = si_prods_lookup (si - state_items); if (prod) @@ -463,10 +465,7 @@ simulate_production (parse_state *ps, symbol_number compat_sym) gl_list_t simulate_reduction (parse_state *ps, int rule_len, bitset symbol_set) { - gl_list_t result = - gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn)free_parse_state, - true); + gl_list_t result = parse_state_list_new (); int s_size = ps->state_items.total_size; int d_size = ps->derivs.total_size; @@ -524,10 +523,7 @@ simulate_reduction (parse_state *ps, int rule_len, bitset symbol_set) gl_list_t parser_prepend (parse_state *ps) { - gl_list_t result = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn) - free_parse_state, - true); + gl_list_t result = parse_state_list_new (); const state_item *head = ps->state_items.head_elt; bitset prev = si_revs[head - state_items]; symbol_number prepend_sym =
