Patchwork Fix undefined behaviour in combine.c:force_to_mode

login
register
mail settings
Submitter Ulrich Weigand
Date July 6, 2012, 1:11 p.m.
Message ID <201207061311.q66DBHwE028422@d06av02.portsmouth.uk.ibm.com>
Download mbox | patch
Permalink /patch/169472/
State New
Headers show

Comments

Ulrich Weigand - July 6, 2012, 1:11 p.m.
Hello,

in testing a patch on arm-linux-gnueabihf, I ran into a bootstrap comparison
failure that turned out to be caused by a pre-existing bug in common code,
where combine.c code exposed undefined behaviour.

The problem is this code in force_to_mode when simplifying a LSHIFTRT:

      /* Here we can only do something if the shift count is a constant,
         this shift constant is valid for the host, and we can do arithmetic
         in OP_MODE.  */

      if (CONST_INT_P (XEXP (x, 1))
          && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT
          && HWI_COMPUTABLE_MODE_P (op_mode))
        {
          rtx inner = XEXP (x, 0);
          unsigned HOST_WIDE_INT inner_mask;

          /* Select the mask of the bits we need for the shift operand.  */
          inner_mask = mask << INTVAL (XEXP (x, 1));


If the source code being compiled contains a shift by a negative constant
shift amount, INTVAL (XEXP (x, 1)) at this point can be negative.  (Note
that this does not necessarily mean that the original program has
undefined behaviour, since that shift could be in a place that is never
actually executed.)

If this is the case, then the last line above will cause undefined
behaviour of GCC itself.

Note that most other places in force_to_mode optimizing shifts already
check for non-negative shift amounts; but in this place the check is
missing.

The following patch adds the check here as well, fixing the undefined
behaviour (and subsequent bootstrap comparison failure) in my test.

Tested on arm-linux-gnueabihf.  OK for mainline?

Bye,
Ulrich

ChangeLog:

	* combine.c (force_to_mode) [LSHIFTRT]: Avoid undefined behaviour
	due to negative shift amount.
Eric Botcazou - July 6, 2012, 5:27 p.m.
> Note that most other places in force_to_mode optimizing shifts already
> check for non-negative shift amounts; but in this place the check is
> missing.
>
> The following patch adds the check here as well, fixing the undefined
> behaviour (and subsequent bootstrap comparison failure) in my test.
>
> Tested on arm-linux-gnueabihf.  OK for mainline?

Sure, but given that there is indeed the same pattern a few lines above, you 
could as well have installed it as obvious.  OK for 4.7 too if you need it.

Patch

=== modified file 'gcc/combine.c'
--- gcc/combine.c	2012-02-22 12:22:43 +0000
+++ gcc/combine.c	2012-07-03 19:46:18 +0000
@@ -8432,6 +8432,7 @@ 
 	 in OP_MODE.  */
 
       if (CONST_INT_P (XEXP (x, 1))
+	  && INTVAL (XEXP (x, 1)) >= 0
 	  && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT
 	  && HWI_COMPUTABLE_MODE_P (op_mode))
 	{