Re: archer gdb and gcc 4.1 global namespace DIE

Re: archer gdb and gcc 4.1 global namespace DIE

[Jan Kratochvil]

Hi Richard,

forwarding to Sami, author of the new C++ namespaces code.

Fedora 11 IMO should get a fix as GDB has now a regression for such simple
case for code from with F11 g++34 compiler.


Thanks for the bugreport,
Jan

On Mon, 11 May 2009 13:39:48 +0200, Richard Guenther wrote:

With the current Fedora archer based gdb I get for the G++ 4.1 compiled
testcase

int i;
int main() {}

(gdb) start
Temporary breakpoint 1, main () at t.C:3
3       int main() {}
(gdb) ptype i
type = <data variable, no debug info>

because of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28460

former gdbs appearantly tried to handle this gracefully - and I am now
thinking how to add grace to archer based gdb as well.  The simple idea
is to strip the

 <1><6e>: Abbrev Number: 2 (DW_TAG_namespace)
     DW_AT_sibling     : <86>   
     DW_AT_name        : ::
     DW_AT_decl_file   : 1
     DW_AT_decl_line   : 0

DIE during dwarf parsing (thus, fix up debug info inside gdb).  Do you
think this is possible and if so, do you have a hint where this could
be done?

Many thanks,
Richard.

[patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 62 bytes --]

Hi Sami,

sending a followup with patch I got.


Regards,
Jan

[-- Attachment #2: Type: message/rfc822, Size: 3940 bytes --]

From: Michael Matz <matz@suse.de>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Richard Guenther <rguenther@suse.de>
Subject: Support bogus global namespace emitted by g++ 4.1
Date: Wed, 13 May 2009 16:44:53 +0200 (CEST)
Message-ID: <Pine.LNX.4.64.0905131623200.29566@wotan.suse.de>

Hi,

(it's my understanding that Richard already mentioned this, but anyway)
due to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28460 some old G++ emit 
an explicit namespace declaration for the global namespace for some decls.  
This is bogus and indeed confuses gdb (not the current CVS it seems, but 
the archer branch at least).  Unfortunately we now have to deal with this 
in gdb as the bogus debug info is already out there.

So, here's a patch for that.  I saw two options:
(1) hacking determine_prefix to return "" when it was just about to
    return "::" for namespaces.
(2) not even linking that bogus namespace DIE into its children

It seems to me that option (2) is slightly more clear, so that's what I've 
chosen.  It works with the simple testcases I have using global decls.

Let me know what you think.


Ciao,
Michael.
-- 
Index: gdb-6.8.50.20090302/gdb/dwarf2read.c
===================================================================
--- gdb-6.8.50.20090302.orig/gdb/dwarf2read.c	2009-05-13 15:53:22.000000000 +0200
+++ gdb-6.8.50.20090302/gdb/dwarf2read.c	2009-05-13 16:40:44.000000000 +0200
@@ -6540,6 +6540,13 @@ load_partial_dies (bfd *abfd, gdb_byte *
 
       /* We'll save this DIE so link it in.  */
       part_die->die_parent = parent_die;
+      /* Special hack for bogus global namespace that is emitted as an
+	 explicit namespace with the name '::' in g++ 4.1, for some decls.  */
+      if (parent_die && parent_die->name && parent_die->die_parent == NULL
+	  && parent_die->name[0] == ':'
+	  && parent_die->name[1] == ':'
+	  && parent_die->name[2] == 0)
+	part_die->die_parent = NULL;
       part_die->die_sibling = NULL;
       part_die->die_child = NULL;
 

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Sami Wagiaalla]

Hi Richard,

Thanks for the bug and the patch.

This problem is due to the fact that in Archer we are using 
dwarf2_full_name instead of dwarf2_linkage_name.

I am trying your patch but it does not seem to have solved the problem 
for me. Are you testing against the Archer fedora branch ( 
archer-jankratochvil-fedora-merge ) ?

Sami

Jan Kratochvil wrote:
> Hi Sami,
> 
> sending a followup with patch I got.
> 
> 
> Regards,
> Jan
> 
> 
> ------------------------------------------------------------------------
> 
> Subject:
> Support bogus global namespace emitted by g++ 4.1
> From:
> Michael Matz <matz@suse.de>
> Date:
> Wed, 13 May 2009 16:44:53 +0200 (CEST)
> To:
> Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> To:
> Jan Kratochvil <jan.kratochvil@redhat.com>
> CC:
> Richard Guenther <rguenther@suse.de>
> 
> 
> Hi,
> 
> (it's my understanding that Richard already mentioned this, but anyway)
> due to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28460 some old G++ emit 
> an explicit namespace declaration for the global namespace for some decls.  
> This is bogus and indeed confuses gdb (not the current CVS it seems, but 
> the archer branch at least).  Unfortunately we now have to deal with this 
> in gdb as the bogus debug info is already out there.
> 
> So, here's a patch for that.  I saw two options:
> (1) hacking determine_prefix to return "" when it was just about to
>     return "::" for namespaces.
> (2) not even linking that bogus namespace DIE into its children
> 
> It seems to me that option (2) is slightly more clear, so that's what I've 
> chosen.  It works with the simple testcases I have using global decls.
> 
> Let me know what you think.
> 
> 
> Ciao,
> Michael.

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Michael Matz]

Hi,

On Wed, 13 May 2009, Sami Wagiaalla wrote:

> This problem is due to the fact that in Archer we are using 
> dwarf2_full_name instead of dwarf2_linkage_name.
> 
> I am trying your patch but it does not seem to have solved the problem 
> for me. Are you testing against the Archer fedora branch ( 
> archer-jankratochvil-fedora-merge ) ?

Meh, wrong patch, sorry.  I must have had traces of my other approach 
still in dwarf2read.c that this worked when I checked before sending.  It 
actually fixes only the partial DIEs and I'm not even sure if that's 
necessary.  It certainly does not fix the problem for reading full DIEs.  
As it's a rather generic tree reader (read_die_and_children_1 and friends) 
it seems a bit clumsy to special case bogus namespace dies there, which 
brings me back to my other option (that's the one I started and tested 
with), hacking determine_prefix, like below.  That fixes the problem.  
Really :-)


Ciao,
Michael.
Index: gdb-6.8.50.20090302/gdb/dwarf2read.c
===================================================================
--- gdb-6.8.50.20090302.orig/gdb/dwarf2read.c	2009-05-13 15:53:22.000000000 +0200
+++ gdb-6.8.50.20090302/gdb/dwarf2read.c	2009-05-13 23:48:52.000000000 +0200
@@ -8885,9 +8894,18 @@ determine_prefix (struct die_info *die,
     switch (parent->tag)
       {
       case DW_TAG_namespace:
-	parent_type = read_type_die (parent, cu);
-	/* We give a name to even anonymous namespaces.  */
-	return TYPE_TAG_NAME (parent_type);
+	{
+	  char *prefix;
+	  parent_type = read_type_die (parent, cu);
+	  /* We give a name to even anonymous namespaces.  */
+	  prefix = TYPE_TAG_NAME (parent_type);
+	  /* Special hack for bogus global namespace that is emitted as an
+	     explicit namespace with the name '::' in g++ 4.1, for
+	     some decls.  */
+	  if (prefix[0] == ':' && prefix[1] == ':' && prefix[2] == 0)
+	    return "";
+	  return prefix;
+	}
       case DW_TAG_class_type:
       case DW_TAG_interface_type:
       case DW_TAG_structure_type:

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Sami Wagiaalla]

> [...] which brings me back to my other option (that's the one I started and tested 
> with), hacking determine_prefix, like below.  That fixes the problem.  
> Really :-)
> 

Nice.. I am a fan of solution 2 as well :)
The patch works for me and introduces no regressions, at lease on x86_64 
and gcc43.

One question though. Is there a reason not to use strcmp(prefix,"::")==0 
to compare the string ?


> 
> Ciao,
> Michael.
> Index: gdb-6.8.50.20090302/gdb/dwarf2read.c
> ===================================================================
> --- gdb-6.8.50.20090302.orig/gdb/dwarf2read.c	2009-05-13 15:53:22.000000000 +0200
> +++ gdb-6.8.50.20090302/gdb/dwarf2read.c	2009-05-13 23:48:52.000000000 +0200
> @@ -8885,9 +8894,18 @@ determine_prefix (struct die_info *die,
>      switch (parent->tag)
>        {
>        case DW_TAG_namespace:
> -	parent_type = read_type_die (parent, cu);
> -	/* We give a name to even anonymous namespaces.  */
> -	return TYPE_TAG_NAME (parent_type);
> +	{
> +	  char *prefix;
> +	  parent_type = read_type_die (parent, cu);
> +	  /* We give a name to even anonymous namespaces.  */
> +	  prefix = TYPE_TAG_NAME (parent_type);
> +	  /* Special hack for bogus global namespace that is emitted as an
> +	     explicit namespace with the name '::' in g++ 4.1, for
> +	     some decls.  */
> +	  if (prefix[0] == ':' && prefix[1] == ':' && prefix[2] == 0)
> +	    return "";
> +	  return prefix;
> +	}
>        case DW_TAG_class_type:
>        case DW_TAG_interface_type:
>        case DW_TAG_structure_type:

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Michael Matz]

Hi,

On Thu, 14 May 2009, Sami Wagiaalla wrote:

> Nice.. I am a fan of solution 2 as well :)
> The patch works for me and introduces no regressions, at lease on x86_64 and
> gcc43.
> 
> One question though. Is there a reason not to use strcmp(prefix,"::")==0 to
> compare the string ?

Pure paranoia that the compiler might not optimize this to the separate 
instructions (and in that case I would feel the call overhead in this 
presumably not-too-cold code path to be too large).  I have that paranoia 
even though I'm a compiler engineer.  Or maybe because of it :)  It's 
probably better to use strcmp anyway, right ;)


Ciao,
Michael.

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Sami Wagiaalla]

> Pure paranoia that the compiler might not optimize this to the separate 
> instructions (and in that case I would feel the call overhead in this 
> presumably not-too-cold code path to be too large).  I have that paranoia 
> even though I'm a compiler engineer.  Or maybe because of it :)  

Totally understandable. I dont trust debuggers the way I used to :)

> It's probably better to use strcmp anyway, right ;)

Okay I have used strcmp and pulled the patch into my branch:

   1c9be9e4055903bca3fc335189139b67ddb3cdd8

I will will test it with gcc 44 just to be sure. Then I will cherry it 
to archer-jankratochvil-fedora-merge.

Thanks for the patch. Please let us know of bugs you find. I am 
personally interested in these expression evaluation bugs.

Sami

Re: [patch by Michael Matz] Re: archer gdb and gcc 4.1 global namespace DIE

[Richard Guenther]

On Thu, 14 May 2009, Sami Wagiaalla wrote:

> 
> > Pure paranoia that the compiler might not optimize this to the separate
> > instructions (and in that case I would feel the call overhead in this
> > presumably not-too-cold code path to be too large).  I have that paranoia
> > even though I'm a compiler engineer.  Or maybe because of it :)  
> 
> Totally understandable. I dont trust debuggers the way I used to :)
> 
> > It's probably better to use strcmp anyway, right ;)
> 
> Okay I have used strcmp and pulled the patch into my branch:
> 
>   1c9be9e4055903bca3fc335189139b67ddb3cdd8
> 
> I will will test it with gcc 44 just to be sure. Then I will cherry it to
> archer-jankratochvil-fedora-merge.
> 
> Thanks for the patch. Please let us know of bugs you find. I am personally
> interested in these expression evaluation bugs.

One more issue is that with g++ 4.1

#include <stdio.h>
const char foo_c[] = "here is foo_c";
int main()
{
  puts(foo_c);
  return 0;
}

foo_c at main is printed as

(gdb) p foo_c
$1 = '\0' <repeats 13 times>
(gdb) ptype foo_c
type = char [14]

it works if I remove the puts call.  It also works with g++ 4.2 and
newer.  I wonder if gdb is confused by g++ 4.1 using DW_AT_const_value
for the string ...

Richard.

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

[patch] Fix DW_AT_const_value using DW_FORM_string

[Jan Kratochvil]

On Fri, 15 May 2009 21:44:43 +0200, Richard Guenther wrote:
> One more issue is that with g++ 4.1
> 
> #include <stdio.h>
> const char foo_c[] = "here is foo_c";
> int main()
> {
>   puts(foo_c);
>   return 0;
> }
> 
> foo_c at main is printed as
> 
> (gdb) p foo_c
> $1 = '\0' <repeats 13 times>
> (gdb) ptype foo_c
> type = char [14]
> 
> it works if I remove the puts call.  It also works with g++ 4.2 and
> newer.  I wonder if gdb is confused by g++ 4.1 using DW_AT_const_value
> for the string ...

Yes, it looks so.  But this issue is unrelated to the Archer patches, it is
a normal FSF GDB bug.

Cross-posting and requesting commit approval.

Regression tested on x86_64-unknown-linux-gnu.


Thanks,
Jan


gdb/
2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix parsing DW_AT_const_value using DW_FORM_string.
	* dwarf2read.c (dwarf2_const_value <DW_FORM_string>): New.

gdb/testsuite/
2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-strp.exp (p a_string2, ptype a_string2): New.
	* gdb.dwarf2/dw2-strp.S (a_string2): New.

--- gdb/dwarf2read.c	1 May 2009 14:43:20 -0000	1.302
+++ gdb/dwarf2read.c	15 May 2009 20:44:44 -0000
@@ -7957,6 +7957,7 @@ dwarf2_const_value (struct attribute *at
 			      DW_ADDR (attr));
       SYMBOL_CLASS (sym) = LOC_CONST_BYTES;
       break;
+    case DW_FORM_string:
     case DW_FORM_strp:
       /* DW_STRING is already allocated on the obstack, point directly
 	 to it.  */
--- gdb/testsuite/gdb.dwarf2/dw2-strp.S	31 Mar 2009 20:21:08 -0000	1.4
+++ gdb/testsuite/gdb.dwarf2/dw2-strp.S	15 May 2009 20:44:44 -0000
@@ -52,17 +52,23 @@
 	.byte		1			/* DW_AT_byte_size */
 	.byte		6			/* DW_AT_encoding */
 
-	.uleb128	5			/* Abbrev: DW_TAG_variable */
+	.uleb128	5			/* Abbrev: DW_TAG_variable DW_FORM_strp */
 	.4byte		.Lvarname		/* DW_AT_name */
 	.4byte		.Lconst_type-.Lcu1_begin/* DW_AT_type */
 	.4byte		.Lvarcontents		/* DW_AT_const_value */
 	.byte		1			/* DW_AT_external */
 
+	.uleb128	6			/* Abbrev: DW_TAG_variable DW_FORM_string */
+	.string		"a_string2"		/* DW_AT_name */
+	.4byte		.Lconst_type-.Lcu1_begin/* DW_AT_type */
+	.string		"hello world2\n"	/* DW_AT_const_value */
+	.byte		1			/* DW_AT_external */
+
 .Lconst_type:
-	.uleb128	6			/* Abbrev: DW_TAG_const_type */
+	.uleb128	7			/* Abbrev: DW_TAG_const_type */
 	.4byte		.Larray_type-.Lcu1_begin/* DW_AT_type */
 
-	.uleb128	7			/* Abbrev: DW_TAG_variable (name "") */
+	.uleb128	8			/* Abbrev: DW_TAG_variable (name "") */
 	.4byte		.Lemptyname		/* DW_AT_name */
 
 	.byte		0			/* End of children of CU */
@@ -125,6 +131,20 @@
 	.byte		0x0			/* Terminator */
 
 	.uleb128	6			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0x0			/* no_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x1c			/* DW_AT_const_value */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	7			/* Abbrev code */
 	.uleb128	0x26			/* DW_TAG_const_type */
 	.byte		0x0			/* DW_children_no */
 	.uleb128	0x49			/* DW_AT_type */
@@ -132,7 +152,7 @@
 	.byte		0x0			/* Terminator */
 	.byte		0x0			/* Terminator */
 
-	.uleb128	7			/* Abbrev code */
+	.uleb128	8			/* Abbrev code */
 	.uleb128	0x34			/* DW_TAG_variable */
 	.byte		0x0			/* DW_children_no */
 	.uleb128	0x3			/* DW_AT_name */
--- gdb/testsuite/gdb.dwarf2/dw2-strp.exp	3 Jan 2009 05:58:04 -0000	1.2
+++ gdb/testsuite/gdb.dwarf2/dw2-strp.exp	15 May 2009 20:44:44 -0000
@@ -48,5 +48,8 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-gdb_test "p a_string" "\\\$1 = \"hello world!\\\\n\""
+gdb_test "p a_string" " = \"hello world!\\\\n\""
 gdb_test "ptype a_string" "type = char \\\[14\\\]"
+
+gdb_test "p a_string2" " = \"hello world2\\\\n\""
+gdb_test "ptype a_string2" "type = char \\\[14\\\]"

Re: [patch] Fix DW_AT_const_value using DW_FORM_string

[Joel Brobecker]

Thanks for looking at this.

> 2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix parsing DW_AT_const_value using DW_FORM_string.
> 	* dwarf2read.c (dwarf2_const_value <DW_FORM_string>): New.
> 
> gdb/testsuite/
> 2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.dwarf2/dw2-strp.exp (p a_string2, ptype a_string2): New.
> 	* gdb.dwarf2/dw2-strp.S (a_string2): New.

Looks all good to me; please go ahead and commit.

-- 
Joel

Re: [patch] Fix DW_AT_const_value using DW_FORM_string

[Jan Kratochvil]

On Wed, 20 May 2009 22:51:42 +0200, Joel Brobecker wrote:
> Thanks for looking at this.
> 
> > 2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	Fix parsing DW_AT_const_value using DW_FORM_string.
> > 	* dwarf2read.c (dwarf2_const_value <DW_FORM_string>): New.
> > 
> > gdb/testsuite/
> > 2009-05-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* gdb.dwarf2/dw2-strp.exp (p a_string2, ptype a_string2): New.
> > 	* gdb.dwarf2/dw2-strp.S (a_string2): New.
> 
> Looks all good to me; please go ahead and commit.

Checked-in.
	http://sourceware.org/ml/gdb-cvs/2009-05/msg00134.html


Thanks,
Jan