[patch] Fix wrong code with small structure return on PowerPC

[Eric Botcazou]

Hi,

this is a bug originally reported in Ada on 32-bit PowerPC with the SVR4 ABI 
(hence not Linux) and reproducible in C with the attached testcase at -O1.

The problem lies in function 'foo':

  struct S ret;
  char r, s, c1, c2;
  char *p = &r;

  s = bar (&p);
  if (s)
    c2 = *p;
  c1 = 0;

  ret.c1 = c1;
  ret.c2 = c2;
  return ret;

Since the call to bar returns 0, c2 is uninitialized at run time (but this 
doesn't matter for correctness since its value is never read).  Now both c1 
and c2 are represented at the RTL level by unsigned promoted subregs, i.e. 
SUBREGs verifying SUBREG_PROMOTED_VAR_P and SUBREG_PROMOTED_UNSIGNED_P

As a consequence, when store_fixed_bit_field_1 stores the 8-bit values into 
the 'ret' object, it considers that the underlying 32-bit objects have only 8 
bits set and the rest cleared so it doesn't mask the other 24 bits.  That's 
OK for c1 but not for c2, since c2 is uninitialized so the bits are random.

This appears to be an inherent weakness of the promoted subregs mechanism, but 
I don't think that we want to go for an upheaval at this point.  So the patch 
addresses the problem in an ad-hoc manner directly in store_fixed_bit_field_1 
and yields the expected 8-bit insertion:

@@ -26,7 +26,7 @@
        lwz 9,12(1)
        lbz 31,0(9)
 .L3:
-       slwi 3,31,16
+       rlwinm 3,31,16,8,15
        lwz 0,36(1)
        mtlr 0
        lwz 31,28(1)

Tested on x86-64/Linux and PowerPC64/Linux, OK for the mainline?


2017-09-11  Eric Botcazou  <ebotca...@adacore.com>

        * expmed.c (store_fixed_bit_field_1): Force the masking if the value
        is an unsigned promoted SUBREG of a sufficiently large object.


2017-09-11  Eric Botcazou  <ebotca...@adacore.com>

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

-- 
Eric Botcazou
Index: expmed.c
===================================================================
--- expmed.c	(revision 251906)
+++ expmed.c	(working copy)
@@ -1213,13 +1213,25 @@ store_fixed_bit_field_1 (rtx op0, scalar
     }
   else
     {
-      int must_and = (GET_MODE_BITSIZE (value_mode) != bitsize
-		      && bitnum + bitsize != GET_MODE_BITSIZE (mode));
+      /* Compute whether we must mask the value in order to make sure that the
+	 bits outside the bitfield are all cleared.  Note that, even though the
+	 value has the right size, if it has a mode different from MODE, then
+	 converting it to MODE may unmask uninitialized bits in the case where
+	 it is an unsigned promoted SUBREG of a sufficiently large object.  */
+      const bool must_mask
+	= (GET_MODE_BITSIZE (value_mode) != bitsize
+	   || (value_mode != mode
+	       && GET_CODE (value) == SUBREG
+	       && SUBREG_PROMOTED_VAR_P (value)
+	       && SUBREG_PROMOTED_UNSIGNED_P (value)
+	       && GET_MODE_SIZE (GET_MODE (SUBREG_REG (value)))
+		  >= GET_MODE_SIZE (mode)))
+	  && bitnum + bitsize != GET_MODE_BITSIZE (mode);
 
       if (value_mode != mode)
 	value = convert_to_mode (mode, value, 1);
 
-      if (must_and)
+      if (must_mask)
 	value = expand_binop (mode, and_optab, value,
 			      mask_rtx (mode, 0, bitsize, 0),
 			      NULL_RTX, 1, OPTAB_LIB_WIDEN);

unsigned long foo (int x)
{
  return x > 0 ? (unsigned long) x : 0;
}

unsigned long bar (int x, int y)
{
  return x > y ? (unsigned long) x : (unsigned long) y;
}