Re: [rfa] pc bounds checking and namespaces

Re: [rfa] pc bounds checking and namespaces

[Michael Elizabeth Chastain]

Elena Zannoni writes:
> Again, a sanity check with 2.95 and -gdwarf-2 would be good.

I'm about to add carlton_dictionary-branch and drow-cplus-branch
to my test bed.  That ought to help.

Instead of finding these problems at the point of merging
the branches onto HEAD, how about if we fix the branches so that
they have zero regressions versus gdb 6.0, and then merge the
branches?

Michael C

Re: [rfa] pc bounds checking and namespaces

[Andrew Cagney]

> Elena Zannoni writes:
> 
>> Again, a sanity check with 2.95 and -gdwarf-2 would be good.
> 
> 
> I'm about to add carlton_dictionary-branch and drow-cplus-branch
> to my test bed.  That ought to help.
> 
> Instead of finding these problems at the point of merging
> the branches onto HEAD, how about if we fix the branches so that
> they have zero regressions versus gdb 6.0, and then merge the
> branches?

There's a trade off.  Being able to push a button and get back test 
results (or see them posted) for relevant cases will be very helpful, I 
think we need to be careful to avoid introducing absolute criteria such 
as zero regressions.  After all the branch is there largely to explore a 
specific idea and not for major long term development.

enjoy,
Andrew

Re: [rfa] pc bounds checking and namespaces

[David Carlton]

On Mon, 19 Jan 2004 01:06:42 -0500 (EST), mec.gnu@mindspring.com (Michael Elizabeth Chastain) said:

> I'm about to add carlton_dictionary-branch and drow-cplus-branch to
> my test bed.  That ought to help.

> Instead of finding these problems at the point of merging
> the branches onto HEAD, how about if we fix the branches so that
> they have zero regressions versus gdb 6.0, and then merge the
> branches?

In general, that's obviously a good idea.  Having said that,
carlton_dictionary-branch is attempting a delicate enough task that I
suspect that, in some circumstances, we'll have to accept regressions.
It's trying to convert GDB from using the wrong names for things to
using the right names for things, and to do this whenever possible in
the presence of inadequate debug information; doing this while
simultaneously supporting the bugs and foibles of multiple versions of
GCC is a very hard task, and one which can adversely affect GDB's
maintainability.

But, at the very least, all regressions should be analyzed, so we can
figure out what's causing them, in what circumstances they will crop
up, and how easy they would be to fix.

David Carlton
carlton@kealia.com

Re: [rfa] pc bounds checking and namespaces

[David Carlton]

On Mon, 19 Jan 2004 10:36:26 -0800, David Carlton <carlton@kealia.com> said:

> On Sun, 18 Jan 2004 19:02:59 -0500, Elena Zannoni <ezannoni@redhat.com> said:
>> Again, a sanity check with 2.95 and -gdwarf-2 would be good.

> Will do.  And I'll check it with GCC head, given the latest problem
> Michael found. :-(

>>> -    {
>>> -      /* Some compilers don't define a DW_AT_high_pc attribute for
>>> -         the compilation unit.   If the DW_AT_high_pc is missing,
>>> -         synthesize it, by scanning the DIE's below the compilation unit.  */

>> I'd rather keep this comment around, probably at the first call.

> Good idea.

>> Ok otherwise.

> Thanks.

Committed, after the aforementioned testing and comment.

David Carlton
carlton@kealia.com

Re: [rfa] pc bounds checking and namespaces

[David Carlton]

On Sun, 18 Jan 2004 19:02:59 -0500, Elena Zannoni <ezannoni@redhat.com> said:

> Again, a sanity check with 2.95 and -gdwarf-2 would be good.

Will do.  And I'll check it with GCC head, given the latest problem
Michael found. :-(

>> -    {
>> -      /* Some compilers don't define a DW_AT_high_pc attribute for
>> -         the compilation unit.   If the DW_AT_high_pc is missing,
>> -         synthesize it, by scanning the DIE's below the compilation unit.  */

> I'd rather keep this comment around, probably at the first call.

Good idea.

> Ok otherwise.

Thanks.

David Carlton
carlton@kealia.com

Re: [rfa] pc bounds checking and namespaces

[Elena Zannoni]

David Carlton writes:
 > When calculating the pc bounds for a block in dwarf2read.c, we assume
 > that all functions contained in that block are immediate children of
 > the block.  I was using a patched version of GCC where this doesn't
 > hold - the dies associated to definitions of functions in namespaces
 > are children of a DW_TAG_namespace die instead of the
 > DW_TAG_compile_unit die.  I don't know where GCC 3.4 puts the
 > relevant dies, so this might not be an issue with GCC 3.4, but it
 > might be; it seems like fixing that assumption is a good idea in any
 > case.
 > 
 > So this patch takes the code to calculate the lowpc/highpc out of
 > psymtab_to_symtab_1 and read_file_scope (where that functionality had
 > been duplicated), extracts it to a new function get_scope_pc_bounds,
 > and then modifies that new function to handle DW_TAG_namespace
 > appropriately.
 > 
 > I tried to come up with a test case for this, but I wasn't
 > successful. :-( I can say that, before this patch, I used to see a lot
 > of "pc 0xNNNN in read in psymtab, but not in symtab" messages when
 > debugging, and now I don't.
 > 
 > Tested on GCC 3.2 both with and without DW_TAG_namespace,
 > i686-pc-linux-gnu, DWARF-2; no regressions.  OK to commit?
 > 

Again, a sanity check with 2.95 and -gdwarf-2 would be good.



 > -    {
 > -      /* Some compilers don't define a DW_AT_high_pc attribute for
 > -         the compilation unit.   If the DW_AT_high_pc is missing,
 > -         synthesize it, by scanning the DIE's below the compilation unit.  */

I'd rather keep this comment around, probably at the first call.


Ok otherwise.

elena

[rfa] pc bounds checking and namespaces

[David Carlton]

When calculating the pc bounds for a block in dwarf2read.c, we assume
that all functions contained in that block are immediate children of
the block.  I was using a patched version of GCC where this doesn't
hold - the dies associated to definitions of functions in namespaces
are children of a DW_TAG_namespace die instead of the
DW_TAG_compile_unit die.  I don't know where GCC 3.4 puts the
relevant dies, so this might not be an issue with GCC 3.4, but it
might be; it seems like fixing that assumption is a good idea in any
case.

So this patch takes the code to calculate the lowpc/highpc out of
psymtab_to_symtab_1 and read_file_scope (where that functionality had
been duplicated), extracts it to a new function get_scope_pc_bounds,
and then modifies that new function to handle DW_TAG_namespace
appropriately.

I tried to come up with a test case for this, but I wasn't
successful. :-( I can say that, before this patch, I used to see a lot
of "pc 0xNNNN in read in psymtab, but not in symtab" messages when
debugging, and now I don't.

Tested on GCC 3.2 both with and without DW_TAG_namespace,
i686-pc-linux-gnu, DWARF-2; no regressions.  OK to commit?

David Carlton
carlton@kealia.com

2004-01-16  David Carlton  <carlton@kealia.com>

	* dwarf2read.c (psymtab_to_symtab_1): Calculate lowpc, highpc via
	get_scope_pc_bounds.
	(read_file_scope): Ditto.
	(get_scope_pc_bounds): New function, produced by extracting code
	from the above two functions, consolidating it, and adding support
	for DW_TAG_namespace.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.120
diff -u -p -r1.120 dwarf2read.c
--- dwarf2read.c	14 Jan 2004 16:54:41 -0000	1.120
+++ dwarf2read.c	17 Jan 2004 00:27:04 -0000
@@ -805,6 +805,10 @@ static void read_lexical_block_scope (st
 static int dwarf2_get_pc_bounds (struct die_info *,
 				 CORE_ADDR *, CORE_ADDR *, struct dwarf2_cu *);
 
+static void get_scope_pc_bounds (struct die_info *,
+				 CORE_ADDR *, CORE_ADDR *,
+				 struct dwarf2_cu *);
+
 static void dwarf2_add_field (struct field_info *, struct die_info *,
 			      struct dwarf2_cu *);
 
@@ -1882,30 +1886,8 @@ psymtab_to_symtab_1 (struct partial_symt
   /* Do line number decoding in read_file_scope () */
   process_die (dies, &cu);
 
-  if (!dwarf2_get_pc_bounds (dies, &lowpc, &highpc, &cu))
-    {
-      /* Some compilers don't define a DW_AT_high_pc attribute for
-         the compilation unit.   If the DW_AT_high_pc is missing,
-         synthesize it, by scanning the DIE's below the compilation unit.  */
-      highpc = 0;
-      if (dies->child != NULL)
-	{
-	  child_die = dies->child;
-	  while (child_die && child_die->tag)
-	    {
-	      if (child_die->tag == DW_TAG_subprogram)
-		{
-		  CORE_ADDR low, high;
+  get_scope_pc_bounds (dies, &lowpc, &highpc, &cu);
 
-		  if (dwarf2_get_pc_bounds (child_die, &low, &high, &cu))
-		    {
-		      highpc = max (highpc, high);
-		    }
-		}
-	      child_die = sibling_die (child_die);
-	    }
-	}
-    }
   symtab = end_symtab (highpc + baseaddr, objfile, SECT_OFF_TEXT (objfile));
 
   /* Set symtab language to language from DW_AT_language.
@@ -2029,27 +2011,7 @@ read_file_scope (struct die_info *die, s
   bfd *abfd = objfile->obfd;
   struct line_header *line_header = 0;
 
-  if (!dwarf2_get_pc_bounds (die, &lowpc, &highpc, cu))
-    {
-      if (die->child != NULL)
-	{
-	  child_die = die->child;
-	  while (child_die && child_die->tag)
-	    {
-	      if (child_die->tag == DW_TAG_subprogram)
-		{
-		  CORE_ADDR low, high;
-
-		  if (dwarf2_get_pc_bounds (child_die, &low, &high, cu))
-		    {
-		      lowpc = min (lowpc, low);
-		      highpc = max (highpc, high);
-		    }
-		}
-	      child_die = sibling_die (child_die);
-	    }
-	}
-    }
+  get_scope_pc_bounds (die, &lowpc, &highpc, cu);
 
   /* If we didn't find a lowpc, set it to highpc to avoid complaints
      from finish_block.  */
@@ -2438,6 +2400,68 @@ dwarf2_get_pc_bounds (struct die_info *d
   *lowpc = low;
   *highpc = high;
   return ret;
+}
+
+/* Get the low and high pc's represented by the scope DIE, and store
+   them in *LOWPC and *HIGHPC.  If the correct values can't be
+   determined, set *LOWPC to -1 and *HIGHPC to 0.  */
+
+static void
+get_scope_pc_bounds (struct die_info *die,
+		     CORE_ADDR *lowpc, CORE_ADDR *highpc,
+		     struct dwarf2_cu *cu)
+{
+  CORE_ADDR best_low = (CORE_ADDR) -1;
+  CORE_ADDR best_high = (CORE_ADDR) 0;
+  CORE_ADDR current_low, current_high;
+
+  if (dwarf2_get_pc_bounds (die, &current_low, &current_high, cu))
+    {
+      best_low = current_low;
+      best_high = current_high;
+    }
+  else
+    {
+      struct die_info *child = die->child;
+
+      while (child && child->tag)
+	{
+	  switch (child->tag) {
+	  case DW_TAG_subprogram:
+	    if (dwarf2_get_pc_bounds (child, &current_low, &current_high, cu))
+	      {
+		best_low = min (best_low, current_low);
+		best_high = max (best_high, current_high);
+	      }
+	    break;
+	  case DW_TAG_namespace:
+	    /* FIXME: carlton/2004-01-16: Should we do this for
+	       DW_TAG_class_type/DW_TAG_structure_type, too?  I think
+	       that current GCC's always emit the DIEs corresponding
+	       to definitions of methods of classes as children of a
+	       DW_TAG_compile_unit or DW_TAG_namespace (as opposed to
+	       the DIEs giving the declarations, which could be
+	       anywhere).  But I don't see any reason why the
+	       standards says that they have to be there.  */
+	    get_scope_pc_bounds (child, &current_low, &current_high, cu);
+
+	    if (current_low != ((CORE_ADDR) -1))
+	      {
+		best_low = min (best_low, current_low);
+		best_high = max (best_high, current_high);
+	      }
+	    break;
+	  default:
+	    /* Ignore. */
+	    break;
+	  }
+
+	  child = sibling_die (child);
+	}
+    }
+
+  *lowpc = best_low;
+  *highpc = best_high;
 }
 
 /* Add an aggregate field to the field list.  */