diff mbox

[RFC] Fix PR48124

Message ID alpine.LNX.2.00.1202011409390.4999@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Feb. 1, 2012, 1:21 p.m. UTC 
This fixes PR48124 where a bitfield store leaks into adjacent
decls if the decl we store to has bigger alignment than what
its type requires (thus there is another decl in that "padding").

What we need to do is limit what get_best_mode returns and we
can do so by specifying a mode on the MEM we pass down to
store_fixed_bit_field.

The fix for this particular issue could be made less agressive
by also checking bitpos + bitsize whether it can possibly overlap
the last mode-sized word of the decl.

I'm not sure what to do for non-constant DECL_SIZE (I suppose
the same issue can pop up for VLA stack variables with an
explicit alignment and adjacent vars).  Should we unconditionally
use TYPE_ALIGN instead assuming that DECL_SIZE is a multiple
of that?  That would of course pessimize things even more,
so maybe only fall back to that if DECL_SIZE is non-constant?

This simply seemed the least invasive fix, it won't fix things
for indirect references where the known alignment is bigger
than what we'd expect from the size.  But then we'd have
to fall back to using TYPE_SIZE and probably MEM_ALIGN
(which is what we pass down to get_best_mode in the end).

We could also fixup the interface of get_best_mode to also
pass down an object size so it can properly adjust the mode
according to that.  But as it already has a mode argument
the callers might be the better place(s) to fix, possibly
using a helper function (the caller in question could use
MEM_SIZE).

Thoughts?

Bootstraped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2012-02-01  Richard Guenther  <rguenther@suse.de>

	PR middle-end/48124
	* expr.c (expand_assignment): For bitfield stores to a decl
	that does not have a size that is a multiple of its alignment
	restrict the mode we use for the bitfield accesses to make
	sure they do not cross the object boundary.

	* gcc.dg/torture/pr48124-1.c: New testcase.
	* gcc.dg/torture/pr48124-2.c: Likewise.
	* gcc.dg/torture/pr48124-3.c: Likewise.
diff mbox

Patch

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 183791)
+++ gcc/expr.c	(working copy)
@@ -4705,6 +4705,22 @@  expand_assignment (tree to, tree from, b
 	    to_rtx = adjust_address (to_rtx, mode1, 0);
 	  else if (GET_MODE (to_rtx) == VOIDmode)
 	    to_rtx = adjust_address (to_rtx, BLKmode, 0);
+	  /* If the alignment of tem is larger than its size and we
+	     are performing a bitfield access limit the mode we use
+	     for the access to make sure we do not access the decl
+	     beyond its end.  See PR48124.  */
+	  else if (GET_MODE (to_rtx) == BLKmode
+	  	   && mode1 == VOIDmode
+		   && DECL_P (tem)
+		   && TREE_CODE (DECL_SIZE (tem)) == INTEGER_CST
+		   && (TREE_INT_CST_LOW (DECL_SIZE (tem))
+		       & (DECL_ALIGN (tem) - 1)) != 0)
+	    {
+	      unsigned HOST_WIDE_INT mis;
+	      mis = TREE_INT_CST_LOW (DECL_SIZE (tem)) & (DECL_ALIGN (tem) - 1);
+	      mode1 = mode_for_size (mis & -mis, MODE_INT, 0);
+	      to_rtx = adjust_address (to_rtx, mode1, 0);
+	    }
 	}
  
       if (offset != 0)
Index: gcc/testsuite/gcc.dg/torture/pr48124-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr48124-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr48124-1.c	(revision 0)
@@ -0,0 +1,33 @@ 
+/* { dg-do run } */
+/* { dg-options "-fno-toplevel-reorder" } */
+
+extern void abort (void);
+
+struct S
+{
+  signed a : 26;
+  signed b : 16;
+  signed c : 10;
+  volatile signed d : 14;
+};
+
+static struct S e = { 0, 0, 0, 1 };
+static int f = 1;
+
+void __attribute__((noinline))
+foo (void)
+{
+  e.d = 0;
+  f = 2;
+}
+
+int
+main ()
+{
+  if (e.a || e.b || e.c || e.d != 1 || f != 1)
+    abort ();
+  foo ();
+  if (e.a || e.b || e.c || e.d || f != 2)
+    abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/pr48124-2.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr48124-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr48124-2.c	(revision 0)
@@ -0,0 +1,27 @@ 
+/* { dg-do run } */
+
+extern void abort (void);
+
+static volatile struct S0 {
+    short f3[9];
+    unsigned f8 : 15;
+} s = {1};
+static unsigned short sh = 0x1234;
+
+struct S0 a, b;
+int vi = 0;
+
+void func_4()
+{
+  s.f8 |= 1;
+  sh = 15;
+  if (vi) a = b;
+}
+
+int main()
+{
+  func_4();
+  if (sh != 15)
+    abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/pr48124-3.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr48124-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr48124-3.c	(revision 0)
@@ -0,0 +1,32 @@ 
+/* { dg-do run } */
+
+extern void abort (void);
+struct S1
+{
+  int f0;
+  int:1;
+  int f3;
+  int:1;
+  int:0;
+  int f6:1;
+};
+int g_13 = 1;
+volatile struct S1 g_118 = {
+    1
+};
+
+void __attribute__((noinline))
+func_46 ()
+{
+  for (g_13 = 0; g_13 >= 0; g_13 -= 1)
+    g_118.f6 = 0;
+}
+
+int
+main ()
+{
+  func_46 ();
+  if (g_13 != -1)
+    abort ();
+  return 0;
+}