Fix two problems with reorder_blocks and debugging

Daniel Jacobowitz drow@false.org
Sat Jul 22 03:18:00 GMT 2006


This patch fixes a bug in debugging output that I encountered today.
In reorder_blocks_1, we attach every fragment of a BLOCK to an original
block using BLOCK_FRAGMENT_CHAIN and BLOCK_FRAGMENT_ORIGIN.  However, we
then attach subblocks to whichever fragment first includes them.  The
dwarf writer does have code to output subblocks of every fragment, but it
generates a very strange looking tree.  Suppose you have block A, containing
B, containing C:

  A
  -> B
     -> C

But the first bit of C encountered is in the second fragment of B. 
dwarf2out ignores fragments except to output their children.  But it has to
output the parents of blocks to output the blocks.  So in the end it
generates this tree:

  A
  -> B
  -> B
     -> C

The first copy of B has the block's local variables in it.  The second copy
doesn't.  So when we're in C, we don't realize we're also in B.  And B and C
may have similar large ranges, so it's quite possible for the debugger to
select C when you wanted it to select B, and suddenly your locals are
missing.  I fixed this by attaching all blocks to the origin.

While there I realized that reorder_fix_fragments is basically dead.  It
prunes !TREE_ASM_WRITTEN fragments, but reorder_blocks_1 nowadays only
creates fragments if they are written.

I tested this patch by eye on x86_64-pc-linux-gnu (since neither the GCC nor
GDB testsuites exercise optimized debug info at the moment).  I also
regression tested it twice, once with strategic aborts in
reorder_fix_fragments to make sure it really wasn't doing anything.

OK to commit?

-- 
Daniel Jacobowitz
CodeSourcery

2006-07-21  Daniel Jacobowitz  <dan@codesourcery.com>

	* function.c (reorder_fix_fragments): Delete.
	(reorder_blocks): Don't call it.
	(reorder_blocks_1): Put all subblocks under the origin block.

Index: function.c
===================================================================
--- function.c	(revision 115644)
+++ function.c	(working copy)
@@ -195,7 +195,6 @@ static struct temp_slot *find_temp_slot_
 static void pad_to_arg_alignment (struct args_size *, int, struct args_size *);
 static void pad_below (struct args_size *, enum machine_mode, tree);
 static void reorder_blocks_1 (rtx, tree, VEC(tree,heap) **);
-static void reorder_fix_fragments (tree);
 static int all_blocks (tree, tree *);
 static tree *get_block_vector (tree, int *);
 extern tree debug_find_var_in_block_tree (tree, tree);
@@ -3584,9 +3583,6 @@ reorder_blocks (void)
   reorder_blocks_1 (get_insns (), block, &block_stack);
   BLOCK_SUBBLOCKS (block) = blocks_nreverse (BLOCK_SUBBLOCKS (block));
 
-  /* Remove deleted blocks from the block fragment chains.  */
-  reorder_fix_fragments (block);
-
   VEC_free (tree, heap, block_stack);
 }
 
@@ -3615,17 +3611,18 @@ reorder_blocks_1 (rtx insns, tree curren
 	  if (NOTE_LINE_NUMBER (insn) == NOTE_INSN_BLOCK_BEG)
 	    {
 	      tree block = NOTE_BLOCK (insn);
+	      tree origin;
+
+	      origin = (BLOCK_FRAGMENT_ORIGIN (block)
+			? BLOCK_FRAGMENT_ORIGIN (block)
+			: block);
 
 	      /* If we have seen this block before, that means it now
 		 spans multiple address regions.  Create a new fragment.  */
 	      if (TREE_ASM_WRITTEN (block))
 		{
 		  tree new_block = copy_node (block);
-		  tree origin;
 
-		  origin = (BLOCK_FRAGMENT_ORIGIN (block)
-			    ? BLOCK_FRAGMENT_ORIGIN (block)
-			    : block);
 		  BLOCK_FRAGMENT_ORIGIN (new_block) = origin;
 		  BLOCK_FRAGMENT_CHAIN (new_block)
 		    = BLOCK_FRAGMENT_CHAIN (origin);
@@ -3642,10 +3639,13 @@ reorder_blocks_1 (rtx insns, tree curren
 		 will cause infinite recursion.  */
 	      if (block != current_block)
 		{
+		  if (block != origin)
+		    gcc_assert (BLOCK_SUPERCONTEXT (origin) == current_block);
+
 		  BLOCK_SUPERCONTEXT (block) = current_block;
 		  BLOCK_CHAIN (block) = BLOCK_SUBBLOCKS (current_block);
 		  BLOCK_SUBBLOCKS (current_block) = block;
-		  current_block = block;
+		  current_block = origin;
 		}
 	      VEC_safe_push (tree, heap, *p_block_stack, block);
 	    }
@@ -3660,61 +3660,6 @@ reorder_blocks_1 (rtx insns, tree curren
     }
 }
 
-/* Rationalize BLOCK_FRAGMENT_ORIGIN.  If an origin block no longer
-   appears in the block tree, select one of the fragments to become
-   the new origin block.  */
-
-static void
-reorder_fix_fragments (tree block)
-{
-  while (block)
-    {
-      tree dup_origin = BLOCK_FRAGMENT_ORIGIN (block);
-      tree new_origin = NULL_TREE;
-
-      if (dup_origin)
-	{
-	  if (! TREE_ASM_WRITTEN (dup_origin))
-	    {
-	      new_origin = BLOCK_FRAGMENT_CHAIN (dup_origin);
-
-	      /* Find the first of the remaining fragments.  There must
-		 be at least one -- the current block.  */
-	      while (! TREE_ASM_WRITTEN (new_origin))
-		new_origin = BLOCK_FRAGMENT_CHAIN (new_origin);
-	      BLOCK_FRAGMENT_ORIGIN (new_origin) = NULL_TREE;
-	    }
-	}
-      else if (! dup_origin)
-	new_origin = block;
-
-      /* Re-root the rest of the fragments to the new origin.  In the
-	 case that DUP_ORIGIN was null, that means BLOCK was the origin
-	 of a chain of fragments and we want to remove those fragments
-	 that didn't make it to the output.  */
-      if (new_origin)
-	{
-	  tree *pp = &BLOCK_FRAGMENT_CHAIN (new_origin);
-	  tree chain = *pp;
-
-	  while (chain)
-	    {
-	      if (TREE_ASM_WRITTEN (chain))
-		{
-		  BLOCK_FRAGMENT_ORIGIN (chain) = new_origin;
-		  *pp = chain;
-		  pp = &BLOCK_FRAGMENT_CHAIN (chain);
-		}
-	      chain = BLOCK_FRAGMENT_CHAIN (chain);
-	    }
-	  *pp = NULL_TREE;
-	}
-
-      reorder_fix_fragments (BLOCK_SUBBLOCKS (block));
-      block = BLOCK_CHAIN (block);
-    }
-}
-
 /* Reverse the order of elements in the chain T of blocks,
    and return the new head of the chain (old last element).  */
 



More information about the Gcc-patches mailing list