[PATCH] tighten gimple addressability checks

[Olivier Hainque <hainque at adacore dot com>]

Hello,

Attached is a patch suggestion to fix regressions compared to the 3.4
series. Problem/context description first:

is_gimple_addressable is fairly permissive today, and I think too
permissive in some cases.

In particular, COMPONENT_REFs are all considered addressable while
the address of BIT_FIELD decls sometimes really can't be taken.

This is an issue for the gimplifier itself, as it uses the predicate
to decide if it can take the address of an operand, for instance as
part of

     gimplify_modify_expr
     ...
     /* If [...] we can make things a bit more straightforward
	by converting the assignment to memcpy or memset.  */
     if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
       { ...
	 if (is_gimple_addressable (from))

In this specific case, we are even totally missing a check on the
destination's addressability and get misbehavior on the Ada testcase
below:

     procedure P5 is

	type Message is record
	   B : Boolean;
	   Data : String (1 .. 4);
	end record;
	pragma Pack (Message);

	procedure Process (IM : Message; Size : Natural) is
	   LM : Message;
	begin
	   LM.Data (1 .. Size) := IM.Data (1 .. Size);
	end;

     begin
	null;
     end;

>From the gimplifier code mentioned above, the assignment in "Process"
is gimplified into

     D.555 = (character[1:D.536] *) &im.data[1 ...]{lb: 1 sz: 1};
     __builtin_memcpy (&lm.data[1 ...]{lb: 1 sz: 1}, D.555, D.543);

taking the address of both lm.data and im.data. The field is packed at
*bit* position 1, and the compiler (x86_64-linux in my case) ICEs later
on from

     expand_expr_addr_expr_1
     ...
      /* Someone beforehand should have rejected taking the address
	 of such an object.  */
      gcc_assert ((bitpos % BITS_PER_UNIT) == 0);


The general idea of the patch suggested here is simple: tighten the
addressability checks to prevent those too wild address-taking
operations.

There are two complications:

1) Problematic COMPONENT_REFs can be nested within outer tree constructs,
   for instance, "from" in the testcase above is like

      <view_convert_expr
	 type <array_type p5__process__T4b
	...
	arg 0 <array_range_ref
	    ...
	    arg 0 <component_ref 
		...
		arg 0 <parm_decl im>
		...
		arg 1 <field_decl data
		       ...
		       bit offset <integer_cst constant invariant 1>
  
   so we have to recurse to tell.

2) Some DECL_BIT_FIELD components can actually have their address taken
   and rejecting all of those would lead to severe pessimization, typically
   from useless creation of temporaries, if not compiler crashes when
   variable sizes are at sake.

   The approach chosen here is to consider BLKmode fields, known to be
   byte aligned, addressable when the address is taken for the purposes
   of a block oriented operation, where address vs type alignment
   expectations are less strict than in the general case.

   We introduce a new flag on ADDR_EXPR (BLOCK_ADDRESS_P) to convey
   this purpose and an alternate predicate (is_gimple_block_addressable)
   to perform the check.

The patch was bootstrapped and regtested with languages=all,ada on
x86_64-unknown-linux-gnu. We have been using it internally in 4.1
based compilers on a large number of targets for more than a year now.

I realize this is invasive and would be happy to discuss enhancements
or alternate ways out as necessary.

Thanks in advance,

Olivier

2007-10-25  Olivier Hainque  <hainque@adacore.com>
            Eric Botcazou  <ebotcazou@adacore.com>

	* tree.h (BLOCK_ADDRESS_P): New flag on ADDR_EXPR - indicates if
	the address is taken for the purposes of a block oriented operation
	such as memset, memcmp or memcpy.
	* tree-gimple.c (is_gimple_addressable_1): New function.  Similar to
	the former is_gimple_addressable with an extra argument to state if
	the check is made for a BLOCK_ADDRESS_P outer node.  Reject bitfield
	COMPONENT_REFs unless BLKmode for a block address, and recurse to find
	them out.
	(is_gimple_addressable): is_gimple_addressable_1 for the general
	case, with full	constraints on bitfield COMPONENT_REFs.
	(is_gimple_block_addressable): New function, is_gimple_addressable_1
	for the purpose of a block oriented operation.
	(is_gimple_lvalue): Adjust not to rely on is_gimple_addressable.
	* tree-gimple.h: Declare it.
	* gimplify.c: Add notes to explain the need for different sorts of
	addressability checks.
	(build_fold_block_addr_expr): New function.  Returns an expression
	representing the address of its operand for the purposes of a block
	oriented operation.
	(gimplify_modify_expr_to_memcpy): Use build_fold_block_addr_expr
	instead of build_fold_addr_expr to capture the operands addresses.
	(gimplify_modify_expr_to_memset): Likewise.
	(gimplify_variable_sized_compare): Likewise.
	(gimplify_modify_expr): For assignments we turn into memcpy or memset,
	check that both source and destination are block-addressable.
	(gimplify_addr_expr) <VIEW_CONVERT_EXPR operand>: If the input
	ADDR_EXPR is a block address, so is the transposed operand address we
	make.
	<default case>: If we are gimplifying a BLOCK_ADDRESS_P expression,
	use is_gimple_block_addressable instead of is_gimple_addressable as
	the predicate for our operand gimplification.
	* builtins.c (get_memory_rtx): Accept BLKmode DECL_BIT_FIELDs in
	COMPONENT_REFs.

	testsuite/
	* gnat.dg/packed_varsize_copy.adb: New test.

Attachment: block_address.dif
Description: Text document