Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfa] add 'parent' field to struct die_info
@ 2003-09-30 16:29 David Carlton
  2003-09-30 22:13 ` Jim Blandy
  0 siblings, 1 reply; 8+ messages in thread
From: David Carlton @ 2003-09-30 16:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Elena Zannoni, Jim Blandy

This patch adds a 'parent' field to struct die_info.  There's no way
to get that information out of our current data structures, and it is
necessary for nested type support: die A might reference a type die B
via DW_AT_specification, and to correctly deal with the possibility
that B might be a nested type, we'll have to know B's parent (if any).
(See the thread starting in
<http://sources.redhat.com/ml/gdb/2003-05/msg00173.html>.)  I was
going to do this as part of a big nested type patch, but I figured
Elena was going to ask me to commit this part separately anyways. :-)
(But I would appreciate it if you could review it quickly: it's
simple.)

While I was at it, I replaced the existing 'has_children' and 'next'
fields by 'child' and 'sibling' fields: it seems clearer to me that
way.  The result of this is that the function 'sibling_die' isn't
really pulling its weight any more; I left it in there for now,
though.

Tested on i686-pc-linux-gnu, GCC 3.2, DWARF 2.  OK to commit?

David Carlton
carlton@kealia.com

2003-09-30  David Carlton  <carlton@kealia.com>

	* dwarf2read.c (struct die_info): Add 'parent' field; replace
	'has_children' and 'next' by 'child' and 'sibling'.
	(read_comp_unit): Rework algorithm, breaking body into
	read_die_and_children and read_die_and_siblings.
	(read_die_and_children, read_die_and_siblings): New.
	(read_full_die): Add 'has_children' argument; set it instead of
	the die's 'has_children' field.  Minor formatting cleanup.
	(free_die_list): Use die->child and die->sibling instead of
	die->next.
	(dump_die_list): Ditto.
	(sibling_die): Use die->sibling.
	(psymtab_to_symtab_1): Use die's 'child' field in place of its
	'has_children' and 'next' fields.
	(process_die, read_file_scope, read_func_scope) 
	(read_lexical_block_scope, read_structure_scope) 
	(read_enumeration, read_array_type, read_common_block) 
	(read_namespace, read_subroutine_type, dump_die): Ditto.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.108
diff -u -p -r1.108 dwarf2read.c
--- dwarf2read.c	23 Sep 2003 16:25:13 -0000	1.108
+++ dwarf2read.c	30 Sep 2003 15:58:30 -0000
@@ -315,13 +315,14 @@ struct attr_abbrev
 struct die_info
   {
     enum dwarf_tag tag;		/* Tag indicating type of die */
-    unsigned short has_children;	/* Does the die have children */
     unsigned int abbrev;	/* Abbrev number */
     unsigned int offset;	/* Offset in .debug_info section */
     unsigned int num_attrs;	/* Number of attributes */
     struct attribute *attrs;	/* An array of attributes */
     struct die_info *next_ref;	/* Next die in ref hash table */
-    struct die_info *next;	/* Next die in linked list */
+    struct die_info *child;	/* Its first child, if any.  */
+    struct die_info *sibling;	/* Its next sibling, if any.  */
+    struct die_info *parent;	/* Its parent, if any.  */
     struct type *type;		/* Cached type information */
   };
 
@@ -716,7 +717,7 @@ static char *read_partial_die (struct pa
 			       const struct comp_unit_head *);
 
 static char *read_full_die (struct die_info **, bfd *, char *,
-			    const struct comp_unit_head *);
+			    const struct comp_unit_head *, int *);
 
 static char *read_attribute (struct attribute *, struct attr_abbrev *,
 			     bfd *, char *, const struct comp_unit_head *);
@@ -873,6 +874,16 @@ static void read_subroutine_type (struct
 static struct die_info *read_comp_unit (char *, bfd *,
                                         const struct comp_unit_head *);
 
+static struct die_info *read_die_and_children (char *info_ptr, bfd *abfd,
+					       const struct comp_unit_head *,
+					       char **new_info_ptr,
+					       struct die_info *parent);
+
+static struct die_info *read_die_and_siblings (char *info_ptr, bfd *abfd,
+					       const struct comp_unit_head *,
+					       char **new_info_ptr,
+					       struct die_info *parent);
+
 static void free_die_list (struct die_info *);
 
 static struct cleanup *make_cleanup_free_die_list (struct die_info *);
@@ -1836,9 +1847,9 @@ psymtab_to_symtab_1 (struct partial_symt
          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->has_children)
+      if (dies->child != NULL)
 	{
-	  child_die = dies->next;
+	  child_die = dies->child;
 	  while (child_die && child_die->tag)
 	    {
 	      if (child_die->tag == DW_TAG_subprogram)
@@ -1958,7 +1969,7 @@ process_die (struct die_info *die, struc
 	  processing_has_namespace_info = 1;
 	  processing_current_namespace = "";
 	}
-      gdb_assert (!die->has_children);
+      gdb_assert (die->child == NULL);
       break;
     default:
       new_symbol (die, NULL, objfile, cu_header);
@@ -1988,9 +1999,9 @@ read_file_scope (struct die_info *die, s
 
   if (!dwarf2_get_pc_bounds (die, &lowpc, &highpc, objfile, cu_header))
     {
-      if (die->has_children)
+      if (die->child != NULL)
 	{
-	  child_die = die->next;
+	  child_die = die->child;
 	  while (child_die && child_die->tag)
 	    {
 	      if (child_die->tag == DW_TAG_subprogram)
@@ -2069,9 +2080,9 @@ read_file_scope (struct die_info *die, s
   initialize_cu_func_list ();
 
   /* Process all dies in compilation unit.  */
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  process_die (child_die, objfile, cu_header);
@@ -2209,9 +2220,9 @@ read_func_scope (struct die_info *die, s
 
   list_in_scope = &local_symbols;
 
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  process_die (child_die, objfile, cu_header);
@@ -2259,9 +2270,9 @@ read_lexical_block_scope (struct die_inf
   highpc += baseaddr;
 
   push_context (0, lowpc);
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  process_die (child_die, objfile, cu_header);
@@ -2936,7 +2947,7 @@ read_structure_scope (struct die_info *d
      type within the structure itself. */
   die->type = type;
 
-  if (die->has_children && ! die_is_declaration (die))
+  if (die->child != NULL && ! die_is_declaration (die))
     {
       struct field_info fi;
       struct die_info *child_die;
@@ -2944,7 +2955,7 @@ read_structure_scope (struct die_info *d
 
       memset (&fi, 0, sizeof (struct field_info));
 
-      child_die = die->next;
+      child_die = die->child;
 
       while (child_die && child_die->tag)
 	{
@@ -3082,9 +3093,9 @@ read_enumeration (struct die_info *die, 
 
   num_fields = 0;
   fields = NULL;
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  if (child_die->tag != DW_TAG_enumerator)
@@ -3163,7 +3174,7 @@ read_array_type (struct die_info *die, s
 
   /* Irix 6.2 native cc creates array types without children for
      arrays with unspecified length.  */
-  if (die->has_children == 0)
+  if (die->child == NULL)
     {
       index_type = dwarf2_fundamental_type (objfile, FT_INTEGER);
       range_type = create_range_type (NULL, index_type, 0, -1);
@@ -3172,7 +3183,7 @@ read_array_type (struct die_info *die, s
     }
 
   back_to = make_cleanup (null_cleanup, NULL);
-  child_die = die->next;
+  child_die = die->child;
   while (child_die && child_die->tag)
     {
       if (child_die->tag == DW_TAG_subrange_type)
@@ -3324,9 +3335,9 @@ read_common_block (struct die_info *die,
 						 "common block member");
         }
     }
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  sym = new_symbol (child_die, NULL, objfile, cu_header);
@@ -3412,9 +3423,9 @@ read_namespace (struct die_info *die, st
 				strlen (processing_current_namespace));
     }
 
-  if (die->has_children)
+  if (die->child != NULL)
     {
-      struct die_info *child_die = die->next;
+      struct die_info *child_die = die->child;
       
       while (child_die && child_die->tag)
 	{
@@ -3649,7 +3660,7 @@ read_subroutine_type (struct die_info *d
       || cu_language == language_cplus)
     TYPE_FLAGS (ftype) |= TYPE_FLAG_PROTOTYPED;
 
-  if (die->has_children)
+  if (die->child != NULL)
     {
       struct die_info *child_die;
       int nparams = 0;
@@ -3658,7 +3669,7 @@ read_subroutine_type (struct die_info *d
       /* Count the number of parameters.
          FIXME: GDB currently ignores vararg functions, but knows about
          vararg member functions.  */
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  if (child_die->tag == DW_TAG_formal_parameter)
@@ -3673,7 +3684,7 @@ read_subroutine_type (struct die_info *d
       TYPE_FIELDS (ftype) = (struct field *)
 	TYPE_ALLOC (ftype, nparams * sizeof (struct field));
 
-      child_die = die->next;
+      child_die = die->child;
       while (child_die && child_die->tag)
 	{
 	  if (child_die->tag == DW_TAG_formal_parameter)
@@ -3808,46 +3819,89 @@ static struct die_info *
 read_comp_unit (char *info_ptr, bfd *abfd,
 		const struct comp_unit_head *cu_header)
 {
-  struct die_info *first_die, *last_die, *die;
-  char *cur_ptr;
-  int nesting_level;
-
   /* Reset die reference table; we are
      building new ones now.  */
   dwarf2_empty_hash_tables ();
 
+  return read_die_and_children (info_ptr, abfd, cu_header, &info_ptr, NULL);
+}
+
+/* Read a single die and all its descendents.  Set the die's sibling
+   field to NULL; set other fields in the die correctly, and set all
+   of the descendents' fields correctly.  Set *NEW_INFO_PTR to the
+   location of the info_ptr after reading all of those dies.  PARENT
+   is the parent of the die in question.  */
+
+static struct die_info *
+read_die_and_children (char *info_ptr, bfd *abfd,
+		       const struct comp_unit_head *cu_header,
+		       char **new_info_ptr,
+		       struct die_info *parent)
+{
+  struct die_info *die;
+  char *cur_ptr;
+  int has_children;
+
+  cur_ptr = read_full_die (&die, abfd, info_ptr, cu_header, &has_children);
+  store_in_ref_table (die->offset, die);
+
+  if (has_children)
+    {
+      die->child = read_die_and_siblings (cur_ptr, abfd, cu_header,
+					  new_info_ptr, die);
+    }
+  else
+    {
+      die->child = NULL;
+      *new_info_ptr = cur_ptr;
+    }
+
+  die->sibling = NULL;
+  die->parent = parent;
+  return die;
+}
+
+/* Read a die, all of its descendents, and all of its siblings; set
+   all of the fields of all of the dies correctly.  Arguments are as
+   in read_die_and_children.  */
+
+static struct die_info *
+read_die_and_siblings (char *info_ptr, bfd *abfd,
+		       const struct comp_unit_head *cu_header,
+		       char **new_info_ptr,
+		       struct die_info *parent)
+{
+  struct die_info *first_die, *last_sibling;
+  char *cur_ptr;
+
   cur_ptr = info_ptr;
-  nesting_level = 0;
-  first_die = last_die = NULL;
-  do
+  first_die = last_sibling = NULL;
+
+  while (1)
     {
-      cur_ptr = read_full_die (&die, abfd, cur_ptr, cu_header);
-      if (die->has_children)
+      struct die_info *die
+	= read_die_and_children (cur_ptr, abfd, cu_header,
+				 &cur_ptr, parent);
+
+      if (!first_die)
 	{
-	  nesting_level++;
+	  first_die = die;
 	}
-      if (die->tag == 0)
+      else
 	{
-	  nesting_level--;
+	  last_sibling->sibling = die;
 	}
 
-      die->next = NULL;
-
-      /* Enter die in reference hash table */
-      store_in_ref_table (die->offset, die);
-
-      if (!first_die)
+      if (die->tag == 0)
 	{
-	  first_die = last_die = die;
+	  *new_info_ptr = cur_ptr;
+	  return first_die;
 	}
       else
 	{
-	  last_die->next = die;
-	  last_die = die;
+	  last_sibling = die;
 	}
     }
-  while (nesting_level > 0);
-  return first_die;
 }
 
 /* Free a linked list of dies.  */
@@ -3860,7 +3914,9 @@ free_die_list (struct die_info *dies)
   die = dies;
   while (die)
     {
-      next = die->next;
+      if (die->child != NULL)
+	free_die_list (die->child);
+      next = die->sibling;
       xfree (die->attrs);
       xfree (die);
       die = next;
@@ -4173,12 +4229,14 @@ read_partial_die (struct partial_die_inf
   return info_ptr;
 }
 
-/* Read the die from the .debug_info section buffer.  And set diep to
-   point to a newly allocated die with its information.  */
+/* Read the die from the .debug_info section buffer.  Set DIEP to
+   point to a newly allocated die with its information, except for its
+   child, sibling, and parent fields.  Set HAS_CHILDREN to tell
+   whether the die has children or not.  */
 
 static char *
 read_full_die (struct die_info **diep, bfd *abfd, char *info_ptr,
-	       const struct comp_unit_head *cu_header)
+	       const struct comp_unit_head *cu_header, int *has_children)
 {
   unsigned int abbrev_number, bytes_read, i, offset;
   struct abbrev_info *abbrev;
@@ -4194,19 +4252,20 @@ read_full_die (struct die_info **diep, b
       die->abbrev = abbrev_number;
       die->type = NULL;
       *diep = die;
+      *has_children = 0;
       return info_ptr;
     }
 
   abbrev = dwarf2_lookup_abbrev (abbrev_number, cu_header);
   if (!abbrev)
     {
-      error ("Dwarf Error: could not find abbrev number %d [in module %s]", abbrev_number, 
-		      bfd_get_filename (abfd));
+      error ("Dwarf Error: could not find abbrev number %d [in module %s]",
+	     abbrev_number, 
+	     bfd_get_filename (abfd));
     }
   die = dwarf_alloc_die ();
   die->offset = offset;
   die->tag = abbrev->tag;
-  die->has_children = abbrev->has_children;
   die->abbrev = abbrev_number;
   die->type = NULL;
 
@@ -4221,6 +4280,7 @@ read_full_die (struct die_info **diep, b
     }
 
   *diep = die;
+  *has_children = abbrev->has_children;
   return info_ptr;
 }
 
@@ -5822,43 +5882,7 @@ copy_die (struct die_info *old_die)
 static struct die_info *
 sibling_die (struct die_info *die)
 {
-  int nesting_level = 0;
-
-  if (!die->has_children)
-    {
-      if (die->next && (die->next->tag == 0))
-	{
-	  return NULL;
-	}
-      else
-	{
-	  return die->next;
-	}
-    }
-  else
-    {
-      do
-	{
-	  if (die->has_children)
-	    {
-	      nesting_level++;
-	    }
-	  if (die->tag == 0)
-	    {
-	      nesting_level--;
-	    }
-	  die = die->next;
-	}
-      while (nesting_level);
-      if (die && (die->tag == 0))
-	{
-	  return NULL;
-	}
-      else
-	{
-	  return die;
-	}
-    }
+  return die->sibling;
 }
 
 /* Get linkage name of a die, return NULL if not found.  */
@@ -6727,7 +6751,7 @@ dump_die (struct die_info *die)
   fprintf_unfiltered (gdb_stderr, "Die: %s (abbrev = %d, offset = %d)\n",
 	   dwarf_tag_name (die->tag), die->abbrev, die->offset);
   fprintf_unfiltered (gdb_stderr, "\thas children: %s\n",
-	   dwarf_bool_name (die->has_children));
+	   dwarf_bool_name (die->child != NULL));
 
   fprintf_unfiltered (gdb_stderr, "\tattributes:\n");
   for (i = 0; i < die->num_attrs; ++i)
@@ -6790,7 +6814,10 @@ dump_die_list (struct die_info *die)
   while (die)
     {
       dump_die (die);
-      die = die->next;
+      if (die->child != NULL)
+	dump_die_list (die->child);
+      if (die->sibling != NULL)
+	dump_die_list (die->sibling);
     }
 }
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-09-30 16:29 [rfa] add 'parent' field to struct die_info David Carlton
@ 2003-09-30 22:13 ` Jim Blandy
  2003-09-30 22:54   ` David Carlton
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Blandy @ 2003-09-30 22:13 UTC (permalink / raw)
  To: David Carlton; +Cc: gdb-patches, Elena Zannoni


David Carlton <carlton@kealia.com> writes:
> This patch adds a 'parent' field to struct die_info.  There's no way
> to get that information out of our current data structures, and it is
> necessary for nested type support: die A might reference a type die B
> via DW_AT_specification, and to correctly deal with the possibility
> that B might be a nested type, we'll have to know B's parent (if any).
> (See the thread starting in
> <http://sources.redhat.com/ml/gdb/2003-05/msg00173.html>.)  I was
> going to do this as part of a big nested type patch, but I figured
> Elena was going to ask me to commit this part separately anyways. :-)
> (But I would appreciate it if you could review it quickly: it's
> simple.)
> 
> While I was at it, I replaced the existing 'has_children' and 'next'
> fields by 'child' and 'sibling' fields: it seems clearer to me that
> way.  The result of this is that the function 'sibling_die' isn't
> really pulling its weight any more; I left it in there for now,
> though.

Looks great --- please commit.

Adding the parent pointer is great.  But I also really appreciate the
child/sibling rearrangement... the way it stands is really confusing,
and I think this is much more intuitive.

It doesn't seem to me like the 'abbrev' field of 'struct die_info' is
being used for anything very important; removing that would bring
'struct die_info' back to the size it has now.

I wonder if there isn't a clearer way to read the trees.  The
read_die_and_children / read_die_and_siblings pair is harder for me to
understand than it seems like it should be.  But the burden is on me
to show that there's a better way...


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-09-30 22:13 ` Jim Blandy
@ 2003-09-30 22:54   ` David Carlton
  2003-10-01 17:24     ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: David Carlton @ 2003-09-30 22:54 UTC (permalink / raw)
  To: Jim Blandy; +Cc: gdb-patches, Elena Zannoni

On 30 Sep 2003 17:09:38 -0500, Jim Blandy <jimb@redhat.com> said:

> Looks great --- please commit.

Thanks, done.

> Adding the parent pointer is great.  But I also really appreciate the
> child/sibling rearrangement... the way it stands is really confusing,
> and I think this is much more intuitive.

That was my attitude, too: before, we were too closely tied to the
data structure that the debug info was originally stored in, for no
good reason.

> It doesn't seem to me like the 'abbrev' field of 'struct die_info'
> is being used for anything very important; removing that would bring
> 'struct die_info' back to the size it has now.

Good point.

> I wonder if there isn't a clearer way to read the trees.  The
> read_die_and_children / read_die_and_siblings pair is harder for me
> to understand than it seems like it should be.  But the burden is on
> me to show that there's a better way...

I would believe that there's a better way out there.  I remember
making some mistakes when first writing the code; and when I went back
and looked at it to generate this patch, I'd forgotten about the
invariants of the various functions, so I was confused again.  (Which
is why I added comments at the starts of the functions about what
fields were being set to NULL instead of being filled in correctly.)

At some point in our copious free time, we can revisit the issue.
Hmm.  I guess one thing that's annoying is the existence and number of
out parameters: for example, read_full_die has a return value and
two(!) out parameters now.  The thing is, info_ptr always causes
trouble this way: it's an out parameter for read_die_and_XXX, too, and
similarly when reading partial dies.  And it's not like we ever need
to remember the old values of info_ptr: we're reading the debug info
sequentially, and info_ptr just stores our current position.  So
keeping it around as a parameter doesn't make any more sense than,
say, designing a file structure where you always have to pass in (and
have returned back to you after the read) the current read position.

So if we were doing this in an object-oriented language, I guess we
could make a DWARF 2 parser class that these various functions would
be methods of; then info_ptr would be a data member of that class, and
all of the methods would update that, getting rid of an input and an
output parameter.  (Not to mention, say, abfd or cu_header: those
could be const data members.)  That would be a nice cleanup.
Unfortunately, it still leaves read_full_die with one output
parameter, namely has_children.  And there, I suspect, my structure is
simply wrong: read_full_die and read_die_and_children should probably
be merged somehow, or split into two functions along different lines.
So probably I stuck too closely to the original structure of the code:
I mostly restricted myself to modifying read_comp_unit, but I should
have modified read_full_die more as well.

Well, it's something to think about: certainly this patch won't get in
the way of such a further cleanup.

David Carlton
carlton@kealia.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-09-30 22:54   ` David Carlton
@ 2003-10-01 17:24     ` Elena Zannoni
  2003-10-02  4:07       ` Jim Blandy
  0 siblings, 1 reply; 8+ messages in thread
From: Elena Zannoni @ 2003-10-01 17:24 UTC (permalink / raw)
  To: David Carlton; +Cc: Jim Blandy, gdb-patches, Elena Zannoni

David Carlton writes:
 > On 30 Sep 2003 17:09:38 -0500, Jim Blandy <jimb@redhat.com> said:
 > 
 > > Looks great --- please commit.
 > 
 > Thanks, done.
 > 
 > > Adding the parent pointer is great.  But I also really appreciate the
 > > child/sibling rearrangement... the way it stands is really confusing,
 > > and I think this is much more intuitive.
 > 
 > That was my attitude, too: before, we were too closely tied to the
 > data structure that the debug info was originally stored in, for no
 > good reason.

May I suggest to add a comment where the structure is defined that explains
in plain English the structure/relations of the dies?
From the Dwarf manual:

"The ownership relation of debugging information entries is achieved
naturally because the debugging information is represented as a
tree. The nodes of the tree are the debugging information entries
themselves. The child entries of any node are exactly those debugging
information entries owned by that node. While the ownership relation
of the debugging information entries is represented as a tree, other
relations among the entries exist, for example, a pointer from an
entry representing a variable to another entry representing the type
of that variable. If all such relations are taken into account, the
debugging entries form a graph, not a tree. The tree itself is
represented by flattening it in prefix order. Each debugging
information entry is defined either to have child entries or not to
have child entries. If an entry is defined not to have children, the
next physically succeeding entry is a sibling. If an entry is defined
to have children, the next physically succeeding entry is its first
child. Additional children are represented as siblings of the first
child. A chain of sibling entries is terminated by a null entry."

I think this would help a bit too (of course a bit shortened/paraphrased).

elena


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-10-01 17:24     ` Elena Zannoni
@ 2003-10-02  4:07       ` Jim Blandy
  2003-10-02 15:44         ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Blandy @ 2003-10-02  4:07 UTC (permalink / raw)
  To: Elena Zannoni; +Cc: David Carlton, gdb-patches


Elena Zannoni <ezannoni@redhat.com> writes:
> May I suggest to add a comment where the structure is defined that explains
> in plain English the structure/relations of the dies?

How's this?

2003-10-01  Jim Blandy  <jimb@redhat.com>

	* dwarf2read.c (struct die_info): Doc fix.

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.109
diff -c -c -F'^(' -r1.109 dwarf2read.c
*** gdb/dwarf2read.c	30 Sep 2003 22:29:28 -0000	1.109
--- gdb/dwarf2read.c	2 Oct 2003 04:06:12 -0000
***************
*** 320,328 ****
--- 320,335 ----
      unsigned int num_attrs;	/* Number of attributes */
      struct attribute *attrs;	/* An array of attributes */
      struct die_info *next_ref;	/* Next die in ref hash table */
+ 
+     /* The dies in a compilation unit form an n-ary tree.  PARENT
+        points to this die's parent; CHILD points to the first child of
+        this node; and all the children of a given node are chained
+        together via their SIBLING fields, terminated by a die whose
+        tag is zero.  */
      struct die_info *child;	/* Its first child, if any.  */
      struct die_info *sibling;	/* Its next sibling, if any.  */
      struct die_info *parent;	/* Its parent, if any.  */
+ 
      struct type *type;		/* Cached type information */
    };
  


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-10-02  4:07       ` Jim Blandy
@ 2003-10-02 15:44         ` Elena Zannoni
  2003-10-02 15:57           ` Jim Blandy
  0 siblings, 1 reply; 8+ messages in thread
From: Elena Zannoni @ 2003-10-02 15:44 UTC (permalink / raw)
  To: Jim Blandy; +Cc: Elena Zannoni, David Carlton, gdb-patches

Jim Blandy writes:
 > 
 > Elena Zannoni <ezannoni@redhat.com> writes:
 > > May I suggest to add a comment where the structure is defined that explains
 > > in plain English the structure/relations of the dies?
 > 
 > How's this?
 > 

The thing I am trying to capture is the organization of the
tree/graph, i.e.  how the various functions that loop through the
entries following the pointers navigate the structure.

I get often confused by statements like:
      if (dies->has_children)
	{
	  child_die = dies->next;

which now is much clearer anyway. I think basically we need something
to capture the definition of next die to be visited. Like below.

Each debugging information entry is defined either to have child
entries or not to have child entries. If an entry is defined not to
have children, the next physically succeeding entry is a sibling. If
an entry is defined to have children, the next physically succeeding
entry is its first child. Additional children are represented as
siblings of the first child. A chain of sibling entries is terminated
by a null entry.

elena

 > 2003-10-01  Jim Blandy  <jimb@redhat.com>
 > 
 > 	* dwarf2read.c (struct die_info): Doc fix.
 > 
 > Index: gdb/dwarf2read.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dwarf2read.c,v
 > retrieving revision 1.109
 > diff -c -c -F'^(' -r1.109 dwarf2read.c
 > *** gdb/dwarf2read.c	30 Sep 2003 22:29:28 -0000	1.109
 > --- gdb/dwarf2read.c	2 Oct 2003 04:06:12 -0000
 > ***************
 > *** 320,328 ****
 > --- 320,335 ----
 >       unsigned int num_attrs;	/* Number of attributes */
 >       struct attribute *attrs;	/* An array of attributes */
 >       struct die_info *next_ref;	/* Next die in ref hash table */
 > + 
 > +     /* The dies in a compilation unit form an n-ary tree.  PARENT
 > +        points to this die's parent; CHILD points to the first child of
 > +        this node; and all the children of a given node are chained
 > +        together via their SIBLING fields, terminated by a die whose
 > +        tag is zero.  */
 >       struct die_info *child;	/* Its first child, if any.  */
 >       struct die_info *sibling;	/* Its next sibling, if any.  */
 >       struct die_info *parent;	/* Its parent, if any.  */
 > + 
 >       struct type *type;		/* Cached type information */
 >     };
 >   


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-10-02 15:44         ` Elena Zannoni
@ 2003-10-02 15:57           ` Jim Blandy
  2003-10-02 16:00             ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Blandy @ 2003-10-02 15:57 UTC (permalink / raw)
  To: Elena Zannoni; +Cc: David Carlton, gdb-patches


Elena Zannoni <ezannoni@redhat.com> writes:
> Each debugging information entry is defined either to have child
> entries or not to have child entries. If an entry is defined not to
> have children, the next physically succeeding entry is a sibling. If
> an entry is defined to have children, the next physically succeeding
> entry is its first child. Additional children are represented as
> siblings of the first child. A chain of sibling entries is terminated
> by a null entry.

Sure, that sort of explanation would be appropriate for the die
reading code, but I think it would just be confusing for 'struct
die_info'.  Once we've built the die tree in memory, all that detail
is gone, and you've just got the pure tree structure.  There's no
concept of "the next physically succeeding entry", for example.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [rfa] add 'parent' field to struct die_info
  2003-10-02 15:57           ` Jim Blandy
@ 2003-10-02 16:00             ` Elena Zannoni
  0 siblings, 0 replies; 8+ messages in thread
From: Elena Zannoni @ 2003-10-02 16:00 UTC (permalink / raw)
  To: Jim Blandy; +Cc: Elena Zannoni, David Carlton, gdb-patches

Jim Blandy writes:
 > 
 > Elena Zannoni <ezannoni@redhat.com> writes:
 > > Each debugging information entry is defined either to have child
 > > entries or not to have child entries. If an entry is defined not to
 > > have children, the next physically succeeding entry is a sibling. If
 > > an entry is defined to have children, the next physically succeeding
 > > entry is its first child. Additional children are represented as
 > > siblings of the first child. A chain of sibling entries is terminated
 > > by a null entry.
 > 
 > Sure, that sort of explanation would be appropriate for the die
 > reading code, but I think it would just be confusing for 'struct
 > die_info'.  Once we've built the die tree in memory, all that detail
 > is gone, and you've just got the pure tree structure.  There's no
 > concept of "the next physically succeeding entry", for example.

whatever


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2003-10-02 16:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-09-30 16:29 [rfa] add 'parent' field to struct die_info David Carlton
2003-09-30 22:13 ` Jim Blandy
2003-09-30 22:54   ` David Carlton
2003-10-01 17:24     ` Elena Zannoni
2003-10-02  4:07       ` Jim Blandy
2003-10-02 15:44         ` Elena Zannoni
2003-10-02 15:57           ` Jim Blandy
2003-10-02 16:00             ` Elena Zannoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox