[expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[Jan Kratochvil]

commit 281278326412f9d6a3fabb8adc1d419fd7ddc7d7 Mon Sep 17 00:00:00 2001
From: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Sun, 15 Mar 2009 18:01:08 +0100
Subject: [PATCH] Fix crash on C++ DW_AT_name "".

Testcase is:
12fab790f9fd169f857fbffcf66241e913d1f473

Bugreport:
https://bugzilla.redhat.com/show_bug.cgi?id=490319

Reproducer besides the testcase is:
xulrunner-debuginfo-1.9.1-0.10.beta3.fc11.x86_64
gdb /usr/lib/debug/usr/lib64/xulrunner-1.9.1/libxul.so.debug
<crash>

#0  0x0000000000458730 in cp_already_canonical (string=0x0) at cp-support.c:83
#1  0x000000000045881e in cp_canonicalize_string (string=0x0) at cp-support.c:115
#2  0x0000000000573f21 in dwarf2_canonicalize_name (name=0x0, cu=0x7fffffffca70, obstack=0x7fffffffcb48) at dwarf2read.c:8931
#3  0x000000000056f9a9 in read_partial_die (part_die=0x7d5cea0, abbrev=0x7ee3d80, abbrev_len=1, abfd=0x235a520, info_ptr=0x7fffeb41b385 "\5O\4\362\17/", cu=0x7fffffffca70) at dwarf2read.c:6652

	* gdb/dwarf2read.c (dwarf2_canonicalize_name): Fix crash on NULL name.
---
 gdb/dwarf2read.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 489e9ea..a0228a0 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8497,7 +8497,7 @@ static char *
 dwarf2_canonicalize_name (char *name, struct dwarf2_cu *cu,
 			  struct obstack *obstack)
 {
-  if (cu->language == language_cplus)
+  if (name && cu->language == language_cplus)
     {
       char *canon_name = cp_canonicalize_string (name);
 
-- 
1.6.0.6

commit 12fab790f9fd169f857fbffcf66241e913d1f473 Mon Sep 17 00:00:00 2001
From: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Sun, 15 Mar 2009 17:54:08 +0100
Subject: [PATCH] New testcase: Exploit a bug in parsing DW_AT_name "".

xulrunner-debuginfo-1.9.1-0.10.beta3.fc11.x86_64 file
/usr/lib/debug/usr/lib64/xulrunner-1.9.1/libxul.so.debug
on DIE offset 0x2070330 there is empty DW_AT_name:
 <2><2070330>: Abbrev Number: 24 (DW_TAG_variable)
    <2070331>   DW_AT_name        : (indirect string, offset: 0x1daea5):
    <2070335>   DW_AT_decl_file   : 5
    <2070336>   DW_AT_decl_line   : 1103
    <2070338>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0x2f0ff2): _ZN9nsGkAtoms7genericE
    <207033c>   DW_AT_type        : <0x206bc9f>
    <2070340>   DW_AT_external    : 1
    <2070341>   DW_AT_declaration : 1

which crashes [expr-cumulative] due to its imported patch:
b2f7b66e72ecc20a08dbec1e867b16adcf7b4e97

Fedora Rawhide bugreport: https://bugzilla.redhat.com/show_bug.cgi?id=490319

This is only a testcase, the bug does not exist on FSF GDB.

gdb/testsuite/
	* dw2-strp.S (DW_AT_language): Change to C++.
	(DW_TAG_variable (name ""), Abbrev code 7, .Lemptyname): New.
---
 gdb/testsuite/gdb.dwarf2/dw2-strp.S |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-strp.S b/gdb/testsuite/gdb.dwarf2/dw2-strp.S
index a1602a5..293cc1b 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-strp.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-strp.S
@@ -28,7 +28,8 @@
 	/* CU die */
 	.uleb128	1			/* Abbrev: DW_TAG_compile_unit */
 	.4byte		.Lproducer		/* DW_AT_producer */
-	.byte		1			/* DW_AT_language (C) */
+	/* Use C++ to exploit a bug in parsing DW_AT_name "".  */
+	.byte		4			/* DW_AT_language (C++) -  */
 
 .Larray_type:
 	.uleb128	2			/* Abbrev: DW_TAG_array_type */
@@ -60,6 +61,10 @@
 .Lconst_type:
 	.uleb128	6			/* Abbrev: DW_TAG_const_type */
 	.4byte		.Larray_type-.Lcu1_begin/* DW_AT_type */
+
+	.uleb128	7			/* Abbrev: DW_TAG_variable (name "") */
+	.4byte		.Lemptyname		/* DW_AT_name */
+
 	.byte		0			/* End of children of CU */
 .Lcu1_end:
 
@@ -126,6 +131,15 @@
 	.uleb128	0x13			/* DW_FORM_ref4 */
 	.byte		0x0			/* Terminator */
 	.byte		0x0			/* Terminator */
+
+	.uleb128	7			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0x0			/* DW_children_no */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0xe			/* DW_FORM_strp */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
 	.byte		0x0			/* Terminator */
 
 /* String table */
@@ -140,3 +154,5 @@
 	.string		"a_string"
 .Lvarcontents:
 	.string		"hello world!\n"
+.Lemptyname:
+	.string		""
-- 
1.6.0.6

Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> #0  0x0000000000458730 in cp_already_canonical (string=0x0) at cp-support.c:83
Jan> #1  0x000000000045881e in cp_canonicalize_string (string=0x0) at cp-support.c:115
Jan> #2  0x0000000000573f21 in dwarf2_canonicalize_name (name=0x0, cu=0x7fffffffca70, obstack=0x7fffffffcb48) at dwarf2read.c:8931

Jan> 	* gdb/dwarf2read.c (dwarf2_canonicalize_name): Fix crash on NULL name.

I suspect this should go on archer-keiths-dwarf-names-branch.
That's where we imported Daniel's canonicalization patch.
(Though I see on the wiki that this is listed as a dead branch... is
that accurate?)

Tom

Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[Jan Kratochvil]

On Tue, 17 Mar 2009 22:20:44 +0100, Tom Tromey wrote:
> Jan> 	* gdb/dwarf2read.c (dwarf2_canonicalize_name): Fix crash on NULL name.
> 
> I suspect this should go on archer-keiths-dwarf-names-branch.

As I am not sure if the NULL check should not be rather in the caller (and not
the callee where did I put it myself) I left this decision on the branch
owner.


> That's where we imported Daniel's canonicalization patch.

It is now contained at least in [expr] (how I shortened called
[archer-keiths-expr-cumulative], I hope it is understandable this way).


Regards,
Jan

Dead branches deletion? [Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".]

[Jan Kratochvil]

On Tue, 17 Mar 2009 22:20:44 +0100, Tom Tromey wrote:
> (Though I see on the wiki that this is listed as a dead branch... is
> that accurate?)

Dodji suggested one can delete the remote GIT branches by:

	git push origin :branch-to-delete

(it worked for me when I was fixing up `archer-jankratochvil-fedora-merge')

Is there any reason to keep the `Dead Branches' wiki section and not just
delete those branches to make the `git branch -r | grep archer' listing
easier?  Although CVS can also delete the branches while is is usual to keep
them in the repositories.

Or maybe just delete completely empty branches (like my
archer-jankratochvil-fedora) and keep the real but obsolete ones for
historical reasons?


Thanks,
Jan

Re: Dead branches deletion? [Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".]

[Phil Muldoon]

Jan Kratochvil wrote:
> On Tue, 17 Mar 2009 22:20:44 +0100, Tom Tromey wrote:
>   
>> (Though I see on the wiki that this is listed as a dead branch... is
>> that accurate?)
>>     
>
> Is there any reason to keep the `Dead Branches' wiki section and not just
> delete those branches to make the `git branch -r | grep archer' listing
> easier?  Although CVS can also delete the branches while is is usual to keep
> them in the repositories.
>   

I'd rather keep mine around. Though from a purely personal point of 
view, I don't mind/care what people do to their own dead branches. But I 
like to keep history around. I learn from it when I forget things ;)

Regards

Phil

Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[Keith Seitz]

Jan Kratochvil wrote:
> On Tue, 17 Mar 2009 22:20:44 +0100, Tom Tromey wrote:
>> Jan> 	* gdb/dwarf2read.c (dwarf2_canonicalize_name): Fix crash on NULL name.
>>
>> I suspect this should go on archer-keiths-dwarf-names-branch.
> 
> As I am not sure if the NULL check should not be rather in the caller (and not
> the callee where did I put it myself) I left this decision on the branch
> owner.

I was curious about how it was possible to even tickle this bug, but I 
don't think the proposed patch is neither unreasonable nor incorrect. 
Just defensive. [Which is good IMO.]

>> That's where we imported Daniel's canonicalization patch.
> 
> It is now contained at least in [expr] (how I shortened called
> [archer-keiths-expr-cumulative], I hope it is understandable this way).

It was my intent to deprecate all the previous expr-related branches, 
for better or worse. There's just too much interaction between them. 
But, yeah, you are right: My original import is on that branch. 
Additional fixes are already in expr-cumulative, though.

I can commit this to expr-cumulative, if you want me to...

Keith

Re: [expr]-fix [jk-expr-committed] Fix crash on C++ DW_AT_name "".

[Jan Kratochvil]

On Tue, 17 Mar 2009 22:59:51 +0100, Keith Seitz wrote:
> I was curious about how it was possible to even tickle this bug,

I hope it can be seen from the new testcase.


> I can commit this to expr-cumulative, if you want me to...

It would be nice, when you agree with it.



Regards,
Jan