This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[vta, trunk?] fix latent VRP bootstrap-failure-inducing undefined-behavior


Bootstrapping the VTA branch on i686-pc-linux-gnu triggered a compare
error in libcpp/expr.c.  It turned out to be an error in VRP that,
most often, gets hidden, but that in the VTA branch, caused actual
codegen differences.

It's easy to see from the patch that, when ior_max.low is zero, we end
up shifting a value by floor_log2(0), which is -1, invoking undefined
behavior.

It just so happens that, on x86, stage 1 is compiled without
optimization, and thus the shift operation that masks the count to 5
bits ends up storing 0x7FFFFFFFll in ior_max.low, whereas in stage 2,
floor_log2 gets inlined from toplev.h:

extern inline int
floor_log2 (unsigned HOST_WIDE_INT x)
{
  return x ? HOST_BITS_PER_WIDE_INT - 1 - (int) CLZ_HWI (x) : -1;
}

... and the exposed test for zero resulting -1 ends up storing a
*different* value in ior_max.low: -1ll.

As a result, while compiling stage 2, an anonymous temporary variable
is initially given a non-negative range, while in stage 3, it gets a
varying range because max is lower than min.

I haven't investigated while this causes visible codegen differences
in VTA but not in the trunk, but I have verified that the initial
range computed for the same temp variable is indeed different
depending on whether the -O0 stage1 or the -O2 stage2 cc1 is used to
build libcpp/expr.c, and that this simple fix cures the compare error
in the branch.

Although this difference didn't survive through to the end of
compilation on my trunk builds, there's a possibility that other
machines that behave differently for out-of-range shifts could produce
results that are so different that the error makes it to the end of
compilation, causing bootstrap failures.

Since this code is relatively new (2008-09-11), these bootstrap
failures would amount to regressions.  I'll install it in the branch
as soon as my testing completes.  As for the trunk, I'm tempted to
check it in as obvious, but since we're so close to cutting the
release branch, I'll wait for an explicit ACK.

for  gcc/ChangeLog.vta
from  Alexandre Oliva  <aoliva@redhat.com>

	* tree-vrp.c (extract_range_from_binary_expr): Don't shift by
	floor_log2 of zero.  Negate widened zero.

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c.orig	2008-12-10 02:39:37.000000000 -0200
+++ gcc/tree-vrp.c	2008-12-11 05:05:26.000000000 -0200
@@ -2557,11 +2557,11 @@ extract_range_from_binary_expr (value_ra
 	  ior_max.high = vr0_max.high | vr1_max.high;
 	  if (ior_max.high != 0)
 	    {
-	      ior_max.low = ~0u;
+	      ior_max.low = ~(unsigned HOST_WIDE_INT)0u;
 	      ior_max.high |= ((HOST_WIDE_INT) 1
 			       << floor_log2 (ior_max.high)) - 1;
 	    }
-	  else
+	  else if (ior_max.low != 0)
 	    ior_max.low |= ((unsigned HOST_WIDE_INT) 1u
 			    << floor_log2 (ior_max.low)) - 1;
 
-- 
Alexandre Oliva           http://www.lsd.ic.unicamp.br/~oliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]