libdw_visit_scopes: Don't recurse into imported unit children.

    There were two issues with __libdw_visit_scopes. First it returned an
    error whenever called with a root DIE that didn't have any children.
    However a DIE abbrev can have DW_CHILDREN_yes set and still have an
    empty child list. It is inefficient and should be fixed in the DWARF
    producer, but it is legal DWARF. Also the function shouldn't hide real
    DWARF errors found by dwarf_child () or dwarf_siblingof (). Secondly
    __libdw_visit_scopes would recurse into an imported unit and then
    forget about the rest of the children of the root DIE. For an imported
    unit the children of that unit DIE should be treated as if they were
    logical children of the root DIE and not increase the recursion depth.
    Both issues were shown by a systemtap bug using dwarf_getscopes () on
    a CU that had various partial units imported through dwz.

    See http://sourceware.org/bugzilla/show_bug.cgi?id=15671

    Signed-off-by: Mark Wielaard <mjw@redhat.com>

2 files changed, 63 insertions, 44 deletions

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 59b6c63a..700c166d 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,11 @@
+2013-06-26  Mark Wielaard  <mjw@redhat.com>
+
+	* libdw_visit_scopes.c (__libdw_visit_scopes): Don't reject root
+	DIEs without children. Return an error whenever dwarf_child or
+	dwarf_siblingof return an error. Don't call recurse and increase
+	the depth for an imported unit. Walk the children of an imported
+	unit as if they are logical children of the parent root DIE.
+
 2013-05-03  Mark Wielaard  <mjw@redhat.com>
 
 	* dwarf_getsrclines.c (dwarf_getsrclines): Only set end_sequence
diff --git a/libdw/libdw_visit_scopes.c b/libdw/libdw_visit_scopes.c
index ea0c6b47..a8555ce4 100644
--- a/libdw/libdw_visit_scopes.c
+++ b/libdw/libdw_visit_scopes.c
@@ -84,10 +84,11 @@ __libdw_visit_scopes (depth, root, previsit, postvisit, arg)
      void *arg;
 {
   struct Dwarf_Die_Chain child;
+  int ret;
 
   child.parent = root;
-  if (INTUSE(dwarf_child) (&root->die, &child.die) != 0)
-    return -1;
+  if ((ret = INTUSE(dwarf_child) (&root->die, &child.die)) != 0)
+    return ret < 0 ? -1 : 0; // Having zero children is legal.
 
   inline int recurse (void)
     {
@@ -95,63 +96,73 @@ __libdw_visit_scopes (depth, root, previsit, postvisit, arg)
 				   previsit, postvisit, arg);
     }
 
-  do
-    {
-      child.prune = false;
-
-      if (previsit != NULL)
-	{
-	  int result = (*previsit) (depth + 1, &child, arg);
-	  if (result != DWARF_CB_OK)
-	    return result;
-	}
-
-      if (!child.prune)
-	switch (classify_die (&child.die))
+  inline int walk_children ()
+  {
+    do
+      {
+	/* For an imported unit, it is logically as if the children of
+	   that unit are siblings of the other children.  So don't do
+	   a full recursion into the imported unit, but just walk the
+	   children in place before moving to the next real child.  */
+	while (classify_die (&child.die) == imported)
 	  {
-	  case match:
-	  case match_inline:
-	  case walk:
-	    if (INTUSE(dwarf_haschildren) (&child.die))
+	    Dwarf_Die orig_child_die = child.die;
+	    Dwarf_Attribute attr_mem;
+	    Dwarf_Attribute *attr = INTUSE(dwarf_attr) (&child.die,
+							DW_AT_import,
+							&attr_mem);
+	    if (INTUSE(dwarf_formref_die) (attr, &child.die) != NULL
+                && INTUSE(dwarf_child) (&child.die, &child.die) == 0)
 	      {
-		int result = recurse ();
+		int result = walk_children ();
 		if (result != DWARF_CB_OK)
 		  return result;
 	      }
-	    break;
 
-	  case imported:
+	    /* Any "real" children left?  */
+	    if ((ret = INTUSE(dwarf_siblingof) (&orig_child_die,
+						&child.die)) != 0)
+	      return ret < 0 ? -1 : 0;
+	  };
+
+	child.prune = false;
+
+	if (previsit != NULL)
+	  {
+	    int result = (*previsit) (depth + 1, &child, arg);
+	    if (result != DWARF_CB_OK)
+	      return result;
+	  }
+
+	if (!child.prune)
+	  switch (classify_die (&child.die))
 	    {
-	      /* This imports another compilation unit to appear
-		 as part of this one, inside the current scope.
-		 Recurse to search the referenced unit, but without
-		 recording it as an inner scoping level.  */
-
-	      Dwarf_Attribute attr_mem;
-	      Dwarf_Attribute *attr = INTUSE(dwarf_attr) (&child.die,
-							  DW_AT_import,
-							  &attr_mem);
-	      if (INTUSE(dwarf_formref_die) (attr, &child.die) != NULL)
+	    case match:
+	    case match_inline:
+	    case walk:
+	      if (INTUSE(dwarf_haschildren) (&child.die))
 		{
 		  int result = recurse ();
 		  if (result != DWARF_CB_OK)
 		    return result;
 		}
+	      break;
+
+	    default:
+	      break;
 	    }
-	    break;
 
-	  default:
-	    break;
+	if (postvisit != NULL)
+	  {
+	    int result = (*postvisit) (depth + 1, &child, arg);
+	    if (result != DWARF_CB_OK)
+	      return result;
 	  }
+      }
+    while ((ret = INTUSE(dwarf_siblingof) (&child.die, &child.die)) == 0);
 
-      if (postvisit != NULL)
-	{
-	  int result = (*postvisit) (depth + 1, &child, arg);
-	  if (result != DWARF_CB_OK)
-	    return result;
-	}
-    }
-  while (INTUSE(dwarf_siblingof) (&child.die, &child.die) == 0);
+    return ret < 0 ? -1 : 0;
+  }
 
-  return 0;
+  return walk_children ();
 }