| Message ID | 58188275.2080000@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 1, 2016 at 12:54 PM, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi all, > > In this PR the code writes a -1 to a bitfield of size 17 bits and ends up > overwriting another bitfields. > The problem is that the intermediate buffer in encode_tree_to_bitpos holding > the value to merge holds > a 24-bit temporary with -1 written to it i.e. sign-extended to all ones. > That is how native_encode_expr works.This gets then written to > the final buffer (well, a shifted version of it). > > We should instead be truncating the intermediate value to contain zeros in > all the bits that we don't want. > This is already performed for big-endian, this patch just wires it up for > little-endian. > > Bootstrapped and tested on x86_64. > Ok for trunk? Ok. Richard. > Thanks, > Kyrill > > 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR tree-optimization/78170 > * gimple-ssa-store-merging.c (encode_tree_to_bitpos): Truncate padding > introduced by native_encode_expr on little-endian as well. > > 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR tree-optimization/78170 > * gcc.c-torture/execute/pr78170.c: New test.
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c index 5a293d7f30735499aafebeb935b073946eab5691..f82cad35afbc10eea76957d38100acdce137d271 100644 --- a/gcc/gimple-ssa-store-merging.c +++ b/gcc/gimple-ssa-store-merging.c @@ -432,13 +432,23 @@ encode_tree_to_bitpos (tree expr, unsigned char *ptr, int bitlen, int bitpos, contain a sign bit due to sign-extension). */ unsigned int padding = byte_size - ROUND_UP (bitlen, BITS_PER_UNIT) / BITS_PER_UNIT - 1; - if (BYTES_BIG_ENDIAN) + if (padding != 0) { - tmpbuf += padding; + /* On big-endian the padding is at the 'front' so just skip the initial + bytes. */ + if (BYTES_BIG_ENDIAN) + tmpbuf += padding; + byte_size -= padding; if (bitlen % BITS_PER_UNIT != 0) - clear_bit_region_be (tmpbuf, BITS_PER_UNIT - 1, - BITS_PER_UNIT - (bitlen % BITS_PER_UNIT)); + { + if (BYTES_BIG_ENDIAN) + clear_bit_region_be (tmpbuf, BITS_PER_UNIT - 1, + BITS_PER_UNIT - (bitlen % BITS_PER_UNIT)); + else + clear_bit_region (tmpbuf, bitlen, + byte_size * BITS_PER_UNIT - bitlen); + } } /* Clear the bit region in PTR where the bits from TMPBUF will be diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78170.c b/gcc/testsuite/gcc.c-torture/execute/pr78170.c new file mode 100644 index 0000000000000000000000000000000000000000..8ef812ee6accb62db8dd6889d74032a88b784d2c --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr78170.c @@ -0,0 +1,37 @@ +/* PR tree-optimization/78170. + Check that sign-extended store to a bitfield + doesn't overwrite other fields. */ + +int a, b, d; + +struct S0 +{ + int f0; + int f1; + int f2; + int f3; + int f4; + int f5:15; + int f6:17; + int f7:2; + int f8:30; +} c; + +void fn1 () +{ + d = b = 1; + for (; b; b = a) + { + struct S0 e = { 0, 0, 0, 0, 0, 0, 1, 0, 1 }; + c = e; + c.f6 = -1; + } +} + +int main () +{ + fn1 (); + if (c.f7 != 0) + __builtin_abort (); + return 0; +}
Hi all, In this PR the code writes a -1 to a bitfield of size 17 bits and ends up overwriting another bitfields. The problem is that the intermediate buffer in encode_tree_to_bitpos holding the value to merge holds a 24-bit temporary with -1 written to it i.e. sign-extended to all ones. That is how native_encode_expr works.This gets then written to the final buffer (well, a shifted version of it). We should instead be truncating the intermediate value to contain zeros in all the bits that we don't want. This is already performed for big-endian, this patch just wires it up for little-endian. Bootstrapped and tested on x86_64. Ok for trunk? Thanks, Kyrill 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR tree-optimization/78170 * gimple-ssa-store-merging.c (encode_tree_to_bitpos): Truncate padding introduced by native_encode_expr on little-endian as well. 2016-11-01 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR tree-optimization/78170 * gcc.c-torture/execute/pr78170.c: New test.