[patch] Fix PR middle-end/51994

2012-02-07 Thread Eric Botcazou
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  ebotca...@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  ebotca...@adacore.com

* gcc.c-torture/execute/20120207-1.c: New test.


-- 
Eric Botcazou
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;
 }
/* PR middle-end/51994 */
/* Testcase by Uros Bizjak ubiz...@gmail.com */

extern char *strcpy (char *, const char *);
extern void abort (void);

char __attribute__((noinline))
test (int a)
{
  char buf[16];
  char *output = buf;

  strcpy (buf[0], 0123456789);

  output += a;
  output -= 1;

  return output[0];
}

int main ()
{
  if (test (2) != '1')
abort ();

  return 0;
}


Re: [patch] Fix PR middle-end/51994

2012-02-07 Thread Richard Guenther
On Tue, Feb 7, 2012 at 10:50 AM, Eric Botcazou ebotca...@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  ebotca...@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  ebotca...@adacore.com

        * gcc.c-torture/execute/20120207-1.c: New test.


 --
 Eric Botcazou