* [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
@ 2015-08-03 22:47 Kevin Buettner
2015-08-03 23:31 ` Doug Evans
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Buettner @ 2015-08-03 22:47 UTC (permalink / raw)
To: gdb-patches
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\\)"
+
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
2015-08-03 22:47 [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding Kevin Buettner
@ 2015-08-03 23:31 ` Doug Evans
2015-08-04 0:02 ` Kevin Buettner
2015-08-07 2:03 ` Kevin Buettner
0 siblings, 2 replies; 7+ messages in thread
From: Doug Evans @ 2015-08-03 23:31 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
2015-08-03 23:31 ` Doug Evans
@ 2015-08-04 0:02 ` Kevin Buettner
2015-08-07 2:03 ` Kevin Buettner
1 sibling, 0 replies; 7+ messages in thread
From: Kevin Buettner @ 2015-08-04 0:02 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
2015-08-03 23:31 ` Doug Evans
2015-08-04 0:02 ` Kevin Buettner
@ 2015-08-07 2:03 ` Kevin Buettner
2015-08-17 20:38 ` Kevin Buettner
1 sibling, 1 reply; 7+ messages in thread
From: Kevin Buettner @ 2015-08-07 2:03 UTC (permalink / raw)
To: gdb-patches
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 \\(\\)"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
2015-08-07 2:03 ` Kevin Buettner
@ 2015-08-17 20:38 ` Kevin Buettner
0 siblings, 0 replies; 7+ messages in thread
From: Kevin Buettner @ 2015-08-17 20:38 UTC (permalink / raw)
To: gdb-patches
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 \\(\\)"
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
2015-08-17 22:45 Doug Evans
@ 2015-08-19 18:54 ` Kevin Buettner
0 siblings, 0 replies; 7+ messages in thread
From: Kevin Buettner @ 2015-08-19 18:54 UTC (permalink / raw)
To: gdb-patches
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 \\(\\)"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding
@ 2015-08-17 22:45 Doug Evans
2015-08-19 18:54 ` Kevin Buettner
0 siblings, 1 reply; 7+ messages in thread
From: Doug Evans @ 2015-08-17 22:45 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-19 18:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-03 22:47 [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding Kevin Buettner
2015-08-03 23:31 ` Doug Evans
2015-08-04 0:02 ` Kevin Buettner
2015-08-07 2:03 ` Kevin Buettner
2015-08-17 20:38 ` Kevin Buettner
2015-08-17 22:45 Doug Evans
2015-08-19 18:54 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).