[PATCH] PR 16790: extract_muldiv losing a sign extension

[Roger Sayle <roger at eyesopen dot com>]

The following patch is my proposed solution to PR middle-end/16790,
which is a nasty invalid code regression present in gcc 3.2 onwards.

The problem is that extract_muldiv is incorrectly transforming the
expression "(int)(short)(u*2) * 3" into "(int)u * 6", where u is an
unsigned integer.  As demonstrated in the PR, this is unsafe for the
value 0x4000 which should get sign extended by the short->int conversion.

The logic in extract_muldiv is a labyrinth of twisty turny passages,
but I believe the correct fix is the one character change below.  At the
problematic recursive invocation of extract_multdiv, wider_type (the
outer type) is "int", type is "short" and TREE_TYPE (op0) is "unsigned
int".  These values lead to ctype being "int" (the wider of wider_type
and type).  The code that guards against truncations is comparing the
widths of ctype and the inner-type, i.e. TREE_TYPE (op0), which in this
case are both the same width, four bytes, hence the truncation goes
unnoticed and the transformation allowed.

The change below tweaks the test for truncations to compare the current
type, type, against the inner type, to spot that this is a local
truncation, i.e. that this particular NOP_EXPR or CONVERT_EXPR is a
integer truncation.  This should always be safe as "type" is guaranteed
to be narrower than (or equal to) "ctype".  In which case, we'll catch
a superset of the truncations that we found previously.  This change is
"fail safe" as being caught by this condition simply disables this
potentially problematic transformation.

Hopefully, the above makes some sense.

It might even be beneficial to split extract_muldiv into extract_mul
and extract_div so that the semantics of multiplication and division
operators are clearly separated?


The following code has been tested on i686-pc-linux-gnu with a full
"make bootstrap", all default languages, and regression tested with a
top-level "make -k check" with no new failures.

Ok for mainline, 3.4 and 3.3?


2004-08-01  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/16790
	* fold-const.c (extract_muldiv_1) <NOP_EXPR>: Disallow local
	truncations, not just global truncations.

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


Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.430
diff -c -3 -p -r1.430 fold-const.c
*** fold-const.c	25 Jul 2004 23:26:59 -0000	1.430
--- fold-const.c	2 Aug 2004 00:49:31 -0000
*************** extract_muldiv_1 (tree t, tree c, enum t
*** 5103,5111 ****
  		     && TYPE_IS_SIZETYPE (TREE_TYPE (op0)))
  	       && (GET_MODE_SIZE (TYPE_MODE (ctype))
  	           > GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))
! 	      /* ... or its type is larger than ctype,
! 		 then we cannot pass through this truncation.  */
! 	      || (GET_MODE_SIZE (TYPE_MODE (ctype))
  		  < GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))))
  	      /* ... or signedness changes for division or modulus,
  		 then we cannot pass through this conversion.  */
--- 5103,5111 ----
  		     && TYPE_IS_SIZETYPE (TREE_TYPE (op0)))
  	       && (GET_MODE_SIZE (TYPE_MODE (ctype))
  	           > GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))
! 	      /* ... or this is a truncation (t is narrower than op0),
! 		 then we cannot pass through this narrowing.  */
! 	      || (GET_MODE_SIZE (TYPE_MODE (type))
  		  < GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))))
  	      /* ... or signedness changes for division or modulus,
  		 then we cannot pass through this conversion.  */

/* PR middle-end/16790.  */

extern void abort ();

static void test1(unsigned int u1)
{
  unsigned int y_final_1;
  signed short y_middle;
  unsigned int y_final_2;

  y_final_1 = (unsigned int)( (signed short)(u1 * 2) * 3 );
  y_middle  =                 (signed short)(u1 * 2);
  y_final_2 = (unsigned int)( y_middle * 3 );

  if (y_final_1 != y_final_2)
    abort ();
}


static void test2(unsigned int u1)
{
  unsigned int y_final_1;
  signed short y_middle;
  unsigned int y_final_2;

  y_final_1 = (unsigned int)( (signed short)(u1 << 1) * 3 );
  y_middle  =                 (signed short)(u1 << 1);
  y_final_2 = (unsigned int)( y_middle * 3 );

  if (y_final_1 != y_final_2)
    abort ();
}


int main()
{
  test1(0x4000U);
  test2(0x4000U);
  return 0;
}


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833