Re: %destructor

[Hans Aberg]

> On 27 Sep 2014, at 04:01, Bob Rossi <address@hidden> wrote:
> 
> On Sat, Sep 27, 2014 at 12:23:37AM +0200, Hans Aberg wrote:

>>> In the second sentence you mentioned I should write deallocators in each
>>> instance matching the allocator used in the actions. THEN add
>>> %destructor if needed for error recovery.
>>> 
>>> Where do I put deallocators in the bison generator parser, besides
>>> %destructor?
>> 
>> I am normally using C++, which is doing the cleanup, but let’s try C 
>> (pseudocode):
>> 
>> list:
>>   "[“ sequence[x] "]"    { $$ = $x; }
>> ;
>> 
>> sequence:
>>   item[x]                  { $$ = make_list1($x); free($x); }
>> | sequence[s] "," item[x]  { $$ = make_list($s, $x); free($s); free($x); }
>> ;
> 
> OK. Thanks for explaining. I took a different strategy here.
> Maybe my strategy is wrong. You can see a slightly out of date version
> of my grammar here,
>  https://github.com/brasko/gdbwire/blob/master/src/lib/gdbmi/gdbmi_grammar.y
> Please consider the following type,
> 
>    struct gdbmi_result {
>        enum gdbmi_result_kind kind;
>        char *variable;
>        union {
>            char *cstring;
>            struct gdbmi_result *result;
>        } variant;
>        struct gdbmi_result *next;
>    };
> 
> Please consider the following result_list rules,
>    result_list: {
>        $$ = NULL;
>    };
> 
>    result_list: result_list COMMA result {
>        $$ = append_gdbmi_result ($1, $3);
>    };
> 
>    result: opt_variable CSTRING {
>        $$ = gdbmi_result_alloc();
>        $$->variable = $1;
>        $$->kind = GDBMI_CSTRING;
>        $$->variant.cstring = strdup(gdbmi_text);
>    };
> 
>    result: opt_variable tuple {
>        $$ = gdbmi_result_alloc();
>        $$->variable = $1;
>        $$->kind = GDBMI_TUPLE;
>        $$->variant.result = $2;
>    };
> 
>    result: opt_variable list {
>        $$ = gdbmi_result_alloc();
>        $$->variable = $1;
>        $$->kind = GDBMI_LIST;
>        $$->variant.result = $2;
>    };
> 
> My approach is to build up $$. I never deallocate in the actions, and
> valgrind claims I have no memory leaks in the unit tests. In the end, on
> a successful parse, I end up with a 'struct gdbmi_result *' which has
> the next pointer set, etc. Hopefully the above is an OK strategy.

It looks like you are building a linked list, so the pointer you have created 
are always in use. Then you do not have to deallocate those.

> The problem I'm facing is related to error handling for sure.
> If I add this, (and I use malloc under the hood for all allocations)
>    %destructor { free($$); } result result_list;
> I still have memory leaks (valgrind assures me). I'm not a bison expert,
> and so it's not exactly obvious to me which items it chooses to call the
> destructor on.  If I change the free routine to
>    %destructor { gdbmi_result_free($$); } result result_list;
> then the memory leaks go away (according to valgrind). gdbmi_result_free
> free's each member in 'struct gdbmi_result' and also free's the next
> field.

You should deallocate the object that $$ points to, that is, if it is a linked 
list, and the components are not in use elsewhere, the free_list($$) should be 
used. This is what is done in the Bison manual. 

> Since I don't understand what exactly Bison calls the destructor on,

Just the parser stack $$ values in the part that it unwinds during error 
recovery.

> I'm
> wondering,
> 1) If I free the entire struct gdbmi_result with gdbmi_result_free($$)
> and I also have a destructor for $1 in the above examples,
>    %destructor { free($$); } variable
> Is it possible I'll have a double free?

The %destructor is only called during error recovery, so it does not double 
deallocations in the actions.

> 2) Because I'm using gdbmi_result_free($$) which also free's the next
> pointer of a list, is it possible Bison will also call the destructor on
> each member of the list item, causing a double free?

It depends: are those components used by something else? Then that is going to 
be complicated. Build a component from below and up. Then those parts that are 
not in use can be deallocated: you have some parts that not yet have been 
connected, and %destructor takes away those.

> I know I'm not doing a great job here asking this question. Sorry.
> If this is to unclear, I'll spend some time formulating a much more
> precise question, which may perhaps help me answer my own question.

Just shoot. Perhaps somebody else will chime in.