Patchwork Fix PR middle-end/51994

login
register
mail settings
Submitter Eric Botcazou
Date Feb. 7, 2012, 9:50 a.m.
Message ID <201202071050.47451.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/139892/
State New
Headers show

Comments

Eric Botcazou - Feb. 7, 2012, 9:50 a.m.
Hi,

this is a regression present on mainline and 4.6 branch, apparently a fallout 
of the MEM_REF introduction.  get_inner_reference can be called on MEM_REFs 
whose base has been shifted to the left

  char *output = buf;

  output += a;
  output -= 1;

  output[0];

and, since the constant negative offset is merged into the MEM_REF, it will be 
returned as the BITPOS by get_inner_reference, which wreaks havoc later in the 
bitfield manipulation routines which expect non-negative bit positions.

Clearly nothing says that the returned BITPOS should be non-negative but, on 
the other hand, it de facto was in the pre-MEM_REF world for valid programs 
(except maybe in a very specific case in Ada).  The attached patch attempts to 
patch things up to bring us back to the previous de facto situation.

Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 4.6 branch?


2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/51994
	* expr.c (get_inner_reference): If there is an offset, add a negative
	bit position to it (if any).


2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/execute/20120207-1.c: New test.
Richard Guenther - Feb. 7, 2012, 10:01 a.m.
On Tue, Feb 7, 2012 at 10:50 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is a regression present on mainline and 4.6 branch, apparently a fallout
> of the MEM_REF introduction.  get_inner_reference can be called on MEM_REFs
> whose base has been shifted to the left
>
>  char *output = buf;
>
>  output += a;
>  output -= 1;
>
>  output[0];
>
> and, since the constant negative offset is merged into the MEM_REF, it will be
> returned as the BITPOS by get_inner_reference, which wreaks havoc later in the
> bitfield manipulation routines which expect non-negative bit positions.
>
> Clearly nothing says that the returned BITPOS should be non-negative but, on
> the other hand, it de facto was in the pre-MEM_REF world for valid programs
> (except maybe in a very specific case in Ada).  The attached patch attempts to
> patch things up to bring us back to the previous de facto situation.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 4.6 branch?

Ok.

Note that with your patch we can still get negative bitpos for invalid code
like

char *output = buf;
output[-1];

but I suppose we don't need to worry about this case too much (if we do
we'd need to adjust the TREE_CODE (offset) == INTEGER_CST case
as well).

Thanks,
Richard.

>
> 2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>
>
>        PR middle-end/51994
>        * expr.c (get_inner_reference): If there is an offset, add a negative
>        bit position to it (if any).
>
>
> 2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gcc.c-torture/execute/20120207-1.c: New test.
>
>
> --
> Eric Botcazou

Patch

Index: expr.c
===================================================================
--- expr.c	(revision 183864)
+++ expr.c	(working copy)
@@ -6716,6 +6716,24 @@  get_inner_reference (tree exp, HOST_WIDE
   /* Otherwise, split it up.  */
   if (offset)
     {
+      /* Avoid returning a negative bitpos as this may wreak havoc later.  */
+      if (double_int_negative_p (bit_offset))
+        {
+	  double_int mask
+	    = double_int_mask (BITS_PER_UNIT == 8
+			       ? 3 : exact_log2 (BITS_PER_UNIT));
+	  double_int tem = double_int_and_not (bit_offset, mask);
+	  /* TEM is the bitpos rounded to BITS_PER_UNIT towards -Inf.
+	     Subtract it to BIT_OFFSET and add it (scaled) to OFFSET.  */
+	  bit_offset = double_int_sub (bit_offset, tem);
+	  tem = double_int_rshift (tem,
+				   BITS_PER_UNIT == 8
+				   ? 3 : exact_log2 (BITS_PER_UNIT),
+				   HOST_BITS_PER_DOUBLE_INT, true);
+	  offset = size_binop (PLUS_EXPR, offset,
+			       double_int_to_tree (sizetype, tem));
+	}
+
       *pbitpos = double_int_to_shwi (bit_offset);
       *poffset = offset;
     }