[patch] Fix symtab/14601

Doug Evans dje@google.com
Wed Oct 3 17:05:00 GMT 2012


On Fri, Sep 21, 2012 at 12:17 PM, Doug Evans <dje@google.com> wrote:
> On Fri, Sep 21, 2012 at 9:38 AM, Tom Tromey <tromey@redhat.com> wrote:
>> Doug> Not all code paths to handle DW_TAG_imported_{declaration,module}
>> Doug> ensure using_directives is reset to NULL.
>> Doug> This patch fixes things by "going with the flow" and resetting
>> Doug> using_directives to NULL in buildsym_init along with the others.
>>
>> What code paths result in the bad behavior?
>> I think this patch is fine but I wonder if the underlying bug has other
>> possible bad effects.
>
> The only code that currently resets using_directives to NULL is
> finish_block_internal and push_context.
> dwarf2read.c resets it to the previous value in read_func_scope and
> read_lexical_block_scope, but that doesn't guarantee the "the next
> guy" that needs to expand symbols (i.e. invoke the buildsym machinery)
> will have using_directives == NULL at the start.
> Note that this is counter to how buildsym is generally set up.
> Normally (at least as I look at it) globals are always reset at the
> start (e.g. global_symbols), not reset at the end (as for
> using_directives).  [I know really_free_pendings sets global_symbols =
> NULL but to my mind that's just more for robustness sake, not for
> preparing the machinery for the next time - I'm thinking of
> buildsym_init as constructor-like and really_free_pendings as
> destructor-like.]
>
> So the code path here is anything that leaves using_directives != NULL
> at the end of symbol expansion, and then coming back into the buildsym
> machinery and calling cp_add_using_directives without having first
> gone through finish_block_internal or push_context.
>
> If you want to add more robustness, e.g. reset using_directives to
> NULL in, say, really_free_pendings, to say trigger a segv if it's
> errantly used outside of buildsym that'd be great.  But I think
> buildsym_init is the right place to initialize it for the current
> pass.

Committed.



More information about the Gdb-patches mailing list