Re: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when stack-clash is on [Patch (6/6)]
On 07/24/2018 04:28 AM, tamar.christ...@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 > > 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 >> Sent: Wednesday, July 11, 2018 12:23 >> To: gcc-patches@gcc.gnu.org >> Cc: nd ; James Greenhalgh ; >> Richard Earnshaw ; Marcus Shawcroft >> >> 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 >> >> 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
RE: [PATCH][GCC][AArch64] Cleanup the AArch64 testsuite when stack-clash is on [Patch (6/6)]
Hi James, Thanks for the review. > -Original Message- > From: James Greenhalgh > Sent: Tuesday, July 31, 2018 22:07 > To: Tamar Christina > Cc: gcc-patches@gcc.gnu.org; nd ; Richard Earnshaw > ; Marcus Shawcroft > > 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 > > > > 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 > > > Sent: Wednesday, July 11, 2018 12:23 > > > To: gcc-patches@gcc.gnu.org > > > Cc: nd ; James Greenhalgh > ; > > > Richard Earnshaw ;
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. > * 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 > > 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 > > Sent: Wednesday, July 11, 2018 12:23 > > To: gcc-patches@gcc.gnu.org > > Cc: nd ; James Greenhalgh ; > > Richard Earnshaw ; Marcus Shawcroft > > > > 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 > > > > PR target/86486 > > gcc.dg/pr82788.c: Skip for AArch64. > > gcc.dg/guality/vla-1.c: Turn off stack-clash. > >
RE: [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-24 Tamar Christina
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
> Sent: Wednesday, July 11, 2018 12:23
> To: gcc-patches@gcc.gnu.org
> Cc: nd ; James Greenhalgh ;
> Richard Earnshaw ; Marcus Shawcroft
>
> 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
>
> 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
