[patch] Fix PR 9675 - _static_ variables in C++ constructors

[patch] Fix PR 9675 - _static_ variables in C++ constructors

[Jan Kratochvil]

Hi,

Fix visibility of _static_ variables in C++ constructors,
as I saw it now even in GDB BZ:
http://sourceware.org/bugzilla/show_bug.cgi?id=9675
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33044
https://bugzilla.redhat.com/show_bug.cgi?id=445912

(gdb) b 25
Breakpoint 1 at 0x400637: file /tmp/pr9675.C, line 25. (2 locations)
(gdb) r
Starting program: /tmp/pr9675 
c_ = 1946
foofoo = 4196416

Breakpoint 1, A (this=0x7fffffffd3e0, a=34, b=56) at /tmp/pr9675.C:25
25	}
->

before:
(gdb) p c_
$1 = 1946
(gdb) p foofoo
No symbol "foofoo" in current context.
(gdb) 

after:
(gdb) p c_
$1 = 1946
(gdb) p foofoo
$2 = {{0, 1, 2, 3}, {1, 2, 3, 4}, {2, 3, 4, 5}, {3, 4, 5, 6}, {4, 5, 6, 7}, {5, 6, 7, 8}}
(gdb) q


Regards,
Jan


2008-12-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR gdb/9675:
	* dwarf2read.c (unsigned_int_compar): New function.
	(read_func_scope): New variable die_children, initialize it.
	Process all DIEs of DW_AT_abstract_origin not being referenced by any
	of the children of the current DIE die.

2008-12-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.cp/abstract-origin.exp, gdb.cp/abstract-origin.cc: New test.

--- gdb/dwarf2read.c	15 Nov 2008 18:49:50 -0000	1.291
+++ gdb/dwarf2read.c	21 Dec 2008 18:11:00 -0000
@@ -3032,6 +3032,15 @@ add_to_cu_func_list (const char *name, C
   cu->last_fn = thisfn;
 }
 
+static int
+unsigned_int_compar (const void *ap, const void *bp)
+{
+  unsigned int a = *(unsigned int *) ap;
+  unsigned int b = *(unsigned int *) bp;
+
+  return (a > b) - (b > a);
+}
+
 static void
 read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -3044,6 +3053,7 @@ read_func_scope (struct die_info *die, s
   char *name;
   CORE_ADDR baseaddr;
   struct block *block;
+  unsigned die_children_count;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -3080,14 +3090,89 @@ read_func_scope (struct die_info *die, s
 
   cu->list_in_scope = &local_symbols;
 
-  if (die->child != NULL)
+  child_die = die->child;
+  die_children_count = 0;
+  while (child_die && child_die->tag)
     {
+      process_die (child_die, cu);
+      child_die = sibling_die (child_die);
+      die_children_count++;
+    }
+
+  /* DW_AT_abstract_origin inherits whole DIEs (not just their attributes).
+     Inherit only the children of the DW_AT_abstract_origin DIE not being
+     already referenced by DW_AT_abstract_origin from the children of the
+     current DIE.  */
+  attr = dwarf2_attr (die, DW_AT_abstract_origin, cu);
+  if (attr)
+    {
+      /* CU offsets which were referenced by children of the current DIE.  */
+      unsigned *offsets;
+      unsigned *offsets_end, *offsetp;
+      /* Parent of DIE - referenced by DW_AT_abstract_origin.  */
+      struct die_info *origin_die;
+      /* Iterator of the ORIGIN_DIE children.  */
+      struct die_info *origin_child_die;
+      struct cleanup *cleanups;
+
+      origin_die = follow_die_ref (die, attr, &cu);
+      if (die->tag != origin_die->tag)
+	complaint (&symfile_complaints,
+		   _("DIE 0x%x and its abstract origin 0x%x have different "
+		     "tags"),
+		   die->offset, origin_die->offset);
+
+      offsets = xmalloc (sizeof (*offsets) * die_children_count);
+      cleanups = make_cleanup (xfree, offsets);
+
+      offsets_end = offsets;
       child_die = die->child;
       while (child_die && child_die->tag)
 	{
-	  process_die (child_die, cu);
+	  attr = dwarf2_attr (child_die, DW_AT_abstract_origin, cu);
+	  if (!attr)
+	    complaint (&symfile_complaints,
+		       _("Child DIE 0x%x of DIE 0x%x has missing "
+			 "DW_AT_abstract_origin"), child_die->offset,
+		       die->offset);
+	  else
+	    {
+	      struct die_info *child_origin_die;
+
+	      child_origin_die = follow_die_ref (child_die, attr, &cu);
+	      if (child_die->tag != child_origin_die->tag)
+		complaint (&symfile_complaints,
+			   _("Child DIE 0x%x and its abstract origin 0x%x have "
+			     "different tags"), child_die->offset,
+			   child_origin_die->offset);
+	      *offsets_end++ = child_origin_die->offset;
+	    }
 	  child_die = sibling_die (child_die);
 	}
+      qsort (offsets, offsets_end - offsets, sizeof (*offsets),
+	     unsigned_int_compar);
+      for (offsetp = offsets + 1; offsetp < offsets_end; offsetp++)
+	if (offsetp[-1] == *offsetp)
+	  complaint (&symfile_complaints,
+		     _("Child DIEs of DIE 0x%x duplicitly abstract-origin "
+		       "referenced DIE 0x%x"), die->offset, *offsetp);
+
+      offsetp = offsets;
+      origin_child_die = origin_die->child;
+      while (origin_child_die && origin_child_die->tag)
+      	{
+	  /* Is ORIGIN_CHILD_DIE referenced by any of the DIE children?  */
+	  while (offsetp < offsets_end && *offsetp < origin_child_die->offset)
+	    offsetp++;
+	  if (offsetp >= offsets_end || *offsetp > origin_child_die->offset)
+	    {
+	      /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
+	      process_die (origin_child_die, cu);
+	    }
+	  origin_child_die = sibling_die (origin_child_die);
+	}
+
+      do_cleanups (cleanups);
     }
 
   new = pop_context ();
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/abstract-origin.cc	21 Dec 2008 18:11:09 -0000
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 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/>.
+   */
+
+extern void f (int *);
+
+class A
+{
+public:
+  A(int i);
+};
+
+A::A(int i)
+{
+  static int *problem = new int(i);
+  f (problem);				/* break-here */
+}
+
+void f (int *)
+{
+}
+
+int
+main (void)
+{
+  A a(42);
+  return 0;
+}
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.cp/abstract-origin.exp	21 Dec 2008 18:11:09 -0000
@@ -0,0 +1,40 @@
+# Copyright 2008 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/>.
+
+set testfile abstract-origin
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] {
+    untested abstract-origin
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here"
+
+# The Bug was: No symbol "problem" in current context.
+gdb_test "p problem" " = \\(int \\*\\) 0x.*"

Re: [patch] Fix PR 9675 - _static_ variables in C++ constructors

[Tom Tromey]

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

Jan> Fix visibility of _static_ variables in C++ constructors,
Jan> as I saw it now even in GDB BZ:
Jan> http://sourceware.org/bugzilla/show_bug.cgi?id=9675
Jan> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33044
Jan> https://bugzilla.redhat.com/show_bug.cgi?id=445912

Thanks for writing this.

It took me a while to understand this (I had to go read up on the
DWARF involved), but I think I get what is going on here.

I have a couple comments.

Jan>        while (child_die && child_die->tag)
Jan>  	{
Jan> -	  process_die (child_die, cu);
Jan> +	  attr = dwarf2_attr (child_die, DW_AT_abstract_origin, cu);
Jan> +	  if (!attr)
Jan> +	    complaint (&symfile_complaints,
Jan> +		       _("Child DIE 0x%x of DIE 0x%x has missing "
Jan> +			 "DW_AT_abstract_origin"), child_die->offset,
Jan> +		       die->offset);

I suspect this complaint is incorrect.  DWARF 3.3.8.3 exception #3
says that a DIE in the concrete tree does not necessarily need to
refer to a DIE in the origin tree.  If I understand the patch
correctly, this complaint will be issued in exactly this case.  Is
that accurate?

Jan> +	if (offsetp[-1] == *offsetp)
Jan> +	  complaint (&symfile_complaints,
Jan> +		     _("Child DIEs of DIE 0x%x duplicitly abstract-origin "
Jan> +		       "referenced DIE 0x%x"), die->offset, *offsetp);

This text reads strangely.  How about:

 Multiple children of DIE 0x%s refer to DIE 0x%x as their abstract origin


Other than these things this patch looks ok to me.

Tom

Re: [patch] Fix PR 9675 - _static_ variables in C++ constructors

[Jan Kratochvil]

On Fri, 24 Apr 2009 00:40:08 +0200, Tom Tromey wrote:
> Jan>        while (child_die && child_die->tag)
> Jan>  	{
> Jan> -	  process_die (child_die, cu);
> Jan> +	  attr = dwarf2_attr (child_die, DW_AT_abstract_origin, cu);
> Jan> +	  if (!attr)
> Jan> +	    complaint (&symfile_complaints,
> Jan> +		       _("Child DIE 0x%x of DIE 0x%x has missing "
> Jan> +			 "DW_AT_abstract_origin"), child_die->offset,
> Jan> +		       die->offset);
> 
> I suspect this complaint is incorrect.  DWARF 3.3.8.3 exception #3
                                          DWARF 3.3.8.2 exception #3
> says that a DIE in the concrete tree does not necessarily need to
> refer to a DIE in the origin tree.  If I understand the patch
> correctly, this complaint will be issued in exactly this case.

You are right, fixed, new comment there.


> Other than these things this patch looks ok to me.

Moved it to a new function, it was moved this way by Sami Wagiaalla.


Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2009-04/msg00191.html

gdb/
2009-04-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR gdb/9675:
	* dwarf2read.c (unsigned_int_compar, inherit_abstract_dies): New.
	(read_func_scope): Call inherit_abstract_dies.

gdb/testsuite/
2009-04-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.cp/abstract-origin.exp, gdb.cp/abstract-origin.cc: New test.

--- src/gdb/dwarf2read.c	2009/04/02 21:01:40	1.300
+++ src/gdb/dwarf2read.c	2009/04/27 08:36:16	1.301
@@ -3089,6 +3089,103 @@
   cu->last_fn = thisfn;
 }
 
+/* qsort helper for inherit_abstract_dies.  */
+
+static int
+unsigned_int_compar (const void *ap, const void *bp)
+{
+  unsigned int a = *(unsigned int *) ap;
+  unsigned int b = *(unsigned int *) bp;
+
+  return (a > b) - (b > a);
+}
+
+/* DW_AT_abstract_origin inherits whole DIEs (not just their attributes).
+   Inherit only the children of the DW_AT_abstract_origin DIE not being already
+   referenced by DW_AT_abstract_origin from the children of the current DIE.  */
+
+static void
+inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
+{
+  struct die_info *child_die;
+  unsigned die_children_count;
+  /* CU offsets which were referenced by children of the current DIE.  */
+  unsigned *offsets;
+  unsigned *offsets_end, *offsetp;
+  /* Parent of DIE - referenced by DW_AT_abstract_origin.  */
+  struct die_info *origin_die;
+  /* Iterator of the ORIGIN_DIE children.  */
+  struct die_info *origin_child_die;
+  struct cleanup *cleanups;
+  struct attribute *attr;
+
+  attr = dwarf2_attr (die, DW_AT_abstract_origin, cu);
+  if (!attr)
+    return;
+
+  origin_die = follow_die_ref (die, attr, &cu);
+  if (die->tag != origin_die->tag)
+    complaint (&symfile_complaints,
+	       _("DIE 0x%x and its abstract origin 0x%x have different tags"),
+	       die->offset, origin_die->offset);
+
+  child_die = die->child;
+  die_children_count = 0;
+  while (child_die && child_die->tag)
+    {
+      child_die = sibling_die (child_die);
+      die_children_count++;
+    }
+  offsets = xmalloc (sizeof (*offsets) * die_children_count);
+  cleanups = make_cleanup (xfree, offsets);
+
+  offsets_end = offsets;
+  child_die = die->child;
+  while (child_die && child_die->tag)
+    {
+      attr = dwarf2_attr (child_die, DW_AT_abstract_origin, cu);
+      /* According to DWARF3 3.3.8.2 #3 new entries without their abstract
+	 counterpart may exist.  */
+      if (attr)
+	{
+	  struct die_info *child_origin_die;
+
+	  child_origin_die = follow_die_ref (child_die, attr, &cu);
+	  if (child_die->tag != child_origin_die->tag)
+	    complaint (&symfile_complaints,
+		       _("Child DIE 0x%x and its abstract origin 0x%x have "
+			 "different tags"), child_die->offset,
+		       child_origin_die->offset);
+	  *offsets_end++ = child_origin_die->offset;
+	}
+      child_die = sibling_die (child_die);
+    }
+  qsort (offsets, offsets_end - offsets, sizeof (*offsets),
+	 unsigned_int_compar);
+  for (offsetp = offsets + 1; offsetp < offsets_end; offsetp++)
+    if (offsetp[-1] == *offsetp)
+      complaint (&symfile_complaints, _("Multiple children of DIE 0x%x refer "
+					"to DIE 0x%x as their abstract origin"),
+		 die->offset, *offsetp);
+
+  offsetp = offsets;
+  origin_child_die = origin_die->child;
+  while (origin_child_die && origin_child_die->tag)
+    {
+      /* Is ORIGIN_CHILD_DIE referenced by any of the DIE children?  */
+      while (offsetp < offsets_end && *offsetp < origin_child_die->offset)
+	offsetp++;
+      if (offsetp >= offsets_end || *offsetp > origin_child_die->offset)
+	{
+	  /* Found that ORIGIN_CHILD_DIE is really not referenced.  */
+	  process_die (origin_child_die, cu);
+	}
+      origin_child_die = sibling_die (origin_child_die);
+    }
+
+  do_cleanups (cleanups);
+}
+
 static void
 read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -3147,6 +3244,8 @@
 	}
     }
 
+  inherit_abstract_dies (die, cu);
+
   new = pop_context ();
   /* Make a block for the local symbols within.  */
   block = finish_block (new->name, &local_symbols, new->old_blocks,
--- src/gdb/testsuite/gdb.cp/abstract-origin.cc
+++ src/gdb/testsuite/gdb.cp/abstract-origin.cc	2009-04-27 08:37:32.229120000 +0000
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009 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/>.
+   */
+
+extern void f (int *);
+
+class A
+{
+public:
+  A(int i);
+};
+
+A::A(int i)
+{
+  static int *problem = new int(i);
+  f (problem);				/* break-here */
+}
+
+void f (int *)
+{
+}
+
+int
+main (void)
+{
+  A a(42);
+  return 0;
+}
--- src/gdb/testsuite/gdb.cp/abstract-origin.exp
+++ src/gdb/testsuite/gdb.cp/abstract-origin.exp	2009-04-27 08:37:33.315275000 +0000
@@ -0,0 +1,31 @@
+# Copyright 2009 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/>.
+
+set testfile abstract-origin
+set srcfile ${testfile}.cc
+if [prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}] {
+    return -1
+}
+
+if ![runto_main] {
+    untested abstract-origin
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here"
+
+# The Bug was: No symbol "problem" in current context.
+gdb_test "p problem" " = \\(int \\*\\) 0x.*"

Re: [patch] Fix PR 9675 - _static_ variables in C++ constructors

[Tom Tromey]

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

Jan> 2009-04-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	PR gdb/9675:
Jan> 	* dwarf2read.c (unsigned_int_compar, inherit_abstract_dies): New.
Jan> 	(read_func_scope): Call inherit_abstract_dies.

This is ok.  Thanks.

Tom