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

[Doug Evans <dje@google.com>]

On Mon, Aug 3, 2015 at 3:47 PM, Kevin Buettner <kevinb@redhat.com> wrote:
> This is a fix for PR 16822.  Keith Seitz wrote the patch for the fix. I
> wrote the test case.
>
> The Texas Instruments compiler uses the encoding for
> DW_AT_MIPS_linkage_name for other purposes.  TI uses the encoding,
> 0x2007, for TI_AT_TI_end_line which, unlike DW_AT_MIPS_linkage_name,
> does not have a string-typed value.  Keith's patch simply makes sure
> that linkage_name attributes have a string type prior to attempting to
> decode them as such.
>
> My test case causes GDB to segfault in an unpatched GDB.  There
> will be one PASS in a patched GDB.
>
> Unpatched GDB:
>
> (gdb) file testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name
> Reading symbols from testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name...done.
> ERROR: Couldn't load testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name into gdb (eof).
> ERROR: Couldn't send ptype return_true to GDB.
> UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype return_true
>
> Patched GDB:
>
> (gdb) file testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name
> Reading symbols from /mesquite2/sourceware-git/mesquite-native-5894223/bld/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name...done.
> (gdb) ptype return_true
> type = bool (void)
> (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype return_true
>
> gdb/ChangeLog:
>
>         * dwarf2read.c (dwarf2_physname): Verify that the attribute
>         is a string prior to decoding it as such.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.dwarf2/dw2-bad-mips-linkage-name.S: New file.
>         * gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.

Hi.

If I wanted to regenerate the .S how would I do it?
[generated .S files needs such instructions]
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.

>
> 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.