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

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

[Kevin Buettner]

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.
---
 gdb/dwarf2read.c                                   |   5 +-
 .../gdb.dwarf2/dw2-bad-mips-linkage-name.S         | 207 +++++++++++++++++++++
 .../gdb.dwarf2/dw2-bad-mips-linkage-name.exp       |  36 ++++
 3 files changed, 247 insertions(+), 1 deletion(-)

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;
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.S b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.S
new file mode 100644
index 0000000..562e708
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.S
@@ -0,0 +1,207 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This test includes both DW_AT_MIPS_linkage_name and DW_AT_TI_end_line,
+   both of which use the same encoding.  It doesn't matter to us whether
+   GDB do anything useful with DW_AT_TI_end_line; we just don't want it
+   to crash.  */
+
+	.section	.debug_abbrev,"",@progbits
+$Ldebug_abbrev0:
+	.section	.debug_info,"",@progbits
+$Ldebug_info0:
+	.section	.debug_line,"",@progbits
+$Ldebug_line0:
+	.text
+$Ltext0:
+	.globl	_Z11return_truev
+$LFB0 = .
+	.file 1 "dw2-bad-mips-linkage-name.cc"
+_Z11return_truev:
+	.4byte 0xabcdef
+$LFE0:
+	.globl	main
+$LFB1 = .
+main:
+	.4byte 0xfedbca
+$LFE1:
+	.size	main, .-main
+$Letext0:
+	.section	.debug_info
+	.4byte	0x5d	 # Length of Compilation Unit Info
+	.2byte	0x2	 # DWARF version number
+	.4byte	$Ldebug_abbrev0	 # Offset Into Abbrev. Section
+	.byte	0x4	 # Pointer Size (in bytes)
+	.uleb128 0x1	 # (DIE (0xb) DW_TAG_compile_unit)
+	.4byte	$LASF1	 # DW_AT_producer: "GNU C++"
+	.byte	0x4	 # DW_AT_language
+	.4byte	$LASF2	 # DW_AT_name: "dw2-bad-mips-linkage-name.cc"
+	.4byte	$LASF3	 # DW_AT_comp_dir: "/"
+	.4byte	$Ltext0	 # DW_AT_low_pc
+	.4byte	$Letext0	 # DW_AT_high_pc
+	.4byte	$Ldebug_line0	 # DW_AT_stmt_list
+	.uleb128 0x2	 # (DIE (0x25) DW_TAG_subprogram)
+	.byte	0x1	 # DW_AT_external
+	.4byte	$LASF4	 # DW_AT_name: "return_true"
+	.byte	0x1	 # DW_AT_decl_file (dw2-bad-mips-linkage-name.cc)
+	.byte	0x2	 # DW_AT_decl_line
+	.4byte	$LASF5	 # DW_AT_MIPS_linkage_name: "_Z11return_truev"
+	.4byte	0x3d	 # DW_AT_type
+	.4byte	$LFB0	 # DW_AT_low_pc
+	.4byte	$LFE0	 # DW_AT_high_pc
+	.uleb128 0x3	 # (DIE (0x41) DW_TAG_base_type)
+	.byte	0x1	 # DW_AT_byte_size
+	.byte	0x2	 # DW_AT_encoding
+	.4byte	$LASF0	 # DW_AT_name: "bool"
+	.uleb128 0x4	 # (DIE (0x48) DW_TAG_subprogram)
+	.byte	0x1	 # DW_AT_external
+	.4byte	$LASF6	 # DW_AT_name: "main"
+	.byte	0x1	 # DW_AT_decl_file (dw2-bad-mips-linkage-name.cc)
+	.byte	0x8	 # DW_AT_decl_line
+	.byte	0x6      # DW_AT_TI_end_line (which shares same encoding
+	                 # with DW_AT_MIPS_linkage_name)
+	.4byte	0x59	 # DW_AT_type
+	.4byte	$LFB1	 # DW_AT_low_pc
+	.4byte	$LFE1	 # DW_AT_high_pc
+	.uleb128 0x5	 # (DIE (0x60) DW_TAG_base_type)
+	.byte	0x4	 # DW_AT_byte_size
+	.byte	0x5	 # DW_AT_encoding
+	.ascii "int\0"	 # DW_AT_name
+	.byte	0x0	 # end of children of DIE 0xb
+	.section	.debug_abbrev
+	.uleb128 0x1	 # (abbrev code)
+	.uleb128 0x11	 # (TAG: DW_TAG_compile_unit)
+	.byte	0x1	 # DW_children_yes
+	.uleb128 0x25	 # (DW_AT_producer)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x13	 # (DW_AT_language)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3	 # (DW_AT_name)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x1b	 # (DW_AT_comp_dir)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x11	 # (DW_AT_low_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.uleb128 0x12	 # (DW_AT_high_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.uleb128 0x10	 # (DW_AT_stmt_list)
+	.uleb128 0x6	 # (DW_FORM_data4)
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x2	 # (abbrev code)
+	.uleb128 0x2e	 # (TAG: DW_TAG_subprogram)
+	.byte	0x0	 # DW_children_no
+	.uleb128 0x3f	 # (DW_AT_external)
+	.uleb128 0xc	 # (DW_FORM_flag)
+	.uleb128 0x3	 # (DW_AT_name)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x3a	 # (DW_AT_decl_file)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3b	 # (DW_AT_decl_line)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x2007	 # (DW_AT_MIPS_linkage_name)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x49	 # (DW_AT_type)
+	.uleb128 0x13	 # (DW_FORM_ref4)
+	.uleb128 0x11	 # (DW_AT_low_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.uleb128 0x12	 # (DW_AT_high_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x3	 # (abbrev code)
+	.uleb128 0x24	 # (TAG: DW_TAG_base_type)
+	.byte	0x0	 # DW_children_no
+	.uleb128 0xb	 # (DW_AT_byte_size)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3e	 # (DW_AT_encoding)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3	 # (DW_AT_name)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x4	 # (abbrev code)
+	.uleb128 0x2e	 # (TAG: DW_TAG_subprogram)
+	.byte	0x0	 # DW_children_no
+	.uleb128 0x3f	 # (DW_AT_external)
+	.uleb128 0xc	 # (DW_FORM_flag)
+	.uleb128 0x3	 # (DW_AT_name)
+	.uleb128 0xe	 # (DW_FORM_strp)
+	.uleb128 0x3a	 # (DW_AT_decl_file)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3b	 # (DW_AT_decl_line)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x2007  # (DW_AT_TI_end_line / DW_AT_MIPS_linkage_name)
+	.uleb128 0xb     # (DW_FORM_data1)
+	.uleb128 0x49	 # (DW_AT_type)
+	.uleb128 0x13	 # (DW_FORM_ref4)
+	.uleb128 0x11	 # (DW_AT_low_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.uleb128 0x12	 # (DW_AT_high_pc)
+	.uleb128 0x1	 # (DW_FORM_addr)
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x5	 # (abbrev code)
+	.uleb128 0x24	 # (TAG: DW_TAG_base_type)
+	.byte	0x0	 # DW_children_no
+	.uleb128 0xb	 # (DW_AT_byte_size)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3e	 # (DW_AT_encoding)
+	.uleb128 0xb	 # (DW_FORM_data1)
+	.uleb128 0x3	 # (DW_AT_name)
+	.uleb128 0x8	 # (DW_FORM_string)
+	.byte	0x0
+	.byte	0x0
+	.byte	0x0
+	.section	.debug_pubnames,"",@progbits
+	.4byte	0x27	 # Length of Public Names Info
+	.2byte	0x2	 # DWARF Version
+	.4byte	$Ldebug_info0	 # Offset of Compilation Unit Info
+	.4byte	0x68	 # Compilation Unit Length
+	.4byte	0x25	 # DIE offset
+	.ascii "return_true\0"	 # external name
+	.4byte	0x40	 # DIE offset
+	.ascii "main\0"	 # external name
+	.4byte	0x0
+	.section	.debug_aranges,"",@progbits
+	.4byte	0x1c	 # Length of Address Ranges Info
+	.2byte	0x2	 # DWARF Version
+	.4byte	$Ldebug_info0	 # Offset of Compilation Unit Info
+	.byte	0x4	 # Size of Address
+	.byte	0x0	 # Size of Segment Descriptor
+	.2byte	0x0	 # Pad to 8 byte boundary
+	.2byte	0x0
+	.4byte	$Ltext0	 # Address
+	.4byte	$Letext0-$Ltext0	 # Length
+	.4byte	0x0
+	.4byte	0x0
+	.section	.debug_str,"MS",@progbits,1
+$LASF2:
+	.ascii	"dw2-bad-mips-linkage-name.cc\000"
+$LASF0:
+	.ascii	"bool\000"
+$LASF5:
+	.ascii	"_Z11return_truev\000"
+$LASF1:
+	.ascii	"GNU C++"
+$LASF4:
+	.ascii	"return_true\000"
+$LASF3:
+	.ascii	"/\000"
+$LASF6:
+	.ascii	"main\000"
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
new file mode 100644
index 0000000..91c640a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -0,0 +1,36 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+# This testfile has reproducibility only with cc-with-index.sh.
+
+standard_testfile .S
+
+if [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] {
+    return -1
+}
+
+# A successful run will have just one PASS.  An unsuccessful run will
+# likely show errors out before getting to this point due to a
+# segfault in GDB.
+
+gdb_test "ptype return_true" " = bool \\(void\\)"
+

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

[Doug Evans]

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.

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

[Kevin Buettner]

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

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

[Kevin Buettner]

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

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

My updated patch, below, does this.

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

I've introduced the wrapper that you recommend and have used it in
all places that made sense to me.  There were a few spots where using
it would have made things more complicated, so I left those alone.

Here's the updated change / patch:

dwarf2read.c: Check type of string valued attributes prior to decoding.

This change introduces a new function, dwarf2_string_attr(), which is
a wrapper for dwarf2_attr().  dwarf2read.c has been updated to
call dwarf2_string_attr in most instances where a string-valued
attribute is decoded to produce a string value.  In most cases, it
simplifies the code; in some instances, the complexity of the code
remains unchanged.

I performed this change by looking for instances where the
result of DW_STRING was used in an assignment.  Many of these
had a pattern which (roughly) looks something like this:

  struct attribute *attr = NULL;

  attr = dwarf2_attr (die, name, cu);
  if (attr != NULL && DW_STRING (attr))
    {
      const char *str;
      ...
      str = DW_STRING (attr);
      ... /* Use str in some fashion.  */
    }

Code of this form is transformed to look like this instead:

  const char *str;

  str = dwarf2_string_attr (die, name, cu)
  if (str != NULL)
    {
       ...
       /* Use str in some fashion.  */
       ...
    }

In addition to invoking dwarf2_attr() and DW_STRING(),
dwarf2_string_attr() checks to make sure that the attribute's
`form' field matches one of DW_FORM_strp, DW_FORM_string, or
DW_FORM_GNU_strp_alt.  If it does not match one of these forms,
it will return a NULL value in addition to calling complaint().

An earlier version of this patch did this type checking for one
particular instance where a string attribute was being decoded.
The situation that I was attempting to handle in that earlier patch is
this:

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.  In this instance, GDB was attempting
to use an integer value as a string pointer, with predictable results.
(GDB would die with a segmentation fault.)

I've added a test which reproduces the problem that I was orignally
wanting to fix.  It uses DW_AT_MIPS_linkage name with an associate
value which is a string, and again, where the value is a small
integer.

My test case causes GDB to segfault in an unpatched GDB.  There
will be two PASSes in a patched GDB.

Unpatched GDB:

(gdb) ptype f
ERROR: Process no longer exists
UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
ERROR: Couldn't send ptype g to GDB.
UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g

Patched GDB:

(gdb) ptype f
type = bool ()
(gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
ptype g
type = bool ()
(gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g

I see no regressions on an x86_64 native target.

gdb/ChangeLog:

	* dwarf2read.c (dwarf2_string_attr): New function.
	(lookup_dwo_unit, process_psymtab_comp_unit_reader)
	(dwarf2_compute_name, dwarf2_physname, find_file_and_directory)
	(read_call_site_scope, namespace_name, guess_full_die_structure_name)
	(anonymous_struct_prefix, prepare_one_comp_unit): Use
	dwarf2_string_attr in place of dwarf2_attr and DW_STRING.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file.
	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.
---
 gdb/dwarf2read.c                                   | 134 +++++++++++----------
 .../gdb.dwarf2/dw2-bad-mips-linkage-name.exp       |  72 +++++++++++
 2 files changed, 141 insertions(+), 65 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7e79342..e017549 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1523,6 +1523,9 @@ static struct attribute *dwarf2_attr (struct die_info *, unsigned int,
 static struct attribute *dwarf2_attr_no_follow (struct die_info *,
 						unsigned int);
 
+static const char *dwarf2_string_attr (struct die_info *die, unsigned int name,
+                                       struct dwarf2_cu *cu);
+
 static int dwarf2_flag_true_p (struct die_info *die, unsigned name,
                                struct dwarf2_cu *cu);
 
@@ -5267,13 +5270,8 @@ lookup_dwo_unit (struct dwarf2_per_cu_data *this_cu,
   gdb_assert (cu != NULL);
 
   /* Yeah, we look dwo_name up again, but it simplifies the code.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
-  gdb_assert (attr != NULL);
-  dwo_name = DW_STRING (attr);
-  comp_dir = NULL;
-  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
-  if (attr)
-    comp_dir = DW_STRING (attr);
+  dwo_name = dwarf2_string_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
+  comp_dir = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
 
   if (this_cu->is_debug_types)
     {
@@ -5935,7 +5933,6 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = cu->objfile;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct dwarf2_per_cu_data *per_cu = cu->per_cu;
-  struct attribute *attr;
   CORE_ADDR baseaddr;
   CORE_ADDR best_lowpc = 0, best_highpc = 0;
   struct partial_symtab *pst;
@@ -5953,18 +5950,14 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
   cu->list_in_scope = &file_symbols;
 
   /* Allocate a new partial symbol table structure.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_name, cu);
-  if (attr == NULL || !DW_STRING (attr))
+  filename = dwarf2_string_attr (comp_unit_die, DW_AT_name, cu);
+  if (filename == NULL)
     filename = "";
-  else
-    filename = DW_STRING (attr);
 
   pst = create_partial_symtab (per_cu, filename);
 
   /* This must be done before calling dwarf2_build_include_psymtabs.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
-  if (attr != NULL)
-    pst->dirname = DW_STRING (attr);
+  pst->dirname = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -8457,13 +8450,13 @@ dwarf2_compute_name (const char *name,
 	 to be able to reference.  Ideally, we want the user to be able
 	 to reference this entity using either natural or linkage name,
 	 but we haven't started looking at this enhancement yet.  */
-      struct attribute *attr;
+      const char *name;
 
-      attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
-      if (attr == NULL)
-	attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
-      if (attr && DW_STRING (attr))
-	return DW_STRING (attr);
+      name = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
+      if (name == NULL)
+	name = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
+      if (name != NULL)
+	return name;
     }
 
   /* These are the only languages we know how to qualify names in.  */
@@ -8716,18 +8709,16 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
 
   back_to = make_cleanup (null_cleanup, NULL);
 
-  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
-  if (!attr)
-    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
+  mangled = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
+  if (mangled == NULL)
+    mangled = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
 
   /* DW_AT_linkage_name is missing in some cases - depend on what GDB
      has computed.  */
-  if (attr && DW_STRING (attr))
+  if (mangled != NULL)
     {
       char *demangled;
 
-      mangled = DW_STRING (attr);
-
       /* Use DMGL_RET_DROP for C++ template functions to suppress their return
 	 type.  It is easier for GDB users to search for such functions as
 	 `name(params)' than `long name(params)'.  In such case the minimal
@@ -9038,24 +9029,14 @@ static void
 find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu,
 			 const char **name, const char **comp_dir)
 {
-  struct attribute *attr;
-
-  *name = NULL;
-  *comp_dir = NULL;
-
   /* Find the filename.  Do not use dwarf2_name here, since the filename
      is not a source language identifier.  */
-  attr = dwarf2_attr (die, DW_AT_name, cu);
-  if (attr)
-    {
-      *name = DW_STRING (attr);
-    }
+  *name = dwarf2_string_attr (die, DW_AT_name, cu);
+  *comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
 
-  attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
-  if (attr)
-    *comp_dir = DW_STRING (attr);
-  else if (producer_is_gcc_lt_4_3 (cu) && *name != NULL
-	   && IS_ABSOLUTE_PATH (*name))
+  if (*comp_dir == NULL
+      && producer_is_gcc_lt_4_3 (cu) && *name != NULL
+      && IS_ABSOLUTE_PATH (*name))
     {
       char *d = ldirname (*name);
 
@@ -11697,17 +11678,17 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       gdb_assert (target_cu->objfile == objfile);
       if (die_is_declaration (target_die, target_cu))
 	{
-	  const char *target_physname = NULL;
-	  struct attribute *target_attr;
+	  const char *target_physname;
 
 	  /* Prefer the mangled name; otherwise compute the demangled one.  */
-	  target_attr = dwarf2_attr (target_die, DW_AT_linkage_name, target_cu);
-	  if (target_attr == NULL)
-	    target_attr = dwarf2_attr (target_die, DW_AT_MIPS_linkage_name,
-				       target_cu);
-	  if (target_attr != NULL && DW_STRING (target_attr) != NULL)
-	    target_physname = DW_STRING (target_attr);
-	  else
+	  target_physname = dwarf2_string_attr (target_die,
+						DW_AT_linkage_name,
+						target_cu);
+	  if (target_physname == NULL)
+	    target_physname = dwarf2_string_attr (target_die,
+						 DW_AT_MIPS_linkage_name,
+						 target_cu);
+	  if (target_physname == NULL)
 	    target_physname = dwarf2_physname (NULL, target_die, target_cu);
 	  if (target_physname == NULL)
 	    complaint (&symfile_complaints,
@@ -14177,10 +14158,8 @@ namespace_name (struct die_info *die, int *is_anonymous, struct dwarf2_cu *cu)
     {
       /* We don't use dwarf2_name here so that we can detect the absence
 	 of a name -> anonymous namespace.  */
-      struct attribute *attr = dwarf2_attr (die, DW_AT_name, cu);
+      name = dwarf2_string_attr (die, DW_AT_name, cu);
 
-      if (attr != NULL)
-	name = DW_STRING (attr);
       if (name != NULL)
 	break;
     }
@@ -17083,6 +17062,33 @@ dwarf2_attr_no_follow (struct die_info *die, unsigned int name)
   return NULL;
 }
 
+/* Return the string associated with a string-typed attribute, or NULL if it
+   is either not found or is of an incorrect type.  */
+
+static const char *
+dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *cu)
+{
+  struct attribute *attr;
+  const char *str = NULL;
+
+  attr = dwarf2_attr (die, name, cu);
+
+  if (attr != NULL)
+    {
+      if (attr->form == DW_FORM_strp || attr->form == DW_FORM_string
+	  || attr->form == DW_FORM_GNU_strp_alt)
+	str = DW_STRING (attr);
+      else
+        complaint (&symfile_complaints,
+	           _("string type expected for attribute %s for "
+		     "DIE at 0x%x in module %s"),
+		   dwarf_attr_name (name), die->offset.sect_off,
+		   objfile_name (cu->objfile));
+    }
+
+  return str;
+}
+
 /* Return non-zero iff the attribute NAME is defined for the given DIE,
    and holds a non-zero value.  This function should only be used for
    DW_FORM_flag or DW_FORM_flag_present attributes.  */
@@ -19140,16 +19146,17 @@ guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
     {
       if (child->tag == DW_TAG_subprogram)
 	{
-	  struct attribute *attr;
+	  const char *linkage_name;
 
-	  attr = dwarf2_attr (child, DW_AT_linkage_name, cu);
-	  if (attr == NULL)
-	    attr = dwarf2_attr (child, DW_AT_MIPS_linkage_name, cu);
-	  if (attr != NULL)
+	  linkage_name = dwarf2_string_attr (child, DW_AT_linkage_name, cu);
+	  if (linkage_name == NULL)
+	    linkage_name = dwarf2_string_attr (child, DW_AT_MIPS_linkage_name,
+	                                       cu);
+	  if (linkage_name != NULL)
 	    {
 	      char *actual_name
 		= language_class_name_from_physname (cu->language_defn,
-						     DW_STRING (attr));
+						     linkage_name);
 	      char *name = NULL;
 
 	      if (actual_name != NULL)
@@ -19197,8 +19204,7 @@ anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
       && die->tag != DW_TAG_structure_type && die->tag != DW_TAG_union_type)
     return NULL;
 
-  attr = dwarf2_attr (die, DW_AT_name, cu);
-  if (attr != NULL && DW_STRING (attr) != NULL)
+  if (dwarf2_string_attr (die, DW_AT_name, cu) != NULL)
     return NULL;
 
   attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
@@ -22069,9 +22075,7 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
       cu->language_defn = language_def (cu->language);
     }
 
-  attr = dwarf2_attr (comp_unit_die, DW_AT_producer, cu);
-  if (attr)
-    cu->producer = DW_STRING (attr);
+  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);
 }
 
 /* Release one cached compilation unit, CU.  We unlink it from the tree
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
new file mode 100644
index 0000000..77f6175
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -0,0 +1,72 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile dw2-bad-mips-linkage-name.c dw2-bad-mips-linkage-name.S
+
+# Set up the DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+	DW_TAG_compile_unit {
+                {DW_AT_language @DW_LANG_C}
+                {DW_AT_name     dw2-bad-mips-linkage-name.c}
+                {DW_AT_comp_dir /tmp}
+
+        } {
+	    declare_labels b_l
+
+	    b_l: DW_TAG_base_type {
+		{DW_AT_byte_size 1 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      bool}
+	    }
+            DW_TAG_subprogram {
+                {name f}
+                {low_pc f addr}
+                {high_pc f_end_lbl addr}
+                {type :$b_l}
+		{DW_AT_MIPS_linkage_name _Z1fv}
+	    }
+            DW_TAG_subprogram {
+                {name g}
+                {low_pc g addr}
+                {high_pc g_end_lbl addr}
+                {type :$b_l}
+		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+# A successful run will have two PASSes.  A GDB that's lacking
+# attribute type checking will segfault at some point.  It doesn't
+# much matter what we test here, so long as we do something to make
+# sure that the DWARF is read.
+
+gdb_test "ptype f" " = bool \\(\\)"
+gdb_test "ptype g" " = bool \\(\\)"

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

[Kevin Buettner]

Ping.

On Thu, 6 Aug 2015 15:38:45 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> On Mon, 3 Aug 2015 16:31:08 -0700
> Doug Evans <dje@google.com> wrote:
> 
> > 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.
> 
> My updated patch, below, does this.
> 
> > 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.
> 
> I've introduced the wrapper that you recommend and have used it in
> all places that made sense to me.  There were a few spots where using
> it would have made things more complicated, so I left those alone.
> 
> Here's the updated change / patch:
> 
> dwarf2read.c: Check type of string valued attributes prior to decoding.
> 
> This change introduces a new function, dwarf2_string_attr(), which is
> a wrapper for dwarf2_attr().  dwarf2read.c has been updated to
> call dwarf2_string_attr in most instances where a string-valued
> attribute is decoded to produce a string value.  In most cases, it
> simplifies the code; in some instances, the complexity of the code
> remains unchanged.
> 
> I performed this change by looking for instances where the
> result of DW_STRING was used in an assignment.  Many of these
> had a pattern which (roughly) looks something like this:
> 
>   struct attribute *attr = NULL;
> 
>   attr = dwarf2_attr (die, name, cu);
>   if (attr != NULL && DW_STRING (attr))
>     {
>       const char *str;
>       ...
>       str = DW_STRING (attr);
>       ... /* Use str in some fashion.  */
>     }
> 
> Code of this form is transformed to look like this instead:
> 
>   const char *str;
> 
>   str = dwarf2_string_attr (die, name, cu)
>   if (str != NULL)
>     {
>        ...
>        /* Use str in some fashion.  */
>        ...
>     }
> 
> In addition to invoking dwarf2_attr() and DW_STRING(),
> dwarf2_string_attr() checks to make sure that the attribute's
> `form' field matches one of DW_FORM_strp, DW_FORM_string, or
> DW_FORM_GNU_strp_alt.  If it does not match one of these forms,
> it will return a NULL value in addition to calling complaint().
> 
> An earlier version of this patch did this type checking for one
> particular instance where a string attribute was being decoded.
> The situation that I was attempting to handle in that earlier patch is
> this:
> 
> 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.  In this instance, GDB was attempting
> to use an integer value as a string pointer, with predictable results.
> (GDB would die with a segmentation fault.)
> 
> I've added a test which reproduces the problem that I was orignally
> wanting to fix.  It uses DW_AT_MIPS_linkage name with an associate
> value which is a string, and again, where the value is a small
> integer.
> 
> My test case causes GDB to segfault in an unpatched GDB.  There
> will be two PASSes in a patched GDB.
> 
> Unpatched GDB:
> 
> (gdb) ptype f
> ERROR: Process no longer exists
> UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
> ERROR: Couldn't send ptype g to GDB.
> UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
> 
> Patched GDB:
> 
> (gdb) ptype f
> type = bool ()
> (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
> ptype g
> type = bool ()
> (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
> 
> I see no regressions on an x86_64 native target.
> 
> gdb/ChangeLog:
> 
> 	* dwarf2read.c (dwarf2_string_attr): New function.
> 	(lookup_dwo_unit, process_psymtab_comp_unit_reader)
> 	(dwarf2_compute_name, dwarf2_physname, find_file_and_directory)
> 	(read_call_site_scope, namespace_name, guess_full_die_structure_name)
> 	(anonymous_struct_prefix, prepare_one_comp_unit): Use
> 	dwarf2_string_attr in place of dwarf2_attr and DW_STRING.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file.
> 	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.
> ---
>  gdb/dwarf2read.c                                   | 134 +++++++++++----------
>  .../gdb.dwarf2/dw2-bad-mips-linkage-name.exp       |  72 +++++++++++
>  2 files changed, 141 insertions(+), 65 deletions(-)
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 7e79342..e017549 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1523,6 +1523,9 @@ static struct attribute *dwarf2_attr (struct die_info *, unsigned int,
>  static struct attribute *dwarf2_attr_no_follow (struct die_info *,
>  						unsigned int);
>  
> +static const char *dwarf2_string_attr (struct die_info *die, unsigned int name,
> +                                       struct dwarf2_cu *cu);
> +
>  static int dwarf2_flag_true_p (struct die_info *die, unsigned name,
>                                 struct dwarf2_cu *cu);
>  
> @@ -5267,13 +5270,8 @@ lookup_dwo_unit (struct dwarf2_per_cu_data *this_cu,
>    gdb_assert (cu != NULL);
>  
>    /* Yeah, we look dwo_name up again, but it simplifies the code.  */
> -  attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
> -  gdb_assert (attr != NULL);
> -  dwo_name = DW_STRING (attr);
> -  comp_dir = NULL;
> -  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
> -  if (attr)
> -    comp_dir = DW_STRING (attr);
> +  dwo_name = dwarf2_string_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
> +  comp_dir = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
>  
>    if (this_cu->is_debug_types)
>      {
> @@ -5935,7 +5933,6 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
>    struct objfile *objfile = cu->objfile;
>    struct gdbarch *gdbarch = get_objfile_arch (objfile);
>    struct dwarf2_per_cu_data *per_cu = cu->per_cu;
> -  struct attribute *attr;
>    CORE_ADDR baseaddr;
>    CORE_ADDR best_lowpc = 0, best_highpc = 0;
>    struct partial_symtab *pst;
> @@ -5953,18 +5950,14 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
>    cu->list_in_scope = &file_symbols;
>  
>    /* Allocate a new partial symbol table structure.  */
> -  attr = dwarf2_attr (comp_unit_die, DW_AT_name, cu);
> -  if (attr == NULL || !DW_STRING (attr))
> +  filename = dwarf2_string_attr (comp_unit_die, DW_AT_name, cu);
> +  if (filename == NULL)
>      filename = "";
> -  else
> -    filename = DW_STRING (attr);
>  
>    pst = create_partial_symtab (per_cu, filename);
>  
>    /* This must be done before calling dwarf2_build_include_psymtabs.  */
> -  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
> -  if (attr != NULL)
> -    pst->dirname = DW_STRING (attr);
> +  pst->dirname = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
>  
>    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>  
> @@ -8457,13 +8450,13 @@ dwarf2_compute_name (const char *name,
>  	 to be able to reference.  Ideally, we want the user to be able
>  	 to reference this entity using either natural or linkage name,
>  	 but we haven't started looking at this enhancement yet.  */
> -      struct attribute *attr;
> +      const char *name;
>  
> -      attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
> -      if (attr == NULL)
> -	attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
> -      if (attr && DW_STRING (attr))
> -	return DW_STRING (attr);
> +      name = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
> +      if (name == NULL)
> +	name = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
> +      if (name != NULL)
> +	return name;
>      }
>  
>    /* These are the only languages we know how to qualify names in.  */
> @@ -8716,18 +8709,16 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
>  
>    back_to = make_cleanup (null_cleanup, NULL);
>  
> -  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
> -  if (!attr)
> -    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
> +  mangled = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
> +  if (mangled == NULL)
> +    mangled = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
>  
>    /* DW_AT_linkage_name is missing in some cases - depend on what GDB
>       has computed.  */
> -  if (attr && DW_STRING (attr))
> +  if (mangled != NULL)
>      {
>        char *demangled;
>  
> -      mangled = DW_STRING (attr);
> -
>        /* Use DMGL_RET_DROP for C++ template functions to suppress their return
>  	 type.  It is easier for GDB users to search for such functions as
>  	 `name(params)' than `long name(params)'.  In such case the minimal
> @@ -9038,24 +9029,14 @@ static void
>  find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu,
>  			 const char **name, const char **comp_dir)
>  {
> -  struct attribute *attr;
> -
> -  *name = NULL;
> -  *comp_dir = NULL;
> -
>    /* Find the filename.  Do not use dwarf2_name here, since the filename
>       is not a source language identifier.  */
> -  attr = dwarf2_attr (die, DW_AT_name, cu);
> -  if (attr)
> -    {
> -      *name = DW_STRING (attr);
> -    }
> +  *name = dwarf2_string_attr (die, DW_AT_name, cu);
> +  *comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
>  
> -  attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
> -  if (attr)
> -    *comp_dir = DW_STRING (attr);
> -  else if (producer_is_gcc_lt_4_3 (cu) && *name != NULL
> -	   && IS_ABSOLUTE_PATH (*name))
> +  if (*comp_dir == NULL
> +      && producer_is_gcc_lt_4_3 (cu) && *name != NULL
> +      && IS_ABSOLUTE_PATH (*name))
>      {
>        char *d = ldirname (*name);
>  
> @@ -11697,17 +11678,17 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
>        gdb_assert (target_cu->objfile == objfile);
>        if (die_is_declaration (target_die, target_cu))
>  	{
> -	  const char *target_physname = NULL;
> -	  struct attribute *target_attr;
> +	  const char *target_physname;
>  
>  	  /* Prefer the mangled name; otherwise compute the demangled one.  */
> -	  target_attr = dwarf2_attr (target_die, DW_AT_linkage_name, target_cu);
> -	  if (target_attr == NULL)
> -	    target_attr = dwarf2_attr (target_die, DW_AT_MIPS_linkage_name,
> -				       target_cu);
> -	  if (target_attr != NULL && DW_STRING (target_attr) != NULL)
> -	    target_physname = DW_STRING (target_attr);
> -	  else
> +	  target_physname = dwarf2_string_attr (target_die,
> +						DW_AT_linkage_name,
> +						target_cu);
> +	  if (target_physname == NULL)
> +	    target_physname = dwarf2_string_attr (target_die,
> +						 DW_AT_MIPS_linkage_name,
> +						 target_cu);
> +	  if (target_physname == NULL)
>  	    target_physname = dwarf2_physname (NULL, target_die, target_cu);
>  	  if (target_physname == NULL)
>  	    complaint (&symfile_complaints,
> @@ -14177,10 +14158,8 @@ namespace_name (struct die_info *die, int *is_anonymous, struct dwarf2_cu *cu)
>      {
>        /* We don't use dwarf2_name here so that we can detect the absence
>  	 of a name -> anonymous namespace.  */
> -      struct attribute *attr = dwarf2_attr (die, DW_AT_name, cu);
> +      name = dwarf2_string_attr (die, DW_AT_name, cu);
>  
> -      if (attr != NULL)
> -	name = DW_STRING (attr);
>        if (name != NULL)
>  	break;
>      }
> @@ -17083,6 +17062,33 @@ dwarf2_attr_no_follow (struct die_info *die, unsigned int name)
>    return NULL;
>  }
>  
> +/* Return the string associated with a string-typed attribute, or NULL if it
> +   is either not found or is of an incorrect type.  */
> +
> +static const char *
> +dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *cu)
> +{
> +  struct attribute *attr;
> +  const char *str = NULL;
> +
> +  attr = dwarf2_attr (die, name, cu);
> +
> +  if (attr != NULL)
> +    {
> +      if (attr->form == DW_FORM_strp || attr->form == DW_FORM_string
> +	  || attr->form == DW_FORM_GNU_strp_alt)
> +	str = DW_STRING (attr);
> +      else
> +        complaint (&symfile_complaints,
> +	           _("string type expected for attribute %s for "
> +		     "DIE at 0x%x in module %s"),
> +		   dwarf_attr_name (name), die->offset.sect_off,
> +		   objfile_name (cu->objfile));
> +    }
> +
> +  return str;
> +}
> +
>  /* Return non-zero iff the attribute NAME is defined for the given DIE,
>     and holds a non-zero value.  This function should only be used for
>     DW_FORM_flag or DW_FORM_flag_present attributes.  */
> @@ -19140,16 +19146,17 @@ guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
>      {
>        if (child->tag == DW_TAG_subprogram)
>  	{
> -	  struct attribute *attr;
> +	  const char *linkage_name;
>  
> -	  attr = dwarf2_attr (child, DW_AT_linkage_name, cu);
> -	  if (attr == NULL)
> -	    attr = dwarf2_attr (child, DW_AT_MIPS_linkage_name, cu);
> -	  if (attr != NULL)
> +	  linkage_name = dwarf2_string_attr (child, DW_AT_linkage_name, cu);
> +	  if (linkage_name == NULL)
> +	    linkage_name = dwarf2_string_attr (child, DW_AT_MIPS_linkage_name,
> +	                                       cu);
> +	  if (linkage_name != NULL)
>  	    {
>  	      char *actual_name
>  		= language_class_name_from_physname (cu->language_defn,
> -						     DW_STRING (attr));
> +						     linkage_name);
>  	      char *name = NULL;
>  
>  	      if (actual_name != NULL)
> @@ -19197,8 +19204,7 @@ anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
>        && die->tag != DW_TAG_structure_type && die->tag != DW_TAG_union_type)
>      return NULL;
>  
> -  attr = dwarf2_attr (die, DW_AT_name, cu);
> -  if (attr != NULL && DW_STRING (attr) != NULL)
> +  if (dwarf2_string_attr (die, DW_AT_name, cu) != NULL)
>      return NULL;
>  
>    attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
> @@ -22069,9 +22075,7 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
>        cu->language_defn = language_def (cu->language);
>      }
>  
> -  attr = dwarf2_attr (comp_unit_die, DW_AT_producer, cu);
> -  if (attr)
> -    cu->producer = DW_STRING (attr);
> +  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);
>  }
>  
>  /* Release one cached compilation unit, CU.  We unlink it from the tree
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
> new file mode 100644
> index 0000000..77f6175
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
> @@ -0,0 +1,72 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    return 0  
> +}
> +
> +standard_testfile dw2-bad-mips-linkage-name.c dw2-bad-mips-linkage-name.S
> +
> +# Set up the DWARF for the test.
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    cu {} {
> +	DW_TAG_compile_unit {
> +                {DW_AT_language @DW_LANG_C}
> +                {DW_AT_name     dw2-bad-mips-linkage-name.c}
> +                {DW_AT_comp_dir /tmp}
> +
> +        } {
> +	    declare_labels b_l
> +
> +	    b_l: DW_TAG_base_type {
> +		{DW_AT_byte_size 1 DW_FORM_sdata}
> +		{DW_AT_encoding  @DW_ATE_signed}
> +		{DW_AT_name      bool}
> +	    }
> +            DW_TAG_subprogram {
> +                {name f}
> +                {low_pc f addr}
> +                {high_pc f_end_lbl addr}
> +                {type :$b_l}
> +		{DW_AT_MIPS_linkage_name _Z1fv}
> +	    }
> +            DW_TAG_subprogram {
> +                {name g}
> +                {low_pc g addr}
> +                {high_pc g_end_lbl addr}
> +                {type :$b_l}
> +		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
> +	    }
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +# A successful run will have two PASSes.  A GDB that's lacking
> +# attribute type checking will segfault at some point.  It doesn't
> +# much matter what we test here, so long as we do something to make
> +# sure that the DWARF is read.
> +
> +gdb_test "ptype f" " = bool \\(\\)"
> +gdb_test "ptype g" " = bool \\(\\)"
> 

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

[Doug Evans]

Kevin Buettner writes:
  > On Mon, 3 Aug 2015 16:31:08 -0700
  > Doug Evans <dje@google.com> wrote:
  >
  > > 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.
  >
  > My updated patch, below, does this.
  >
  > > 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.
  >
  > I've introduced the wrapper that you recommend and have used it in
  > all places that made sense to me.  There were a few spots where using
  > it would have made things more complicated, so I left those alone.
  >
  > Here's the updated change / patch:
  >
  > dwarf2read.c: Check type of string valued attributes prior to decoding.
  >
  > This change introduces a new function, dwarf2_string_attr(), which is
  > a wrapper for dwarf2_attr().  dwarf2read.c has been updated to
  > call dwarf2_string_attr in most instances where a string-valued
  > attribute is decoded to produce a string value.  In most cases, it
  > simplifies the code; in some instances, the complexity of the code
  > remains unchanged.
  >
  > I performed this change by looking for instances where the
  > result of DW_STRING was used in an assignment.  Many of these
  > had a pattern which (roughly) looks something like this:
  >
  >   struct attribute *attr = NULL;
  >
  >   attr = dwarf2_attr (die, name, cu);
  >   if (attr != NULL && DW_STRING (attr))
  >     {
  >       const char *str;
  >       ...
  >       str = DW_STRING (attr);
  >       ... /* Use str in some fashion.  */
  >     }
  >
  > Code of this form is transformed to look like this instead:
  >
  >   const char *str;
  >
  >   str = dwarf2_string_attr (die, name, cu)
  >   if (str != NULL)
  >     {
  >        ...
  >        /* Use str in some fashion.  */
  >        ...
  >     }
  >
  > In addition to invoking dwarf2_attr() and DW_STRING(),
  > dwarf2_string_attr() checks to make sure that the attribute's
  > `form' field matches one of DW_FORM_strp, DW_FORM_string, or
  > DW_FORM_GNU_strp_alt.  If it does not match one of these forms,
  > it will return a NULL value in addition to calling complaint().
  >
  > An earlier version of this patch did this type checking for one
  > particular instance where a string attribute was being decoded.
  > The situation that I was attempting to handle in that earlier patch is
  > this:
  >
  > 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.  In this instance, GDB was attempting
  > to use an integer value as a string pointer, with predictable results.
  > (GDB would die with a segmentation fault.)
  >
  > I've added a test which reproduces the problem that I was orignally
  > wanting to fix.  It uses DW_AT_MIPS_linkage name with an associate
  > value which is a string, and again, where the value is a small
  > integer.
  >
  > My test case causes GDB to segfault in an unpatched GDB.  There
  > will be two PASSes in a patched GDB.
  >
  > Unpatched GDB:
  >
  > (gdb) ptype f
  > ERROR: Process no longer exists
  > UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
  > ERROR: Couldn't send ptype g to GDB.
  > UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
  >
  > Patched GDB:
  >
  > (gdb) ptype f
  > type = bool ()
  > (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
  > ptype g
  > type = bool ()
  > (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
  >
  > I see no regressions on an x86_64 native target.
  >
  > gdb/ChangeLog:
  >
  > 	* dwarf2read.c (dwarf2_string_attr): New function.
  > 	(lookup_dwo_unit, process_psymtab_comp_unit_reader)
  > 	(dwarf2_compute_name, dwarf2_physname, find_file_and_directory)
  > 	(read_call_site_scope, namespace_name, guess_full_die_structure_name)
  > 	(anonymous_struct_prefix, prepare_one_comp_unit): Use
  > 	dwarf2_string_attr in place of dwarf2_attr and DW_STRING.
  >
  > gdb/testsuite/ChangeLog:
  >
  > 	* gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file.
  > 	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.

Thanks for the ping!

LGTM

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

[Kevin Buettner]

On Mon, 17 Aug 2015 22:45:05 +0000
Doug Evans <dje@google.com> wrote:

> LGTM

Thanks for looking it over!

This is what I've pushed:

commit 7d45c7c3f692d93c3a33a043c347f1386681deb4
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Mon Aug 3 14:17:17 2015 -0700

    dwarf2read.c: Check type of string valued attributes prior to decoding.
    
    This change introduces a new function, dwarf2_string_attr(), which is
    a wrapper for dwarf2_attr().  dwarf2read.c has been updated to
    call dwarf2_string_attr in most instances where a string-valued
    attribute is decoded to produce a string value.  In most cases, it
    simplifies the code; in some instances, the complexity of the code
    remains unchanged.
    
    I performed this change by looking for instances where the
    result of DW_STRING was used in an assignment.  Many of these
    had a pattern which (roughly) looks something like this:
    
      struct attribute *attr = NULL;
    
      attr = dwarf2_attr (die, name, cu);
      if (attr != NULL && DW_STRING (attr))
        {
          const char *str;
          ...
          str = DW_STRING (attr);
          ... /* Use str in some fashion.  */
        }
    
    Code of this form is transformed to look like this instead:
    
      const char *str;
    
      str = dwarf2_string_attr (die, name, cu)
      if (str != NULL)
        {
           ...
           /* Use str in some fashion.  */
           ...
        }
    
    In addition to invoking dwarf2_attr() and DW_STRING(),
    dwarf2_string_attr() checks to make sure that the attribute's
    `form' field matches one of DW_FORM_strp, DW_FORM_string, or
    DW_FORM_GNU_strp_alt.  If it does not match one of these forms,
    it will return a NULL value in addition to calling complaint().
    
    An earlier version of this patch did this type checking for one
    particular instance where a string attribute was being decoded.
    The situation that I was attempting to handle in that earlier patch is
    this:
    
    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.  In this instance, GDB was attempting
    to use an integer value as a string pointer, with predictable results.
    (GDB would die with a segmentation fault.)
    
    I've added a test which reproduces the problem that I was orignally
    wanting to fix.  It uses DW_AT_MIPS_linkage name with an associate
    value which is a string, and again, where the value is a small
    integer.
    
    My test case causes GDB to segfault in an unpatched GDB.  There
    will be two PASSes in a patched GDB.
    
    Unpatched GDB:
    
    (gdb) ptype f
    ERROR: Process no longer exists
    UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
    ERROR: Couldn't send ptype g to GDB.
    UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
    
    Patched GDB:
    
    (gdb) ptype f
    type = bool ()
    (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f
    ptype g
    type = bool ()
    (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g
    
    I see no regressions on an x86_64 native target.
    
    gdb/ChangeLog:
    
    	* dwarf2read.c (dwarf2_string_attr): New function.
    	(lookup_dwo_unit, process_psymtab_comp_unit_reader)
    	(dwarf2_compute_name, dwarf2_physname, find_file_and_directory)
    	(read_call_site_scope, namespace_name, guess_full_die_structure_name)
    	(anonymous_struct_prefix, prepare_one_comp_unit): Use
    	dwarf2_string_attr in place of dwarf2_attr and DW_STRING.
    
    gdb/testsuite/ChangeLog:
    
    	* gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file.
    	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.
---
 gdb/ChangeLog                                      |   9 ++
 gdb/dwarf2read.c                                   | 134 +++++++++++----------
 gdb/testsuite/ChangeLog                            |   5 +
 .../gdb.dwarf2/dw2-bad-mips-linkage-name.c         |  41 +++++++
 .../gdb.dwarf2/dw2-bad-mips-linkage-name.exp       |  72 +++++++++++
 5 files changed, 196 insertions(+), 65 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ef8e493..b510cbc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2015-08-19  Kevin Buettner  <kevinb@redhat.com>
+
+	* dwarf2read.c (dwarf2_string_attr): New function.
+	(lookup_dwo_unit, process_psymtab_comp_unit_reader)
+	(dwarf2_compute_name, dwarf2_physname, find_file_and_directory)
+	(read_call_site_scope, namespace_name, guess_full_die_structure_name)
+	(anonymous_struct_prefix, prepare_one_comp_unit): Use
+	dwarf2_string_attr in place of dwarf2_attr and DW_STRING.
+
 2015-08-18  Doug Evans  <dje@google.com>
 	    Adrian Sendroiu <adrian.sendroiu@freescale.com>
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 445ad86..4cf0a44 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1524,6 +1524,9 @@ static struct attribute *dwarf2_attr (struct die_info *, unsigned int,
 static struct attribute *dwarf2_attr_no_follow (struct die_info *,
 						unsigned int);
 
+static const char *dwarf2_string_attr (struct die_info *die, unsigned int name,
+                                       struct dwarf2_cu *cu);
+
 static int dwarf2_flag_true_p (struct die_info *die, unsigned name,
                                struct dwarf2_cu *cu);
 
@@ -5270,13 +5273,8 @@ lookup_dwo_unit (struct dwarf2_per_cu_data *this_cu,
   gdb_assert (cu != NULL);
 
   /* Yeah, we look dwo_name up again, but it simplifies the code.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
-  gdb_assert (attr != NULL);
-  dwo_name = DW_STRING (attr);
-  comp_dir = NULL;
-  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
-  if (attr)
-    comp_dir = DW_STRING (attr);
+  dwo_name = dwarf2_string_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu);
+  comp_dir = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
 
   if (this_cu->is_debug_types)
     {
@@ -5938,7 +5936,6 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = cu->objfile;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct dwarf2_per_cu_data *per_cu = cu->per_cu;
-  struct attribute *attr;
   CORE_ADDR baseaddr;
   CORE_ADDR best_lowpc = 0, best_highpc = 0;
   struct partial_symtab *pst;
@@ -5956,18 +5953,14 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader,
   cu->list_in_scope = &file_symbols;
 
   /* Allocate a new partial symbol table structure.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_name, cu);
-  if (attr == NULL || !DW_STRING (attr))
+  filename = dwarf2_string_attr (comp_unit_die, DW_AT_name, cu);
+  if (filename == NULL)
     filename = "";
-  else
-    filename = DW_STRING (attr);
 
   pst = create_partial_symtab (per_cu, filename);
 
   /* This must be done before calling dwarf2_build_include_psymtabs.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_comp_dir, cu);
-  if (attr != NULL)
-    pst->dirname = DW_STRING (attr);
+  pst->dirname = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu);
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -8451,13 +8444,13 @@ dwarf2_compute_name (const char *name,
 	 to be able to reference.  Ideally, we want the user to be able
 	 to reference this entity using either natural or linkage name,
 	 but we haven't started looking at this enhancement yet.  */
-      struct attribute *attr;
+      const char *name;
 
-      attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
-      if (attr == NULL)
-	attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
-      if (attr && DW_STRING (attr))
-	return DW_STRING (attr);
+      name = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
+      if (name == NULL)
+	name = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
+      if (name != NULL)
+	return name;
     }
 
   /* These are the only languages we know how to qualify names in.  */
@@ -8710,18 +8703,16 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
 
   back_to = make_cleanup (null_cleanup, NULL);
 
-  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
-  if (!attr)
-    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
+  mangled = dwarf2_string_attr (die, DW_AT_linkage_name, cu);
+  if (mangled == NULL)
+    mangled = dwarf2_string_attr (die, DW_AT_MIPS_linkage_name, cu);
 
   /* DW_AT_linkage_name is missing in some cases - depend on what GDB
      has computed.  */
-  if (attr && DW_STRING (attr))
+  if (mangled != NULL)
     {
       char *demangled;
 
-      mangled = DW_STRING (attr);
-
       /* Use DMGL_RET_DROP for C++ template functions to suppress their return
 	 type.  It is easier for GDB users to search for such functions as
 	 `name(params)' than `long name(params)'.  In such case the minimal
@@ -9051,24 +9042,14 @@ static void
 find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu,
 			 const char **name, const char **comp_dir)
 {
-  struct attribute *attr;
-
-  *name = NULL;
-  *comp_dir = NULL;
-
   /* Find the filename.  Do not use dwarf2_name here, since the filename
      is not a source language identifier.  */
-  attr = dwarf2_attr (die, DW_AT_name, cu);
-  if (attr)
-    {
-      *name = DW_STRING (attr);
-    }
+  *name = dwarf2_string_attr (die, DW_AT_name, cu);
+  *comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
 
-  attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
-  if (attr)
-    *comp_dir = DW_STRING (attr);
-  else if (producer_is_gcc_lt_4_3 (cu) && *name != NULL
-	   && IS_ABSOLUTE_PATH (*name))
+  if (*comp_dir == NULL
+      && producer_is_gcc_lt_4_3 (cu) && *name != NULL
+      && IS_ABSOLUTE_PATH (*name))
     {
       char *d = ldirname (*name);
 
@@ -11710,17 +11691,17 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       gdb_assert (target_cu->objfile == objfile);
       if (die_is_declaration (target_die, target_cu))
 	{
-	  const char *target_physname = NULL;
-	  struct attribute *target_attr;
+	  const char *target_physname;
 
 	  /* Prefer the mangled name; otherwise compute the demangled one.  */
-	  target_attr = dwarf2_attr (target_die, DW_AT_linkage_name, target_cu);
-	  if (target_attr == NULL)
-	    target_attr = dwarf2_attr (target_die, DW_AT_MIPS_linkage_name,
-				       target_cu);
-	  if (target_attr != NULL && DW_STRING (target_attr) != NULL)
-	    target_physname = DW_STRING (target_attr);
-	  else
+	  target_physname = dwarf2_string_attr (target_die,
+						DW_AT_linkage_name,
+						target_cu);
+	  if (target_physname == NULL)
+	    target_physname = dwarf2_string_attr (target_die,
+						 DW_AT_MIPS_linkage_name,
+						 target_cu);
+	  if (target_physname == NULL)
 	    target_physname = dwarf2_physname (NULL, target_die, target_cu);
 	  if (target_physname == NULL)
 	    complaint (&symfile_complaints,
@@ -14191,10 +14172,8 @@ namespace_name (struct die_info *die, int *is_anonymous, struct dwarf2_cu *cu)
     {
       /* We don't use dwarf2_name here so that we can detect the absence
 	 of a name -> anonymous namespace.  */
-      struct attribute *attr = dwarf2_attr (die, DW_AT_name, cu);
+      name = dwarf2_string_attr (die, DW_AT_name, cu);
 
-      if (attr != NULL)
-	name = DW_STRING (attr);
       if (name != NULL)
 	break;
     }
@@ -17097,6 +17076,33 @@ dwarf2_attr_no_follow (struct die_info *die, unsigned int name)
   return NULL;
 }
 
+/* Return the string associated with a string-typed attribute, or NULL if it
+   is either not found or is of an incorrect type.  */
+
+static const char *
+dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *cu)
+{
+  struct attribute *attr;
+  const char *str = NULL;
+
+  attr = dwarf2_attr (die, name, cu);
+
+  if (attr != NULL)
+    {
+      if (attr->form == DW_FORM_strp || attr->form == DW_FORM_string
+	  || attr->form == DW_FORM_GNU_strp_alt)
+	str = DW_STRING (attr);
+      else
+        complaint (&symfile_complaints,
+	           _("string type expected for attribute %s for "
+		     "DIE at 0x%x in module %s"),
+		   dwarf_attr_name (name), die->offset.sect_off,
+		   objfile_name (cu->objfile));
+    }
+
+  return str;
+}
+
 /* Return non-zero iff the attribute NAME is defined for the given DIE,
    and holds a non-zero value.  This function should only be used for
    DW_FORM_flag or DW_FORM_flag_present attributes.  */
@@ -19154,16 +19160,17 @@ guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
     {
       if (child->tag == DW_TAG_subprogram)
 	{
-	  struct attribute *attr;
+	  const char *linkage_name;
 
-	  attr = dwarf2_attr (child, DW_AT_linkage_name, cu);
-	  if (attr == NULL)
-	    attr = dwarf2_attr (child, DW_AT_MIPS_linkage_name, cu);
-	  if (attr != NULL)
+	  linkage_name = dwarf2_string_attr (child, DW_AT_linkage_name, cu);
+	  if (linkage_name == NULL)
+	    linkage_name = dwarf2_string_attr (child, DW_AT_MIPS_linkage_name,
+	                                       cu);
+	  if (linkage_name != NULL)
 	    {
 	      char *actual_name
 		= language_class_name_from_physname (cu->language_defn,
-						     DW_STRING (attr));
+						     linkage_name);
 	      char *name = NULL;
 
 	      if (actual_name != NULL)
@@ -19211,8 +19218,7 @@ anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
       && die->tag != DW_TAG_structure_type && die->tag != DW_TAG_union_type)
     return NULL;
 
-  attr = dwarf2_attr (die, DW_AT_name, cu);
-  if (attr != NULL && DW_STRING (attr) != NULL)
+  if (dwarf2_string_attr (die, DW_AT_name, cu) != NULL)
     return NULL;
 
   attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
@@ -22083,9 +22089,7 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
       cu->language_defn = language_def (cu->language);
     }
 
-  attr = dwarf2_attr (comp_unit_die, DW_AT_producer, cu);
-  if (attr)
-    cu->producer = DW_STRING (attr);
+  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);
 }
 
 /* Release one cached compilation unit, CU.  We unlink it from the tree
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index eac803a..955c9f7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-19  Kevin Buettner  <kevinb@redhat.com>
+    
+	* gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file.
+	* gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file.
+
 2015-08-18  Doug Evans  <dje@google.com>
 	    Adrian Sendroiu <adrian.sendroiu@freescale.com>
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c
new file mode 100644
index 0000000..ffbebf6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.c
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Dummy main function.  */
+
+int
+main (void)
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}
+
+/* dummy f function, DWARF will describe arguments and type differently.  */
+int
+f (char *x)
+{
+  asm (".global f_end_lbl\nf_end_lbl:");
+  return 0;
+}
+
+/* dummy g function, DWARF will describe arguments and type differently.  */
+int
+g (char *x)
+{
+  asm (".global g_end_lbl\ng_end_lbl:");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
new file mode 100644
index 0000000..77f6175
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-mips-linkage-name.exp
@@ -0,0 +1,72 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile dw2-bad-mips-linkage-name.c dw2-bad-mips-linkage-name.S
+
+# Set up the DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+	DW_TAG_compile_unit {
+                {DW_AT_language @DW_LANG_C}
+                {DW_AT_name     dw2-bad-mips-linkage-name.c}
+                {DW_AT_comp_dir /tmp}
+
+        } {
+	    declare_labels b_l
+
+	    b_l: DW_TAG_base_type {
+		{DW_AT_byte_size 1 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      bool}
+	    }
+            DW_TAG_subprogram {
+                {name f}
+                {low_pc f addr}
+                {high_pc f_end_lbl addr}
+                {type :$b_l}
+		{DW_AT_MIPS_linkage_name _Z1fv}
+	    }
+            DW_TAG_subprogram {
+                {name g}
+                {low_pc g addr}
+                {high_pc g_end_lbl addr}
+                {type :$b_l}
+		{DW_AT_MIPS_linkage_name 42 DW_FORM_data1}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+# A successful run will have two PASSes.  A GDB that's lacking
+# attribute type checking will segfault at some point.  It doesn't
+# much matter what we test here, so long as we do something to make
+# sure that the DWARF is read.
+
+gdb_test "ptype f" " = bool \\(\\)"
+gdb_test "ptype g" " = bool \\(\\)"