diff mbox series

[mid-end] Fix DSE big-endian subreg crash

Message ID 20180815125709.GA31341@arm.com
State New
Headers show
Series [mid-end] Fix DSE big-endian subreg crash | expand

Commit Message

Tamar Christina Aug. 15, 2018, 12:57 p.m. UTC
Hi All,

This patch fixes an ICE that would happen when extract_low_bits
is called with modes for which you can't extract a valid subreg.
e.g. taking a 32 bytes subreg from a 48 byte mode.

The ICE happens because convert_modes which eventually calls
simplify_gen_subreg does not expect the convertion to fail.

The assert in gen_lowpart_general would then be hit.  The patch
changes it to validate the subreg before trying to convert the
modes.  If the subreg is not possible we return NULL_RTX and bail
out early.

I don't have a target independent test for this because it depends
on the target having a 48byte mode and using it for loads.

Cross compiled and regtested on
  aarch64_be-none-elf
and no issues

Boostrapped and regtested
 aarch64-none-linux-gnu

and found no issues.

Bootstrapped on 
 x86_64-pc-linux-gnu
 arm-none-linux-gnueabihf

and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/ChangeLog:

2018-08-15  Tamar Christina  <tamar.christina@arm.com>

	* expmed.c (extract_low_bits): Reject invalid subregs early.

gcc/testsuite/ChangeLog:

2018-08-15  Tamar Christina  <tamar.christina@arm.com>

	* gcc.target/aarch64/large_struct_copy.c: New test.

--

Comments

Jeff Law Aug. 20, 2018, 6:43 p.m. UTC | #1
On 08/15/2018 06:57 AM, Tamar Christina wrote:
> Hi All,
> 
> This patch fixes an ICE that would happen when extract_low_bits
> is called with modes for which you can't extract a valid subreg.
> e.g. taking a 32 bytes subreg from a 48 byte mode.
> 
> The ICE happens because convert_modes which eventually calls
> simplify_gen_subreg does not expect the convertion to fail.
> 
> The assert in gen_lowpart_general would then be hit.  The patch
> changes it to validate the subreg before trying to convert the
> modes.  If the subreg is not possible we return NULL_RTX and bail
> out early.
> 
> I don't have a target independent test for this because it depends
> on the target having a 48byte mode and using it for loads.
> 
> Cross compiled and regtested on
>   aarch64_be-none-elf
> and no issues
> 
> Boostrapped and regtested
>  aarch64-none-linux-gnu
> 
> and found no issues.
> 
> Bootstrapped on 
>  x86_64-pc-linux-gnu
>  arm-none-linux-gnueabihf
> 
> and no issues.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
> 2018-08-15  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* expmed.c (extract_low_bits): Reject invalid subregs early.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-08-15  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc.target/aarch64/large_struct_copy.c: New test.
> 
OK.
jeff
diff mbox series

Patch

diff --git a/gcc/expmed.c b/gcc/expmed.c
index 101e7b88107702b06276cfcd94319d6b79751368..8a1222fa69e88fa7ee6e2c3210d6747d6536de98 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -2392,6 +2392,10 @@  extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
     return NULL_RTX;
 
   src = gen_lowpart (src_int_mode, src);
+  if (!validate_subreg (int_mode, src_int_mode, src,
+			subreg_lowpart_offset (int_mode, src_int_mode)))
+    return NULL_RTX;
+
   src = convert_modes (int_mode, src_int_mode, src, true);
   src = gen_lowpart (mode, src);
   return src;
diff --git a/gcc/testsuite/gcc.target/aarch64/large_struct_copy.c b/gcc/testsuite/gcc.target/aarch64/large_struct_copy.c
new file mode 100644
index 0000000000000000000000000000000000000000..2b5e7801bad0598138cbcee7b2f4ffffaaf438df
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/large_struct_copy.c
@@ -0,0 +1,23 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned __attribute__((mode(DI))) uint64_t;
+
+struct S0 {
+  uint64_t f1;
+  uint64_t f2;
+  uint64_t f3;
+  uint64_t f4;
+  uint64_t f5;
+} a;
+struct S2 {
+  uint64_t f0;
+  uint64_t f2;
+  struct S0 f3;
+};
+
+void fn1 () {
+  struct S2 b = {0, 1, 7, 4073709551611, 4, 8, 7};
+  a = b.f3;
+}
+