fix for segmentation violation in dump_generic_node

[Tom de Vries]

Hi Richard,

thanks for the review.

On 08/25/2011 12:45 PM, Richard Guenther wrote:
> On Thu, Aug 25, 2011 at 12:32 PM, Tom de Vries <vries@codesourcery.com> wrote:
>> Jakub,
>>
>> This patch fixes a segmentation violation, which occurs when printing a MEM_REF
>> or COMPONENT_REF containing a released ssa name.  This can happen when we print
>> basic blocks upon removal, enabled by -ftree-dump-tree-*-details (see
>> remove_bb:tree-cfg.c).
> 
> Where do we dump stmts there?
> 

In dump_bb:

static void
remove_bb (basic_block bb)
{
  gimple_stmt_iterator i;

  if (dump_file)
    {
      fprintf (dump_file, "Removing basic block %d\n", bb->index);
      if (dump_flags & TDF_DETAILS)
	{
	  dump_bb (bb, dump_file, 0);
	  fprintf (dump_file, "\n");
	}
    }

>> Bootstrapped and reg-tested on x86_64.
>>
>> OK for trunk?
> 
> At least
> 
>   TREE_TYPE (TREE_OPERAND (node, 1)) != NULL_TREE
> 
> is always true.
> 

Right.

> The comment before the new lines is now in the wrong place and this
> check at least needs a comment as well.
> 

Ok, fixed that.

> But - it's broken to dump freed stuff, why and where do we do this?
> 

Sorry, I did not realize that.

The scenario is as follows: fnsplit splits a function, and as todo
cleanup_tree_cfg is called and unreachable blocks are removed, among which
blocks 12 and 13.

Block 12 contains a use of 45:

  # BLOCK 12 freq:9100
  # PRED: 13
  D.13888_46 = *sD.13886_45;

Block 13 contains a def of 45:

Block 13
  # BLOCK 13
  # PRED: 11 12
  ...
  # sD.13886_45 = PHI <sD.13886_44(11), sD.13886_49(12)>
  ...
  if (sizeD.8479_2 > iD.13887_50)
    goto <bb 12>;
  else
    goto <bb 14>;
  # SUCC: 12 14


First block 13 is removed, and remove_phi_nodes_and_edges_for_unreachable_block
in remove_bb removes the phi def and releases version 45. Then block 12 is
removed, and before removal it is dumped by dump_bb in remove_bb, triggering the
segv.

The order of removal is determined by the 2nd loop in delete_unreachable_blocks,
which is chosen because there is no dominator info present:

     for (b = EXIT_BLOCK_PTR->prev_bb; b != ENTRY_BLOCK_PTR; b = prev_bb)
	{
	  prev_bb = b->prev_bb;

	  if (!(b->flags & BB_REACHABLE))
	    {
	      delete_basic_block (b);
	      changed = true;
	    }
	}

I'm not sure how to fix this.


Another occurance of the same segv is in remove_dead_inserted_code:

  EXECUTE_IF_SET_IN_BITMAP (inserted_exprs, 0, i, bi)
    {
      t = SSA_NAME_DEF_STMT (ssa_name (i));
      if (!gimple_plf (t, NECESSARY))
	{
	  gimple_stmt_iterator gsi;

	  if (dump_file && (dump_flags & TDF_DETAILS))
	    {
	      fprintf (dump_file, "Removing unnecessary insertion:");
	      print_gimple_stmt (dump_file, t, 0, 0);
	    }

	  gsi = gsi_for_stmt (t);
	  if (gimple_code (t) == GIMPLE_PHI)
	    remove_phi_node (&gsi, true);
	  else
	    {
	      gsi_remove (&gsi, true);
	      release_defs (t);
	    }
	}
    }

Here a version is released, while it's used in the defining statement of
version+1, which is subsequently printed. This is easy to fix by splitting the
loop, I'll make a patch for this.


There might be other occurrences (I triggered these 2 doing a gcc build), but I
cannot trigger others until delete_unreachable_blocks does not trigger anymore.

> Richard.
> 

Updated untested patch attached, I'll test this patch together with the
remove_dead_inserted_code patch.

Thanks,
- Tom

>> 2011-08-25  Tom de Vries  <tom@codesourcery.com>
>>
>>        * tree-pretty-print (dump_generic_node): Test for NULL_TREE before
>>        accessing TREE_TYPE.
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: tree-pretty-print-fix.patch
Type: text/x-patch
Size: 1137 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110825/9dd913ba/attachment.bin>