Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding

[Kevin Buettner <kevinb at redhat dot com>]

On Mon, 3 Aug 2015 16:31:08 -0700
Doug Evans <dje@google.com> wrote:

> On Mon, Aug 3, 2015 at 3:47 PM, Kevin Buettner <kevinb@redhat.com> wrote:
>
> >
> >         * gdb.dwarf2/dw2-bad-mips-linkage-name.S: New file.
> >         * gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.
> 
> If I wanted to regenerate the .S how would I do it?
> [generated .S files needs such instructions]

You wouldn't, at least not easily.

I used a circa 2010 mips-elf toolchain to create a .S file which I
then modified by hand, both removing sections not relevant to the test
as well as adding in a reference to the DW_AT_MIPS_linkage_name
attribute that's not a string. 

> I wonder, though, if this is a good place for using the dwarf assembler.
> Seems so. We just need a MIPS_linkage_name attribute
> that isn't a string. The dwarf assembler test would be a lot
> smaller.

Thanks for the suggestion.  I'll look into this.

> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> > index 24a4022..9eb7cbc 100644
> > --- a/gdb/dwarf2read.c
> > +++ b/gdb/dwarf2read.c
> > @@ -8722,7 +8722,10 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
> >
> >    /* DW_AT_linkage_name is missing in some cases - depend on what GDB
> >       has computed.  */
> > -  if (attr && DW_STRING (attr))
> > +  if (attr
> > +      && (attr->form == DW_FORM_strp || attr->form == DW_FORM_string
> > +          || attr->form == DW_FORM_GNU_strp_alt)
> > +      && DW_STRING (attr))
> >      {
> >        char *demangled;
> 
> One thought that comes to mind is that gdb should protect itself
> from all such mistakes.
> 
> What if there was a wrapper on dwarf2_attr, dwarf2_string_attr
> or some such, and it returned either the attribute (if the attribute
> is present *and* is a string) or NULL.
> And if the attribute is present but not a string it logs a
> complaint (standard bad debug info complaint) and returns NULL.

Sounds reasonable.  I'll take a look at this too.

Kevin