diff mbox series

[AArch64] Cleanup the AArch64 testsuite when stack-clash is on [Patch (6/6)]

Message ID 20180711112310.GA15972@arm.com
State New
Headers show
Series [AArch64] Cleanup the AArch64 testsuite when stack-clash is on [Patch (6/6)] | expand

Commit Message

Tamar Christina July 11, 2018, 11:23 a.m. UTC
Hi All,

This patch cleans up the testsuite when a run is done with stack clash
protection turned on.

Concretely this switches off -fstack-clash-protection for a couple of tests:

* sve: We don't yet support stack-clash-protection and sve, so for now turn these off.
* assembler scan: some tests are quite fragile in that they check for exact
       assembly output, e.g. check for exact amount of sub etc.  These won't
       match now.
* vla: Some of the ubsan tests negative array indices. Because the arrays weren't
       used before the incorrect $sp wouldn't have been used. The correct value is
       restored on ret.  Now however we probe the $sp which causes a segfault.
* params: When testing the parameters we have to skip these on AArch64 because of our
          custom constraints on them.  We already test them separately so this isn't a
          loss.

Note that the testsuite is not entire clean due to gdb failure caused by alloca with
stack clash. On AArch64 we output an incorrect .loc directive, but this is already the
case with the current implementation in GCC and is a bug unrelated to this patch series.

Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu and no issues.
Both targets were tested with stack clash on and off by default.

Ok for trunk?

Thanks,
Tamar

gcc/testsuite/
2018-07-11  Tamar Christina  <tamar.christina@arm.com>

	PR target/86486
	gcc.dg/pr82788.c: Skip for AArch64.
	gcc.dg/guality/vla-1.c: Turn off stack-clash.
	gcc.target/aarch64/subsp.c: Likewise.
	gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
	gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
	gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
	gcc.dg/params/blocksort-part.c: Skip stack-clash checks
	on AArch64.

--

Comments

Tamar Christina July 24, 2018, 10:28 a.m. UTC | #1
Hi All,

This patch cleans up the testsuite when a run is done with stack clash
protection turned on.

Concretely this switches off -fstack-clash-protection for a couple of tests:

* sve: We don't yet support stack-clash-protection and sve, so for now turn these off.
* assembler scan: some tests are quite fragile in that they check for exact
       assembly output, e.g. check for exact amount of sub etc.  These won't
       match now.
* vla: Some of the ubsan tests negative array indices. Because the arrays weren't
       used before the incorrect $sp wouldn't have been used. The correct value is
       restored on ret.  Now however we probe the $sp which causes a segfault.
* params: When testing the parameters we have to skip these on AArch64 because of our
          custom constraints on them.  We already test them separately so this isn't a
          loss.

Note that the testsuite is not entire clean due to gdb failure caused by alloca with
stack clash. On AArch64 we output an incorrect .loc directive, but this is already the
case with the current implementation in GCC and is a bug unrelated to this patch series.

Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu and no issues.
Both targets were tested with stack clash on and off by default.

Ok for trunk?

Thanks,
Tamar

gcc/testsuite/
2018-07-24  Tamar Christina  <tamar.christina@arm.com>

	PR target/86486
	* gcc.dg/pr82788.c: Skip for AArch64.
	* gcc.dg/guality/vla-1.c: Turn off stack-clash.
	* gcc.target/aarch64/subsp.c: Likewise.
	* gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
	* gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
	* gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
	* gcc.dg/params/blocksort-part.c: Skip stack-clash checks
	on AArch64.
	* gcc.dg/stack-check-10.c: Add AArch64 specific checks.
	* gcc.dg/stack-check-5.c: Add AArch64 specific checks.
	* gcc.dg/stack-check-6a.c: Skip on AArch64, we don't support this.
	* testsuite/lib/target-supports.exp
	(check_effective_target_frame_pointer_for_non_leaf): AArch64 does not
	require frame pointer for non-leaf functions.

> -----Original Message-----
> From: Tamar Christina <tamar.christina@arm.com>
> Sent: Wednesday, July 11, 2018 12:23
> To: gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>;
> Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>
> Subject: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when stack-
> clash is on [Patch (6/6)]
> 
> Hi All,
> 
> This patch cleans up the testsuite when a run is done with stack clash
> protection turned on.
> 
> Concretely this switches off -fstack-clash-protection for a couple of tests:
> 
> * sve: We don't yet support stack-clash-protection and sve, so for now turn
> these off.
> * assembler scan: some tests are quite fragile in that they check for exact
>        assembly output, e.g. check for exact amount of sub etc.  These won't
>        match now.
> * vla: Some of the ubsan tests negative array indices. Because the arrays
> weren't
>        used before the incorrect $sp wouldn't have been used. The correct
> value is
>        restored on ret.  Now however we probe the $sp which causes a segfault.
> * params: When testing the parameters we have to skip these on AArch64
> because of our
>           custom constraints on them.  We already test them separately so this
> isn't a
>           loss.
> 
> Note that the testsuite is not entire clean due to gdb failure caused by alloca
> with stack clash. On AArch64 we output an incorrect .loc directive, but this is
> already the case with the current implementation in GCC and is a bug
> unrelated to this patch series.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no issues.
> Both targets were tested with stack clash on and off by default.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> gcc/testsuite/
> 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/86486
> 	gcc.dg/pr82788.c: Skip for AArch64.
> 	gcc.dg/guality/vla-1.c: Turn off stack-clash.
> 	gcc.target/aarch64/subsp.c: Likewise.
> 	gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> 	gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> 	gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> 	gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> 	on AArch64.
> 
> --
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
index 52ade3aab7566dce3ca7ef931ac65895005d5e13..c97465edae195442a71ee66ab25015a2ac4fc8fc 100644
--- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable -fno-stack-clash-protection" } */
 
 typedef long int V;
 int x = -1;
diff --git a/gcc/testsuite/gcc.dg/params/blocksort-part.c b/gcc/testsuite/gcc.dg/params/blocksort-part.c
index a9154f2e61ccd21b60153f20be3891b988f9ef2c..1e677878e7bd9c68b026f8c72b0de9f01e15459c 100644
--- a/gcc/testsuite/gcc.dg/params/blocksort-part.c
+++ b/gcc/testsuite/gcc.dg/params/blocksort-part.c
@@ -1,3 +1,4 @@
+/* { dg-skip-if "AArch64 does not support these bounds." { aarch64*-*-* } { "--param stack-clash-protection-*" } } */
 
 /*-------------------------------------------------------------*/
 /*--- Block sorting machinery                               ---*/
diff --git a/gcc/testsuite/gcc.dg/pr82788.c b/gcc/testsuite/gcc.dg/pr82788.c
index a8f628fd7f66c3e56739f6ff491df38b23f4d4df..41c442f61a625c8b350e1e4c870a98d86b167031 100644
--- a/gcc/testsuite/gcc.dg/pr82788.c
+++ b/gcc/testsuite/gcc.dg/pr82788.c
@@ -1,4 +1,5 @@
 /* { dg-do run } */
 /* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-probe-interval=10 --param stack-clash-protection-guard-size=12" } */
 /* { dg-require-effective-target supports_stack_clash_protection } */
+/* { dg-skip-if "AArch64 does not support this interval." { aarch64*-*-* } } */
 int main() { int a[1442]; return 0;}
diff --git a/gcc/testsuite/gcc.dg/stack-check-10.c b/gcc/testsuite/gcc.dg/stack-check-10.c
index a86956ad6925464e4a938a33e609fae5004201c7..2f5a090cb7a4ed6d2e6e4317492150a348a326ab 100644
--- a/gcc/testsuite/gcc.dg/stack-check-10.c
+++ b/gcc/testsuite/gcc.dg/stack-check-10.c
@@ -39,3 +39,4 @@ f3 (void)
    need a frame pointer.  Otherwise neither should.  */
 /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 2 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf } } } } */
 /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
+/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack adjustment in prologue" 2 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
diff --git a/gcc/testsuite/gcc.dg/stack-check-5.c b/gcc/testsuite/gcc.dg/stack-check-5.c
index 604fa3cf6c5b502f74c1e3497b3b8d72a15bb3ea..0243147939c10e8f4632520b12714724af85b332 100644
--- a/gcc/testsuite/gcc.dg/stack-check-5.c
+++ b/gcc/testsuite/gcc.dg/stack-check-5.c
@@ -66,7 +66,9 @@ f3 (void)
 /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 4 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf } } } } */
 /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
 /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
-
+/* AArch64 won't require a probe here due to the allocation amount being less than 1KB.  */
+/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack adjustment in prologue" 3 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
+/* { dg-final { scan-rtl-dump-times "Stack clash no probe no stack adjustment in prologue" 1 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
 
 /* We have selected the size of the array in f2/f3 to be large enough
    to not live in the red zone on targets that support it.
diff --git a/gcc/testsuite/gcc.dg/stack-check-6a.c b/gcc/testsuite/gcc.dg/stack-check-6a.c
index 8fb9c621585957a85877ebadfbc4a8daabe4311c..68dd9bc48a0c26ecb84ddd2c09b8aa74d3276695 100644
--- a/gcc/testsuite/gcc.dg/stack-check-6a.c
+++ b/gcc/testsuite/gcc.dg/stack-check-6a.c
@@ -5,6 +5,7 @@
 /* { dg-options "-O2 -fstack-clash-protection -fdump-rtl-pro_and_epilogue -fno-optimize-sibling-calls --param stack-clash-protection-probe-interval=12 --param stack-clash-protection-guard-size=16" } */
 /* { dg-require-effective-target supports_stack_clash_protection  } */
 /* { dg-skip-if "" { *-*-* } { "-fstack-protector*" } { "" } } */
+/* { dg-skip-if "" { aarch64*-*-* } } */
 
 
 #include "stack-check-6.c"
diff --git a/gcc/testsuite/gcc.target/aarch64/subsp.c b/gcc/testsuite/gcc.target/aarch64/subsp.c
index 70d848c59d1f1e4df4314ca012c7a5d9d3b91ebc..6ef6b2c90ae694055749a94b68cbba5ee4aea882 100644
--- a/gcc/testsuite/gcc.target/aarch64/subsp.c
+++ b/gcc/testsuite/gcc.target/aarch64/subsp.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O" } */
+/* { dg-options "-O -fno-stack-clash-protection" } */
 
 int foo (void *);
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
index 29702ab55f249c3ebd0baf44981870524098e1e4..baeec61bb59aff56f0dcc20fc6ec6b93d517490e 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
index 001f5be8ff58bfcc75eccc4c050bef1e53faffeb..eae3be7a7b24dc124f7c1c26a97fb25400cc62d2 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
index 59e9ee49c4a214b731ed1975da0dcfa46c059f8b..ce9825f73e8495a7f5c1f4ab1d5bf1aaf5035e17 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7ec350213e9225ad342d030afac30bd613851aa1..5624f361c59b97cb967b3485adc8db01da048a84 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9280,10 +9280,6 @@ proc check_effective_target_supports_stack_clash_protection { } {
 # Return 1 if the target creates a frame pointer for non-leaf functions
 # Note we ignore cases where we apply tail call optimization here.
 proc check_effective_target_frame_pointer_for_non_leaf { } {
-  if { [istarget aarch*-*-*] } {
-	return 1
-  }
-
   # Solaris/x86 defaults to -fno-omit-frame-pointer.
   if { [istarget i?86-*-solaris*] || [istarget x86_64-*-solaris*] } {
     return 1
James Greenhalgh July 31, 2018, 9:07 p.m. UTC | #2
On Tue, Jul 24, 2018 at 05:28:03AM -0500, Tamar Christina wrote:
> Hi All,
> 
> This patch cleans up the testsuite when a run is done with stack clash
> protection turned on.
> 
> Concretely this switches off -fstack-clash-protection for a couple of tests:
> 
> * sve: We don't yet support stack-clash-protection and sve, so for now turn these off.
> * assembler scan: some tests are quite fragile in that they check for exact
>        assembly output, e.g. check for exact amount of sub etc.  These won't
>        match now.
> * vla: Some of the ubsan tests negative array indices. Because the arrays weren't
>        used before the incorrect $sp wouldn't have been used. The correct value is
>        restored on ret.  Now however we probe the $sp which causes a segfault.
> * params: When testing the parameters we have to skip these on AArch64 because of our
>           custom constraints on them.  We already test them separately so this isn't a
>           loss.
> 
> Note that the testsuite is not entire clean due to gdb failure caused by alloca with
> stack clash. On AArch64 we output an incorrect .loc directive, but this is already the
> case with the current implementation in GCC and is a bug unrelated to this patch series.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu and no issues.
> Both targets were tested with stack clash on and off by default.
> 
> Ok for trunk?

For each of the generic tests you skip because of mismatched bounds, I think
we should ensure we have an equivalent test checking that behaviour in
gcc.target/aarch64/ . If we have that, it might be good to cross-reference
them with a comment above your skip lines.

> * vla: Some of the ubsan tests negative array indices. Because the arrays weren't
>        used before the incorrect $sp wouldn't have been used. The correct value is
>        restored on ret.  Now however we probe the $sp which causes a segfault.

This is interesting behaviour; is it a desirable side effect of your changes?

Otherwise, this patch is OK.

Thanks,
James


> gcc/testsuite/
> 2018-07-24  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/86486
> 	* gcc.dg/pr82788.c: Skip for AArch64.
> 	* gcc.dg/guality/vla-1.c: Turn off stack-clash.
> 	* gcc.target/aarch64/subsp.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> 	* gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> 	on AArch64.
> 	* gcc.dg/stack-check-10.c: Add AArch64 specific checks.
> 	* gcc.dg/stack-check-5.c: Add AArch64 specific checks.
> 	* gcc.dg/stack-check-6a.c: Skip on AArch64, we don't support this.
> 	* testsuite/lib/target-supports.exp
> 	(check_effective_target_frame_pointer_for_non_leaf): AArch64 does not
> 	require frame pointer for non-leaf functions.
> 
> > -----Original Message-----
> > From: Tamar Christina <tamar.christina@arm.com>
> > Sent: Wednesday, July 11, 2018 12:23
> > To: gcc-patches@gcc.gnu.org
> > Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>;
> > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> > <Marcus.Shawcroft@arm.com>
> > Subject: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when stack-
> > clash is on [Patch (6/6)]
> > 
> > Hi All,
> > 
> > This patch cleans up the testsuite when a run is done with stack clash
> > protection turned on.
> > 
> > Concretely this switches off -fstack-clash-protection for a couple of tests:
> > 
> > * sve: We don't yet support stack-clash-protection and sve, so for now turn
> > these off.
> > * assembler scan: some tests are quite fragile in that they check for exact
> >        assembly output, e.g. check for exact amount of sub etc.  These won't
> >        match now.
> > * vla: Some of the ubsan tests negative array indices. Because the arrays
> > weren't
> >        used before the incorrect $sp wouldn't have been used. The correct
> > value is
> >        restored on ret.  Now however we probe the $sp which causes a segfault.
> > * params: When testing the parameters we have to skip these on AArch64
> > because of our
> >           custom constraints on them.  We already test them separately so this
> > isn't a
> >           loss.
> > 
> > Note that the testsuite is not entire clean due to gdb failure caused by alloca
> > with stack clash. On AArch64 we output an incorrect .loc directive, but this is
> > already the case with the current implementation in GCC and is a bug
> > unrelated to this patch series.
> > 
> > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> > and no issues.
> > Both targets were tested with stack clash on and off by default.
> > 
> > Ok for trunk?
> > 
> > Thanks,
> > Tamar
> > 
> > gcc/testsuite/
> > 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
> > 
> > 	PR target/86486
> > 	gcc.dg/pr82788.c: Skip for AArch64.
> > 	gcc.dg/guality/vla-1.c: Turn off stack-clash.
> > 	gcc.target/aarch64/subsp.c: Likewise.
> > 	gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> > 	gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> > 	gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> > 	gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> > 	on AArch64.
> > 
> > --

> diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> index 52ade3aab7566dce3ca7ef931ac65895005d5e13..c97465edae195442a71ee66ab25015a2ac4fc8fc 100644
> --- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> +++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> @@ -1,5 +1,5 @@
>  /* { dg-do run } */
> -/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
> +/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable -fno-stack-clash-protection" } */
>  
>  typedef long int V;
>  int x = -1;
> diff --git a/gcc/testsuite/gcc.dg/params/blocksort-part.c b/gcc/testsuite/gcc.dg/params/blocksort-part.c
> index a9154f2e61ccd21b60153f20be3891b988f9ef2c..1e677878e7bd9c68b026f8c72b0de9f01e15459c 100644
> --- a/gcc/testsuite/gcc.dg/params/blocksort-part.c
> +++ b/gcc/testsuite/gcc.dg/params/blocksort-part.c
> @@ -1,3 +1,4 @@
> +/* { dg-skip-if "AArch64 does not support these bounds." { aarch64*-*-* } { "--param stack-clash-protection-*" } } */
>  
>  /*-------------------------------------------------------------*/
>  /*--- Block sorting machinery                               ---*/
> diff --git a/gcc/testsuite/gcc.dg/pr82788.c b/gcc/testsuite/gcc.dg/pr82788.c
> index a8f628fd7f66c3e56739f6ff491df38b23f4d4df..41c442f61a625c8b350e1e4c870a98d86b167031 100644
> --- a/gcc/testsuite/gcc.dg/pr82788.c
> +++ b/gcc/testsuite/gcc.dg/pr82788.c
> @@ -1,4 +1,5 @@
>  /* { dg-do run } */
>  /* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-probe-interval=10 --param stack-clash-protection-guard-size=12" } */
>  /* { dg-require-effective-target supports_stack_clash_protection } */
> +/* { dg-skip-if "AArch64 does not support this interval." { aarch64*-*-* } } */
>  int main() { int a[1442]; return 0;}
> diff --git a/gcc/testsuite/gcc.dg/stack-check-10.c b/gcc/testsuite/gcc.dg/stack-check-10.c
> index a86956ad6925464e4a938a33e609fae5004201c7..2f5a090cb7a4ed6d2e6e4317492150a348a326ab 100644
> --- a/gcc/testsuite/gcc.dg/stack-check-10.c
> +++ b/gcc/testsuite/gcc.dg/stack-check-10.c
> @@ -39,3 +39,4 @@ f3 (void)
>     need a frame pointer.  Otherwise neither should.  */
>  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 2 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf } } } } */
>  /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
> +/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack adjustment in prologue" 2 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
> diff --git a/gcc/testsuite/gcc.dg/stack-check-5.c b/gcc/testsuite/gcc.dg/stack-check-5.c
> index 604fa3cf6c5b502f74c1e3497b3b8d72a15bb3ea..0243147939c10e8f4632520b12714724af85b332 100644
> --- a/gcc/testsuite/gcc.dg/stack-check-5.c
> +++ b/gcc/testsuite/gcc.dg/stack-check-5.c
> @@ -66,7 +66,9 @@ f3 (void)
>  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 4 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf } } } } */
>  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
>  /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } } } } */
> -
> +/* AArch64 won't require a probe here due to the allocation amount being less than 1KB.  */
> +/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack adjustment in prologue" 3 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
> +/* { dg-final { scan-rtl-dump-times "Stack clash no probe no stack adjustment in prologue" 1 "pro_and_epilogue" { target { aarch64*-*-* } } } } */
>  
>  /* We have selected the size of the array in f2/f3 to be large enough
>     to not live in the red zone on targets that support it.
> diff --git a/gcc/testsuite/gcc.dg/stack-check-6a.c b/gcc/testsuite/gcc.dg/stack-check-6a.c
> index 8fb9c621585957a85877ebadfbc4a8daabe4311c..68dd9bc48a0c26ecb84ddd2c09b8aa74d3276695 100644
> --- a/gcc/testsuite/gcc.dg/stack-check-6a.c
> +++ b/gcc/testsuite/gcc.dg/stack-check-6a.c
> @@ -5,6 +5,7 @@
>  /* { dg-options "-O2 -fstack-clash-protection -fdump-rtl-pro_and_epilogue -fno-optimize-sibling-calls --param stack-clash-protection-probe-interval=12 --param stack-clash-protection-guard-size=16" } */
>  /* { dg-require-effective-target supports_stack_clash_protection  } */
>  /* { dg-skip-if "" { *-*-* } { "-fstack-protector*" } { "" } } */
> +/* { dg-skip-if "" { aarch64*-*-* } } */
>  
>  
>  #include "stack-check-6.c"
> diff --git a/gcc/testsuite/gcc.target/aarch64/subsp.c b/gcc/testsuite/gcc.target/aarch64/subsp.c
> index 70d848c59d1f1e4df4314ca012c7a5d9d3b91ebc..6ef6b2c90ae694055749a94b68cbba5ee4aea882 100644
> --- a/gcc/testsuite/gcc.target/aarch64/subsp.c
> +++ b/gcc/testsuite/gcc.target/aarch64/subsp.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-O" } */
> +/* { dg-options "-O -fno-stack-clash-protection" } */
>  
>  int foo (void *);
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> index 29702ab55f249c3ebd0baf44981870524098e1e4..baeec61bb59aff56f0dcc20fc6ec6b93d517490e 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> +/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
>  
>  #include <stdint.h>
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> index 001f5be8ff58bfcc75eccc4c050bef1e53faffeb..eae3be7a7b24dc124f7c1c26a97fb25400cc62d2 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> +/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
>  
>  #include <stdint.h>
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> index 59e9ee49c4a214b731ed1975da0dcfa46c059f8b..ce9825f73e8495a7f5c1f4ab1d5bf1aaf5035e17 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> +/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
>  
>  #include <stdint.h>
>  
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 7ec350213e9225ad342d030afac30bd613851aa1..5624f361c59b97cb967b3485adc8db01da048a84 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -9280,10 +9280,6 @@ proc check_effective_target_supports_stack_clash_protection { } {
>  # Return 1 if the target creates a frame pointer for non-leaf functions
>  # Note we ignore cases where we apply tail call optimization here.
>  proc check_effective_target_frame_pointer_for_non_leaf { } {
> -  if { [istarget aarch*-*-*] } {
> -	return 1
> -  }
> -
>    # Solaris/x86 defaults to -fno-omit-frame-pointer.
>    if { [istarget i?86-*-solaris*] || [istarget x86_64-*-solaris*] } {
>      return 1
>
Tamar Christina Aug. 3, 2018, 4:03 p.m. UTC | #3
Hi James,

Thanks for the review.

> -----Original Message-----
> From: James Greenhalgh <james.greenhalgh@arm.com>
> Sent: Tuesday, July 31, 2018 22:07
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>
> Subject: Re: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when
> stack-clash is on [Patch (6/6)]
> 
> On Tue, Jul 24, 2018 at 05:28:03AM -0500, Tamar Christina wrote:
> > Hi All,
> >
> > This patch cleans up the testsuite when a run is done with stack clash
> > protection turned on.
> >
> > Concretely this switches off -fstack-clash-protection for a couple of tests:
> >
> > * sve: We don't yet support stack-clash-protection and sve, so for now turn
> these off.
> > * assembler scan: some tests are quite fragile in that they check for exact
> >        assembly output, e.g. check for exact amount of sub etc.  These won't
> >        match now.
> > * vla: Some of the ubsan tests negative array indices. Because the arrays
> weren't
> >        used before the incorrect $sp wouldn't have been used. The correct
> value is
> >        restored on ret.  Now however we probe the $sp which causes a
> segfault.
> > * params: When testing the parameters we have to skip these on AArch64
> because of our
> >           custom constraints on them.  We already test them separately so this
> isn't a
> >           loss.
> >
> > Note that the testsuite is not entire clean due to gdb failure caused
> > by alloca with stack clash. On AArch64 we output an incorrect .loc
> > directive, but this is already the case with the current implementation in
> GCC and is a bug unrelated to this patch series.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no issues.
> > Both targets were tested with stack clash on and off by default.
> >
> > Ok for trunk?
> 
> For each of the generic tests you skip because of mismatched bounds, I think
> we should ensure we have an equivalent test checking that behaviour in
> gcc.target/aarch64/ . If we have that, it might be good to cross-reference
> them with a comment above your skip lines.

The problem is, fundamentally what these two tests are trying to test we don't support.
pr82788.c is explicitly checking for a bug that happened when you have a 1KB probe interval
with a 4KB guard-size.

And stack-check-6a.c is checking that increasing the guard size with smaller probe interval reduces
the amount of probing you would have to do. 

Both these cases we can't support as we force stack guard to be the same as probing interval.  I can't
think of any equivalent AArch64 test as these class of failures just don't happen for us.

> 
> > * vla: Some of the ubsan tests negative array indices. Because the arrays
> weren't
> >        used before the incorrect $sp wouldn't have been used. The correct
> value is
> >        restored on ret.  Now however we probe the $sp which causes a
> segfault.
> 
> This is interesting behaviour; is it a desirable side effect of your changes?

They should fail for every correct implementation of stack clash protection.  The programs were always invalid.
These test could probably be made to work by allocating some number of stack space before the negative
Index arrays.  They wouldn't crash then as the negative indices would still keep you within your stack space,
but that's arguably a different test then.

> 
> Otherwise, this patch is OK.
> 
> Thanks,
> James
> 
> 
> > gcc/testsuite/
> > 2018-07-24  Tamar Christina  <tamar.christina@arm.com>
> >
> > 	PR target/86486
> > 	* gcc.dg/pr82788.c: Skip for AArch64.
> > 	* gcc.dg/guality/vla-1.c: Turn off stack-clash.
> > 	* gcc.target/aarch64/subsp.c: Likewise.
> > 	* gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> > 	* gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> > 	* gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> > 	* gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> > 	on AArch64.
> > 	* gcc.dg/stack-check-10.c: Add AArch64 specific checks.
> > 	* gcc.dg/stack-check-5.c: Add AArch64 specific checks.
> > 	* gcc.dg/stack-check-6a.c: Skip on AArch64, we don't support this.
> > 	* testsuite/lib/target-supports.exp
> > 	(check_effective_target_frame_pointer_for_non_leaf): AArch64
> does not
> > 	require frame pointer for non-leaf functions.
> >
> > > -----Original Message-----
> > > From: Tamar Christina <tamar.christina@arm.com>
> > > Sent: Wednesday, July 11, 2018 12:23
> > > To: gcc-patches@gcc.gnu.org
> > > Cc: nd <nd@arm.com>; James Greenhalgh
> <James.Greenhalgh@arm.com>;
> > > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> > > <Marcus.Shawcroft@arm.com>
> > > Subject: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when
> > > stack- clash is on [Patch (6/6)]
> > >
> > > Hi All,
> > >
> > > This patch cleans up the testsuite when a run is done with stack
> > > clash protection turned on.
> > >
> > > Concretely this switches off -fstack-clash-protection for a couple of tests:
> > >
> > > * sve: We don't yet support stack-clash-protection and sve, so for
> > > now turn these off.
> > > * assembler scan: some tests are quite fragile in that they check for exact
> > >        assembly output, e.g. check for exact amount of sub etc.  These won't
> > >        match now.
> > > * vla: Some of the ubsan tests negative array indices. Because the
> > > arrays weren't
> > >        used before the incorrect $sp wouldn't have been used. The
> > > correct value is
> > >        restored on ret.  Now however we probe the $sp which causes a
> segfault.
> > > * params: When testing the parameters we have to skip these on
> > > AArch64 because of our
> > >           custom constraints on them.  We already test them
> > > separately so this isn't a
> > >           loss.
> > >
> > > Note that the testsuite is not entire clean due to gdb failure
> > > caused by alloca with stack clash. On AArch64 we output an incorrect
> > > .loc directive, but this is already the case with the current
> > > implementation in GCC and is a bug unrelated to this patch series.
> > >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > x86_64-pc-linux-gnu and no issues.
> > > Both targets were tested with stack clash on and off by default.
> > >
> > > Ok for trunk?
> > >
> > > Thanks,
> > > Tamar
> > >
> > > gcc/testsuite/
> > > 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
> > >
> > > 	PR target/86486
> > > 	gcc.dg/pr82788.c: Skip for AArch64.
> > > 	gcc.dg/guality/vla-1.c: Turn off stack-clash.
> > > 	gcc.target/aarch64/subsp.c: Likewise.
> > > 	gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> > > 	gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> > > 	gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> > > 	gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> > > 	on AArch64.
> > >
> > > --
> 
> > diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> > b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> > index
> >
> 52ade3aab7566dce3ca7ef931ac65895005d5e13..c97465edae195442a71ee66a
> b250
> > 15a2ac4fc8fc 100644
> > --- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> > +++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do run } */
> > -/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" }
> > */
> > +/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable
> > +-fno-stack-clash-protection" } */
> >
> >  typedef long int V;
> >  int x = -1;
> > diff --git a/gcc/testsuite/gcc.dg/params/blocksort-part.c
> > b/gcc/testsuite/gcc.dg/params/blocksort-part.c
> > index
> >
> a9154f2e61ccd21b60153f20be3891b988f9ef2c..1e677878e7bd9c68b026f8c72b
> 0d
> > e9f01e15459c 100644
> > --- a/gcc/testsuite/gcc.dg/params/blocksort-part.c
> > +++ b/gcc/testsuite/gcc.dg/params/blocksort-part.c
> > @@ -1,3 +1,4 @@
> > +/* { dg-skip-if "AArch64 does not support these bounds." {
> > +aarch64*-*-* } { "--param stack-clash-protection-*" } } */
> >
> >  /*-------------------------------------------------------------*/
> >  /*--- Block sorting machinery                               ---*/
> > diff --git a/gcc/testsuite/gcc.dg/pr82788.c
> > b/gcc/testsuite/gcc.dg/pr82788.c index
> >
> a8f628fd7f66c3e56739f6ff491df38b23f4d4df..41c442f61a625c8b350e1e4c870a
> > 98d86b167031 100644
> > --- a/gcc/testsuite/gcc.dg/pr82788.c
> > +++ b/gcc/testsuite/gcc.dg/pr82788.c
> > @@ -1,4 +1,5 @@
> >  /* { dg-do run } */
> >  /* { dg-options "-O2 -fstack-clash-protection --param
> > stack-clash-protection-probe-interval=10 --param
> > stack-clash-protection-guard-size=12" } */
> >  /* { dg-require-effective-target supports_stack_clash_protection } */
> > +/* { dg-skip-if "AArch64 does not support this interval." {
> > +aarch64*-*-* } } */
> >  int main() { int a[1442]; return 0;}
> > diff --git a/gcc/testsuite/gcc.dg/stack-check-10.c
> > b/gcc/testsuite/gcc.dg/stack-check-10.c
> > index
> >
> a86956ad6925464e4a938a33e609fae5004201c7..2f5a090cb7a4ed6d2e6e43174
> 921
> > 50a348a326ab 100644
> > --- a/gcc/testsuite/gcc.dg/stack-check-10.c
> > +++ b/gcc/testsuite/gcc.dg/stack-check-10.c
> > @@ -39,3 +39,4 @@ f3 (void)
> >     need a frame pointer.  Otherwise neither should.  */
> >  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer
> > needed" 2 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf }
> > } } } */
> >  /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer
> > needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } }
> > } } */
> > +/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack
> > +adjustment in prologue" 2 "pro_and_epilogue" { target { aarch64*-*-*
> > +} } } } */
> > diff --git a/gcc/testsuite/gcc.dg/stack-check-5.c
> > b/gcc/testsuite/gcc.dg/stack-check-5.c
> > index
> >
> 604fa3cf6c5b502f74c1e3497b3b8d72a15bb3ea..0243147939c10e8f4632520b12
> 71
> > 4724af85b332 100644
> > --- a/gcc/testsuite/gcc.dg/stack-check-5.c
> > +++ b/gcc/testsuite/gcc.dg/stack-check-5.c
> > @@ -66,7 +66,9 @@ f3 (void)
> >  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer
> > needed" 4 "pro_and_epilogue" { target { ! frame_pointer_for_non_leaf }
> > } } } */
> >  /* { dg-final { scan-rtl-dump-times "Stack clash no frame pointer
> > needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } }
> > } } */
> >  /* { dg-final { scan-rtl-dump-times "Stack clash frame pointer
> > needed" 2 "pro_and_epilogue" { target { frame_pointer_for_non_leaf } }
> > } } */
> > -
> > +/* AArch64 won't require a probe here due to the allocation amount
> > +being less than 1KB.  */
> > +/* { dg-final { scan-rtl-dump-times "Stack clash no probe small stack
> > +adjustment in prologue" 3 "pro_and_epilogue" { target { aarch64*-*-*
> > +} } } } */
> > +/* { dg-final { scan-rtl-dump-times "Stack clash no probe no stack
> > +adjustment in prologue" 1 "pro_and_epilogue" { target { aarch64*-*-*
> > +} } } } */
> >
> >  /* We have selected the size of the array in f2/f3 to be large enough
> >     to not live in the red zone on targets that support it.
> > diff --git a/gcc/testsuite/gcc.dg/stack-check-6a.c
> > b/gcc/testsuite/gcc.dg/stack-check-6a.c
> > index
> >
> 8fb9c621585957a85877ebadfbc4a8daabe4311c..68dd9bc48a0c26ecb84ddd2c0
> 9b8
> > aa74d3276695 100644
> > --- a/gcc/testsuite/gcc.dg/stack-check-6a.c
> > +++ b/gcc/testsuite/gcc.dg/stack-check-6a.c
> > @@ -5,6 +5,7 @@
> >  /* { dg-options "-O2 -fstack-clash-protection
> > -fdump-rtl-pro_and_epilogue -fno-optimize-sibling-calls --param
> > stack-clash-protection-probe-interval=12 --param
> > stack-clash-protection-guard-size=16" } */
> >  /* { dg-require-effective-target supports_stack_clash_protection  }
> > */
> >  /* { dg-skip-if "" { *-*-* } { "-fstack-protector*" } { "" } } */
> > +/* { dg-skip-if "" { aarch64*-*-* } } */
> >
> >
> >  #include "stack-check-6.c"
> > diff --git a/gcc/testsuite/gcc.target/aarch64/subsp.c
> > b/gcc/testsuite/gcc.target/aarch64/subsp.c
> > index
> >
> 70d848c59d1f1e4df4314ca012c7a5d9d3b91ebc..6ef6b2c90ae694055749a94b6
> 8cb
> > ba5ee4aea882 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/subsp.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/subsp.c
> > @@ -1,4 +1,4 @@
> > -/* { dg-options "-O" } */
> > +/* { dg-options "-O -fno-stack-clash-protection" } */
> >
> >  int foo (void *);
> >
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> > index
> >
> 29702ab55f249c3ebd0baf44981870524098e1e4..baeec61bb59aff56f0dcc20fc6
> ec
> > 6b93d517490e 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> > +/* { dg-options "-O2 -ftree-vectorize -ffast-math
> > +-fno-stack-clash-protection" } */
> >
> >  #include <stdint.h>
> >
> > diff --git
> > a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> > index
> >
> 001f5be8ff58bfcc75eccc4c050bef1e53faffeb..eae3be7a7b24dc124f7c1c26a97f
> > b25400cc62d2 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> > +/* { dg-options "-O2 -ftree-vectorize -ffast-math
> > +-fno-stack-clash-protection" } */
> >
> >  #include <stdint.h>
> >
> > diff --git
> > a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> > index
> >
> 59e9ee49c4a214b731ed1975da0dcfa46c059f8b..ce9825f73e8495a7f5c1f4ab1d
> 5b
> > f1aaf5035e17 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
> > +/* { dg-options "-O2 -ftree-vectorize -ffast-math
> > +-fno-stack-clash-protection" } */
> >
> >  #include <stdint.h>
> >
> > diff --git a/gcc/testsuite/lib/target-supports.exp
> > b/gcc/testsuite/lib/target-supports.exp
> > index
> >
> 7ec350213e9225ad342d030afac30bd613851aa1..5624f361c59b97cb967b3485a
> dc8
> > db01da048a84 100644
> > --- a/gcc/testsuite/lib/target-supports.exp
> > +++ b/gcc/testsuite/lib/target-supports.exp
> > @@ -9280,10 +9280,6 @@ proc
> > check_effective_target_supports_stack_clash_protection { } {  # Return
> > 1 if the target creates a frame pointer for non-leaf functions  # Note we
> ignore cases where we apply tail call optimization here.
> >  proc check_effective_target_frame_pointer_for_non_leaf { } {
> > -  if { [istarget aarch*-*-*] } {
> > -	return 1
> > -  }
> > -
> >    # Solaris/x86 defaults to -fno-omit-frame-pointer.
> >    if { [istarget i?86-*-solaris*] || [istarget x86_64-*-solaris*] } {
> >      return 1
> >
Jeff Law Aug. 3, 2018, 6:29 p.m. UTC | #4
On 07/24/2018 04:28 AM, tamar.christina@arm.com wrote:
> Hi All,
> 
> This patch cleans up the testsuite when a run is done with stack clash
> protection turned on.
> 
> Concretely this switches off -fstack-clash-protection for a couple of tests:
> 
> * sve: We don't yet support stack-clash-protection and sve, so for now turn these off.
> * assembler scan: some tests are quite fragile in that they check for exact
>        assembly output, e.g. check for exact amount of sub etc.  These won't
>        match now.
> * vla: Some of the ubsan tests negative array indices. Because the arrays weren't
>        used before the incorrect $sp wouldn't have been used. The correct value is
>        restored on ret.  Now however we probe the $sp which causes a segfault.
> * params: When testing the parameters we have to skip these on AArch64 because of our
>           custom constraints on them.  We already test them separately so this isn't a
>           loss.
> 
> Note that the testsuite is not entire clean due to gdb failure caused by alloca with
> stack clash. On AArch64 we output an incorrect .loc directive, but this is already the
> case with the current implementation in GCC and is a bug unrelated to this patch series.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu and no issues.
> Both targets were tested with stack clash on and off by default.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> gcc/testsuite/
> 2018-07-24  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/86486
> 	* gcc.dg/pr82788.c: Skip for AArch64.
> 	* gcc.dg/guality/vla-1.c: Turn off stack-clash.
> 	* gcc.target/aarch64/subsp.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
> 	* gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
> 	* gcc.dg/params/blocksort-part.c: Skip stack-clash checks
> 	on AArch64.
> 	* gcc.dg/stack-check-10.c: Add AArch64 specific checks.
> 	* gcc.dg/stack-check-5.c: Add AArch64 specific checks.
> 	* gcc.dg/stack-check-6a.c: Skip on AArch64, we don't support this.
> 	* testsuite/lib/target-supports.exp
> 	(check_effective_target_frame_pointer_for_non_leaf): AArch64 does not
> 	require frame pointer for non-leaf functions.
> 
>> -----Original Message-----
>> From: Tamar Christina <tamar.christina@arm.com>
>> Sent: Wednesday, July 11, 2018 12:23
>> To: gcc-patches@gcc.gnu.org
>> Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>;
>> Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
>> <Marcus.Shawcroft@arm.com>
>> Subject: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when stack-
>> clash is on [Patch (6/6)]
>>
>> Hi All,
>>
>> This patch cleans up the testsuite when a run is done with stack clash
>> protection turned on.
>>
>> Concretely this switches off -fstack-clash-protection for a couple of tests:
>>
>> * sve: We don't yet support stack-clash-protection and sve, so for now turn
>> these off.
>> * assembler scan: some tests are quite fragile in that they check for exact
>>        assembly output, e.g. check for exact amount of sub etc.  These won't
>>        match now.
>> * vla: Some of the ubsan tests negative array indices. Because the arrays
>> weren't
>>        used before the incorrect $sp wouldn't have been used. The correct
>> value is
>>        restored on ret.  Now however we probe the $sp which causes a segfault.
>> * params: When testing the parameters we have to skip these on AArch64
>> because of our
>>           custom constraints on them.  We already test them separately so this
>> isn't a
>>           loss.
>>
>> Note that the testsuite is not entire clean due to gdb failure caused by alloca
>> with stack clash. On AArch64 we output an incorrect .loc directive, but this is
>> already the case with the current implementation in GCC and is a bug
>> unrelated to this patch series.
>>
>> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
>> and no issues.
>> Both targets were tested with stack clash on and off by default.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Tamar
>>
>> gcc/testsuite/
>> 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
>>
>> 	PR target/86486
>> 	gcc.dg/pr82788.c: Skip for AArch64.
>> 	gcc.dg/guality/vla-1.c: Turn off stack-clash.
>> 	gcc.target/aarch64/subsp.c: Likewise.
>> 	gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise.
>> 	gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise.
>> 	gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise.
>> 	gcc.dg/params/blocksort-part.c: Skip stack-clash checks
>> 	on AArch64.

This is fine.  FWIW, I'd been ignoring vla-1 and one or two others that
were clearly invalid if stack-clash were on by default locally, but
didn't push any kind of patch for that upstream  since I didn't think
anyone builds with stack-clash on by default (I did for testing purposes
of course).

Jeff
diff mbox series

Patch

diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
index 52ade3aab7566dce3ca7ef931ac65895005d5e13..c97465edae195442a71ee66ab25015a2ac4fc8fc 100644
--- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
@@ -1,5 +1,5 @@ 
 /* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable -fno-stack-clash-protection" } */
 
 typedef long int V;
 int x = -1;
diff --git a/gcc/testsuite/gcc.dg/params/blocksort-part.c b/gcc/testsuite/gcc.dg/params/blocksort-part.c
index a9154f2e61ccd21b60153f20be3891b988f9ef2c..1e677878e7bd9c68b026f8c72b0de9f01e15459c 100644
--- a/gcc/testsuite/gcc.dg/params/blocksort-part.c
+++ b/gcc/testsuite/gcc.dg/params/blocksort-part.c
@@ -1,3 +1,4 @@ 
+/* { dg-skip-if "AArch64 does not support these bounds." { aarch64*-*-* } { "--param stack-clash-protection-*" } } */
 
 /*-------------------------------------------------------------*/
 /*--- Block sorting machinery                               ---*/
diff --git a/gcc/testsuite/gcc.dg/pr82788.c b/gcc/testsuite/gcc.dg/pr82788.c
index a8f628fd7f66c3e56739f6ff491df38b23f4d4df..41c442f61a625c8b350e1e4c870a98d86b167031 100644
--- a/gcc/testsuite/gcc.dg/pr82788.c
+++ b/gcc/testsuite/gcc.dg/pr82788.c
@@ -1,4 +1,5 @@ 
 /* { dg-do run } */
 /* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-probe-interval=10 --param stack-clash-protection-guard-size=12" } */
 /* { dg-require-effective-target supports_stack_clash_protection } */
+/* { dg-skip-if "AArch64 does not support this interval." { aarch64*-*-* } } */
 int main() { int a[1442]; return 0;}
diff --git a/gcc/testsuite/gcc.target/aarch64/subsp.c b/gcc/testsuite/gcc.target/aarch64/subsp.c
index 70d848c59d1f1e4df4314ca012c7a5d9d3b91ebc..6ef6b2c90ae694055749a94b68cbba5ee4aea882 100644
--- a/gcc/testsuite/gcc.target/aarch64/subsp.c
+++ b/gcc/testsuite/gcc.target/aarch64/subsp.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-O" } */
+/* { dg-options "-O -fno-stack-clash-protection" } */
 
 int foo (void *);
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
index 29702ab55f249c3ebd0baf44981870524098e1e4..baeec61bb59aff56f0dcc20fc6ec6b93d517490e 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_load_3.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
index 001f5be8ff58bfcc75eccc4c050bef1e53faffeb..eae3be7a7b24dc124f7c1c26a97fb25400cc62d2 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_3.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
index 59e9ee49c4a214b731ed1975da0dcfa46c059f8b..ce9825f73e8495a7f5c1f4ab1d5bf1aaf5035e17 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/mask_struct_store_4.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -ffast-math" } */
+/* { dg-options "-O2 -ftree-vectorize -ffast-math -fno-stack-clash-protection" } */
 
 #include <stdint.h>