| Message ID | 20180711112121.GA15812@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [AArch64] Ensure that outgoing argument size is at least 8 bytes when alloca and stack-clash. [Patch (3/6)] | expand |
Hi All, I'm sending an updated patch which updates a testcase that hits one of our corner cases. This is an assembler scan only update in a testcase. Regards, Tamar > -----Original Message----- > From: Tamar Christina <tamar.christina@arm.com> > Sent: Wednesday, July 11, 2018 12:21 > 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] Ensure that outgoing argument size is at > least 8 bytes when alloca and stack-clash. [Patch (3/6)] > > Hi All, > > This patch adds a requirement that the number of outgoing arguments for a > function is at least 8 bytes when using stack-clash protection. > > By using this condition we can avoid a check in the alloca code and so have > smaller and simpler code there. > > A simplified version of the AArch64 stack frames is: > > +-----------------------+ > | | > | | > | | > +-----------------------+ > |LR | > +-----------------------+ > |FP | > +-----------------------+ > |dynamic allocations | ---- expanding area which will push the outgoing > +-----------------------+ args down during each allocation. > |padding | > +-----------------------+ > |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > +-----------------------+ > > By always defining an outgoing argument, alloca(0) effectively is safe to > probe at $sp due to the reserved buffer being there. It will never corrupt the > stack. > > This is also safe for alloca(x) where x is 0 or x % page_size == 0. In the former > it is the same case as alloca(0) while the latter is safe because any allocation > pushes the outgoing stack args down: > > |FP | > +-----------------------+ > | | > |dynamic allocations | ---- alloca (x) > | | > +-----------------------+ > |padding | > +-----------------------+ > |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > +-----------------------+ > > Which means when you probe for the residual, if it's 0 you'll again just probe > in the outgoing stack args range, which we know is non-zero (at least 8 bytes). > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > Target was tested with stack clash on and off by default. > > Ok for trunk? > > Thanks, > Tamar > > gcc/ > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, > STACK_DYNAMIC_OFFSET): New. > * config/aarch64/aarch64.c (aarch64_layout_frame): > Update outgoing args size. > (aarch64_stack_clash_protection_alloca_probe_range, > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): > New. > > gcc/testsuite/ > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * gcc.target/aarch64/stack-check-alloca-1.c: New. > * gcc.target/aarch64/stack-check-alloca-10.c: New. > * gcc.target/aarch64/stack-check-alloca-2.c: New. > * gcc.target/aarch64/stack-check-alloca-3.c: New. > * gcc.target/aarch64/stack-check-alloca-4.c: New. > * gcc.target/aarch64/stack-check-alloca-5.c: New. > * gcc.target/aarch64/stack-check-alloca-6.c: New. > * gcc.target/aarch64/stack-check-alloca-7.c: New. > * gcc.target/aarch64/stack-check-alloca-8.c: New. > * gcc.target/aarch64/stack-check-alloca-9.c: New. > * gcc.target/aarch64/stack-check-alloca.h: New. > * gcc.target/aarch64/stack-check-14.c: New. > * gcc.target/aarch64/stack-check-15.c: New. > > -- diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 1345f0eb171d05e2b833935c0a32f79c3db03f99..e9560b53bd8b5761855561dbf82d9c90cc1c282a 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -88,6 +88,10 @@ before probing has to be done for stack clash protection. */ #define STACK_CLASH_CALLER_GUARD 1024 +/* This value represents the minimum amount of bytes we expect the function's + outgoing arguments to be when stack-clash is enabled. */ +#define STACK_CLASH_OUTGOING_ARGS 8 + /* This value controls how many pages we manually unroll the loop for when generating stack clash probes. */ #define STACK_CLASH_MAX_UNROLL_PAGES 4 @@ -1069,4 +1073,15 @@ extern poly_uint16 aarch64_sve_vg; #define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE) +/* Allocate the minimum of STACK_CLASH_OUTGOING_ARGS if stack clash protection + is enabled for the outgoing arguments. This is essential as the extra args + space allows if to skip a check in alloca. */ +#undef STACK_DYNAMIC_OFFSET +#define STACK_DYNAMIC_OFFSET(FUNDECL) \ + ((flag_stack_clash_protection \ + && cfun->calls_alloca \ + && known_lt (crtl->outgoing_args_size, STACK_CLASH_OUTGOING_ARGS)) \ + ? ROUND_UP (STACK_CLASH_OUTGOING_ARGS, STACK_BOUNDARY / BITS_PER_UNIT) \ + : (crtl->outgoing_args_size + STACK_POINTER_OFFSET)) + #endif /* GCC_AARCH64_H */ diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 7c5daf2c62eaeeb1f066d4f2828197b98d31f158..e74a4144bdf64a66f04789421e245deea6204b13 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4043,6 +4043,10 @@ aarch64_layout_frame (void) cfun->machine->frame.emit_frame_chain = aarch64_needs_frame_chain (); + /* Adjust the outgoing arguments size if required. Keep it in sync with what + the mid-end is doing. */ + crtl->outgoing_args_size = STACK_DYNAMIC_OFFSET (cfun); + #define SLOT_NOT_REQUIRED (-2) #define SLOT_REQUIRED (-1) @@ -4810,6 +4814,16 @@ aarch64_set_handled_components (sbitmap components) cfun->machine->reg_is_wrapped_separately[regno] = true; } +/* On AArch64 we have an ABI defined safe buffer for which we have to do no + probing. */ + +static HOST_WIDE_INT +aarch64_stack_clash_protection_alloca_probe_range (void) +{ + return STACK_CLASH_CALLER_GUARD; +} + + /* Allocate POLY_SIZE bytes of stack space using TEMP1 and TEMP2 as scratch registers. If POLY_SIZE is not large enough to require a probe this function will only adjust the stack. When allocation the stack space @@ -18359,6 +18373,10 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_CONSTANT_ALIGNMENT #define TARGET_CONSTANT_ALIGNMENT aarch64_constant_alignment +#undef TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE +#define TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE \ + aarch64_stack_clash_protection_alloca_probe_range + #undef TARGET_COMPUTE_PRESSURE_CLASSES #define TARGET_COMPUTE_PRESSURE_CLASSES aarch64_compute_pressure_classes diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-14.c b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c new file mode 100644 index 0000000000000000000000000000000000000000..a573d108a0ce0243e038411aa01eb65dfd81a460 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (4050); + x = t1 (x); + return p[x]; +} + + +/* This test has a constant sized alloca that is smaller than the + probe interval. Only one probe is required since the value is larger + than 1024 bytes but smaller than 63k. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 1 } } */ + + + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-15.c b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c new file mode 100644 index 0000000000000000000000000000000000000000..497d83e6596a3cbfce898ca6cda1a65d5fe76d0b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (x); + x = t1 (x); + return p[x]; +} + + +/* This test has a variable sized alloca. It requires 3 probes. + One in the loop, one for the residual, one for when it's < 1024 and one for + when it's not. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 3 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c new file mode 100644 index 0000000000000000000000000000000000000000..524713a2c17f51eef0c58851661f9275f24212c9 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE y +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c new file mode 100644 index 0000000000000000000000000000000000000000..24553daf9eedd372c1766b884fd48bebd42de9b2 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127.5 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c new file mode 100644 index 0000000000000000000000000000000000000000..086fa86a4d2d6bffdca6474afa93c6a508000d58 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 0 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-not {str\s+xzr,} } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c new file mode 100644 index 0000000000000000000000000000000000000000..f5c20dad7ace049da0da6bfaafb5ec5e0b985e6e --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 100 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c new file mode 100644 index 0000000000000000000000000000000000000000..729d1184bc981a7fb00bc14d118ef526f96360b8 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 2 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c new file mode 100644 index 0000000000000000000000000000000000000000..2c08aa13dcf5cee5fedbd931535ae2776e422ad6 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c new file mode 100644 index 0000000000000000000000000000000000000000..2881e9666cebb1f5f3d5867d23ade5d4b9d8366c --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63.5 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c new file mode 100644 index 0000000000000000000000000000000000000000..60f91c1c4dbc13c6f281e5d9ff2d89688dc942c7 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c new file mode 100644 index 0000000000000000000000000000000000000000..07bf2e6d9abc65b211f4e128fae432d68d7be8d5 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 65 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c new file mode 100644 index 0000000000000000000000000000000000000000..4b4da189b782c61438715bf5a8f0b791de4e29dd --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127 * 64 * 2014 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h new file mode 100644 index 0000000000000000000000000000000000000000..a4f7fa2dd351cc7b76784103fc645a066c5ba6e3 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h @@ -0,0 +1,13 @@ +#include <alloca.h> + +__attribute__((noinline, noipa)) +void g (char* ptr, int y) +{ + ptr[y] = '\0'; +} + +void f_caller (int y) +{ + char* pStr = alloca(SIZE); + g (pStr, y); +} \ No newline at end of file
Hi All, Attached an updated patch which documents what the test cases are expecting as requested. Ok for trunk? Thanks, Tamar gcc/ 2018-07-25 Tamar Christina <tamar.christina@arm.com> PR target/86486 * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, STACK_DYNAMIC_OFFSET): New. * config/aarch64/aarch64.c (aarch64_layout_frame): Update outgoing args size. (aarch64_stack_clash_protection_alloca_probe_range, TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): New. gcc/testsuite/ 2018-07-25 Tamar Christina <tamar.christina@arm.com> PR target/86486 * gcc.target/aarch64/stack-check-alloca-1.c: New. * gcc.target/aarch64/stack-check-alloca-10.c: New. * gcc.target/aarch64/stack-check-alloca-2.c: New. * gcc.target/aarch64/stack-check-alloca-3.c: New. * gcc.target/aarch64/stack-check-alloca-4.c: New. * gcc.target/aarch64/stack-check-alloca-5.c: New. * gcc.target/aarch64/stack-check-alloca-6.c: New. * gcc.target/aarch64/stack-check-alloca-7.c: New. * gcc.target/aarch64/stack-check-alloca-8.c: New. * gcc.target/aarch64/stack-check-alloca-9.c: New. * gcc.target/aarch64/stack-check-alloca.h: New. * gcc.target/aarch64/stack-check-14.c: New. * gcc.target/aarch64/stack-check-15.c: New. > -----Original Message----- > From: Tamar Christina > Sent: Friday, July 13, 2018 17:36 > To: Tamar Christina <Tamar.Christina@arm.com>; 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: RE: [PATCH][GCC][AArch64] Ensure that outgoing argument size is at > least 8 bytes when alloca and stack-clash. [Patch (3/6)] > > Hi All, > > I'm sending an updated patch which updates a testcase that hits one of our > corner cases. > This is an assembler scan only update in a testcase. > > Regards, > Tamar > > > -----Original Message----- > > From: Tamar Christina <tamar.christina@arm.com> > > Sent: Wednesday, July 11, 2018 12:21 > > 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] Ensure that outgoing argument size is > > at least 8 bytes when alloca and stack-clash. [Patch (3/6)] > > > > Hi All, > > > > This patch adds a requirement that the number of outgoing arguments > > for a function is at least 8 bytes when using stack-clash protection. > > > > By using this condition we can avoid a check in the alloca code and so > > have smaller and simpler code there. > > > > A simplified version of the AArch64 stack frames is: > > > > +-----------------------+ > > | | > > | | > > | | > > +-----------------------+ > > |LR | > > +-----------------------+ > > |FP | > > +-----------------------+ > > |dynamic allocations | ---- expanding area which will push the outgoing > > +-----------------------+ args down during each allocation. > > |padding | > > +-----------------------+ > > |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > > +-----------------------+ > > > > By always defining an outgoing argument, alloca(0) effectively is safe > > to probe at $sp due to the reserved buffer being there. It will never > > corrupt the stack. > > > > This is also safe for alloca(x) where x is 0 or x % page_size == 0. > > In the former it is the same case as alloca(0) while the latter is > > safe because any allocation pushes the outgoing stack args down: > > > > |FP | > > +-----------------------+ > > | | > > |dynamic allocations | ---- alloca (x) > > | | > > +-----------------------+ > > |padding | > > +-----------------------+ > > |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > > +-----------------------+ > > > > Which means when you probe for the residual, if it's 0 you'll again > > just probe in the outgoing stack args range, which we know is non-zero (at > least 8 bytes). > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Target was tested with stack clash on and off by default. > > > > Ok for trunk? > > > > Thanks, > > Tamar > > > > gcc/ > > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, > > STACK_DYNAMIC_OFFSET): New. > > * config/aarch64/aarch64.c (aarch64_layout_frame): > > Update outgoing args size. > > (aarch64_stack_clash_protection_alloca_probe_range, > > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): > > New. > > > > gcc/testsuite/ > > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * gcc.target/aarch64/stack-check-alloca-1.c: New. > > * gcc.target/aarch64/stack-check-alloca-10.c: New. > > * gcc.target/aarch64/stack-check-alloca-2.c: New. > > * gcc.target/aarch64/stack-check-alloca-3.c: New. > > * gcc.target/aarch64/stack-check-alloca-4.c: New. > > * gcc.target/aarch64/stack-check-alloca-5.c: New. > > * gcc.target/aarch64/stack-check-alloca-6.c: New. > > * gcc.target/aarch64/stack-check-alloca-7.c: New. > > * gcc.target/aarch64/stack-check-alloca-8.c: New. > > * gcc.target/aarch64/stack-check-alloca-9.c: New. > > * gcc.target/aarch64/stack-check-alloca.h: New. > > * gcc.target/aarch64/stack-check-14.c: New. > > * gcc.target/aarch64/stack-check-15.c: New. > > > > -- diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 1345f0eb171d05e2b833935c0a32f79c3db03f99..e9560b53bd8b5761855561dbf82d9c90cc1c282a 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -88,6 +88,10 @@ before probing has to be done for stack clash protection. */ #define STACK_CLASH_CALLER_GUARD 1024 +/* This value represents the minimum amount of bytes we expect the function's + outgoing arguments to be when stack-clash is enabled. */ +#define STACK_CLASH_OUTGOING_ARGS 8 + /* This value controls how many pages we manually unroll the loop for when generating stack clash probes. */ #define STACK_CLASH_MAX_UNROLL_PAGES 4 @@ -1069,4 +1073,15 @@ extern poly_uint16 aarch64_sve_vg; #define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE) +/* Allocate the minimum of STACK_CLASH_OUTGOING_ARGS if stack clash protection + is enabled for the outgoing arguments. This is essential as the extra args + space allows if to skip a check in alloca. */ +#undef STACK_DYNAMIC_OFFSET +#define STACK_DYNAMIC_OFFSET(FUNDECL) \ + ((flag_stack_clash_protection \ + && cfun->calls_alloca \ + && known_lt (crtl->outgoing_args_size, STACK_CLASH_OUTGOING_ARGS)) \ + ? ROUND_UP (STACK_CLASH_OUTGOING_ARGS, STACK_BOUNDARY / BITS_PER_UNIT) \ + : (crtl->outgoing_args_size + STACK_POINTER_OFFSET)) + #endif /* GCC_AARCH64_H */ diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index c180b006095ee93dd9d845647486409356e80141..0fd7d3412d9fa60a13eb379ff7f3fdd59995fb9f 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4043,6 +4043,10 @@ aarch64_layout_frame (void) cfun->machine->frame.emit_frame_chain = aarch64_needs_frame_chain (); + /* Adjust the outgoing arguments size if required. Keep it in sync with what + the mid-end is doing. */ + crtl->outgoing_args_size = STACK_DYNAMIC_OFFSET (cfun); + #define SLOT_NOT_REQUIRED (-2) #define SLOT_REQUIRED (-1) @@ -4810,6 +4814,16 @@ aarch64_set_handled_components (sbitmap components) cfun->machine->reg_is_wrapped_separately[regno] = true; } +/* On AArch64 we have an ABI defined safe buffer for which we have to do no + probing. */ + +static HOST_WIDE_INT +aarch64_stack_clash_protection_alloca_probe_range (void) +{ + return STACK_CLASH_CALLER_GUARD; +} + + /* Allocate POLY_SIZE bytes of stack space using TEMP1 and TEMP2 as scratch registers. If POLY_SIZE is not large enough to require a probe this function will only adjust the stack. When allocation the stack space @@ -18374,6 +18388,10 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_CONSTANT_ALIGNMENT #define TARGET_CONSTANT_ALIGNMENT aarch64_constant_alignment +#undef TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE +#define TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE \ + aarch64_stack_clash_protection_alloca_probe_range + #undef TARGET_COMPUTE_PRESSURE_CLASSES #define TARGET_COMPUTE_PRESSURE_CLASSES aarch64_compute_pressure_classes diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-14.c b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c new file mode 100644 index 0000000000000000000000000000000000000000..a573d108a0ce0243e038411aa01eb65dfd81a460 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (4050); + x = t1 (x); + return p[x]; +} + + +/* This test has a constant sized alloca that is smaller than the + probe interval. Only one probe is required since the value is larger + than 1024 bytes but smaller than 63k. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 1 } } */ + + + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-15.c b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c new file mode 100644 index 0000000000000000000000000000000000000000..497d83e6596a3cbfce898ca6cda1a65d5fe76d0b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (x); + x = t1 (x); + return p[x]; +} + + +/* This test has a variable sized alloca. It requires 3 probes. + One in the loop, one for the residual, one for when it's < 1024 and one for + when it's not. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 3 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c new file mode 100644 index 0000000000000000000000000000000000000000..7fc189f6210d97fe87af489f11c94a7a98c9ac94 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE y +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp\]} 1 } } */ + +/* Dynamic alloca, expect loop, and 2 probes with 1kB offset and 1 at sp. + 1st probe is inside the loop for the full guard-size allocations, second + probe is for the case where residual is zero and the final probe for when + residiual is > 1024 bytes. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c new file mode 100644 index 0000000000000000000000000000000000000000..7c42206d3158d624815a5cc13b2e619c4a432dda --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127.5 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ + +/* Large alloca of an amount which isn't a multiple of a guard-size, and + residiual is more than 1kB. Loop expected with one 1Kb probe offset and + one residual probe at offset 1kB. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c new file mode 100644 index 0000000000000000000000000000000000000000..69fdd16e35a296276acaf76bb0e0954ffba5a421 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 0 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-not {str\s+xzr,} } } */ + +/* Alloca of 0 should emit no probes, boundary condition. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c new file mode 100644 index 0000000000000000000000000000000000000000..fba3a7a25b726558d4167f3d71c9490d1c89225d --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 100 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ + +/* Alloca is less than 1kB, 1 probe expected at word offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c new file mode 100644 index 0000000000000000000000000000000000000000..d53f30a4133b7b2af74823d87ce88ff0d2116550 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 2 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at + 1kB offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c new file mode 100644 index 0000000000000000000000000000000000000000..e0ff99ffbe1504d28aff314eeefdcac027321c5d --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at + 1kB offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c new file mode 100644 index 0000000000000000000000000000000000000000..c4bad9a2f46b405a7f01a71733f4534c66515742 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63.5 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at 1kB + offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c new file mode 100644 index 0000000000000000000000000000000000000000..cba9ff89c1a41da1f6b439239114b25351cab1d0 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is exactly one guard-size, 1 probe expected at 1kB offset. + Boundary condition. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c new file mode 100644 index 0000000000000000000000000000000000000000..5a35411b34483231bd05859b963a031d0011ad2b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 65 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ + +/* Alloca is more than one guard-page, and residual is exactly 1Kb. 2 probes + expected. One at 1kB offset for the guard-size allocation and one at word + offset for the residual. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c new file mode 100644 index 0000000000000000000000000000000000000000..9c49c3953d18a215f337c02d40fcfa287219e9ab --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Large alloca of an amount which is a multiple of a guard-size, no residiual. + Loop expected with one 1Kb probe offset and no residual probe. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h new file mode 100644 index 0000000000000000000000000000000000000000..a4f7fa2dd351cc7b76784103fc645a066c5ba6e3 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h @@ -0,0 +1,13 @@ +#include <alloca.h> + +__attribute__((noinline, noipa)) +void g (char* ptr, int y) +{ + ptr[y] = '\0'; +} + +void f_caller (int y) +{ + char* pStr = alloca(SIZE); + g (pStr, y); +} \ No newline at end of file
On 07/25/2018 05:09 AM, Tamar Christina wrote: > Hi All, > > Attached an updated patch which documents what the test cases are expecting as requested. > > Ok for trunk? > > Thanks, > Tamar > > gcc/ > 2018-07-25 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, > STACK_DYNAMIC_OFFSET): New. > * config/aarch64/aarch64.c (aarch64_layout_frame): > Update outgoing args size. > (aarch64_stack_clash_protection_alloca_probe_range, > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): New. > > gcc/testsuite/ > 2018-07-25 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * gcc.target/aarch64/stack-check-alloca-1.c: New. > * gcc.target/aarch64/stack-check-alloca-10.c: New. > * gcc.target/aarch64/stack-check-alloca-2.c: New. > * gcc.target/aarch64/stack-check-alloca-3.c: New. > * gcc.target/aarch64/stack-check-alloca-4.c: New. > * gcc.target/aarch64/stack-check-alloca-5.c: New. > * gcc.target/aarch64/stack-check-alloca-6.c: New. > * gcc.target/aarch64/stack-check-alloca-7.c: New. > * gcc.target/aarch64/stack-check-alloca-8.c: New. > * gcc.target/aarch64/stack-check-alloca-9.c: New. > * gcc.target/aarch64/stack-check-alloca.h: New. > * gcc.target/aarch64/stack-check-14.c: New. > * gcc.target/aarch64/stack-check-15.c: New. > >> -----Original Message----- >> From: Tamar Christina >> Sent: Friday, July 13, 2018 17:36 >> To: Tamar Christina <Tamar.Christina@arm.com>; 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: RE: [PATCH][GCC][AArch64] Ensure that outgoing argument size is at >> least 8 bytes when alloca and stack-clash. [Patch (3/6)] >> >> Hi All, >> >> I'm sending an updated patch which updates a testcase that hits one of our >> corner cases. >> This is an assembler scan only update in a testcase. >> >> Regards, >> Tamar >> >>> -----Original Message----- >>> From: Tamar Christina <tamar.christina@arm.com> >>> Sent: Wednesday, July 11, 2018 12:21 >>> 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] Ensure that outgoing argument size is >>> at least 8 bytes when alloca and stack-clash. [Patch (3/6)] >>> >>> Hi All, >>> >>> This patch adds a requirement that the number of outgoing arguments >>> for a function is at least 8 bytes when using stack-clash protection. >>> >>> By using this condition we can avoid a check in the alloca code and so >>> have smaller and simpler code there. >>> >>> A simplified version of the AArch64 stack frames is: >>> >>> +-----------------------+ >>> | | >>> | | >>> | | >>> +-----------------------+ >>> |LR | >>> +-----------------------+ >>> |FP | >>> +-----------------------+ >>> |dynamic allocations | ---- expanding area which will push the outgoing >>> +-----------------------+ args down during each allocation. >>> |padding | >>> +-----------------------+ >>> |outgoing stack args | ---- safety buffer of 8 bytes (aligned) >>> +-----------------------+ >>> >>> By always defining an outgoing argument, alloca(0) effectively is safe >>> to probe at $sp due to the reserved buffer being there. It will never >>> corrupt the stack. >>> >>> This is also safe for alloca(x) where x is 0 or x % page_size == 0. >>> In the former it is the same case as alloca(0) while the latter is >>> safe because any allocation pushes the outgoing stack args down: >>> >>> |FP | >>> +-----------------------+ >>> | | >>> |dynamic allocations | ---- alloca (x) >>> | | >>> +-----------------------+ >>> |padding | >>> +-----------------------+ >>> |outgoing stack args | ---- safety buffer of 8 bytes (aligned) >>> +-----------------------+ >>> >>> Which means when you probe for the residual, if it's 0 you'll again >>> just probe in the outgoing stack args range, which we know is non-zero (at >> least 8 bytes). >>> >>> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >>> Target was tested with stack clash on and off by default. >>> >>> Ok for trunk? >>> >>> Thanks, >>> Tamar >>> >>> gcc/ >>> 2018-07-11 Tamar Christina <tamar.christina@arm.com> >>> >>> PR target/86486 >>> * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, >>> STACK_DYNAMIC_OFFSET): New. >>> * config/aarch64/aarch64.c (aarch64_layout_frame): >>> Update outgoing args size. >>> (aarch64_stack_clash_protection_alloca_probe_range, >>> TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): >>> New. >>> >>> gcc/testsuite/ >>> 2018-07-11 Tamar Christina <tamar.christina@arm.com> >>> >>> PR target/86486 >>> * gcc.target/aarch64/stack-check-alloca-1.c: New. >>> * gcc.target/aarch64/stack-check-alloca-10.c: New. >>> * gcc.target/aarch64/stack-check-alloca-2.c: New. >>> * gcc.target/aarch64/stack-check-alloca-3.c: New. >>> * gcc.target/aarch64/stack-check-alloca-4.c: New. >>> * gcc.target/aarch64/stack-check-alloca-5.c: New. >>> * gcc.target/aarch64/stack-check-alloca-6.c: New. >>> * gcc.target/aarch64/stack-check-alloca-7.c: New. >>> * gcc.target/aarch64/stack-check-alloca-8.c: New. >>> * gcc.target/aarch64/stack-check-alloca-9.c: New. >>> * gcc.target/aarch64/stack-check-alloca.h: New. >>> * gcc.target/aarch64/stack-check-14.c: New. >>> * gcc.target/aarch64/stack-check-15.c: New. >>> >>> -- Likewise -- no concerns on my end. AArch64 maintainers have the final say. jeff
Hi All, This is a re-spin to address review comments. No code change aside from a variable rename. Ok for trunk? Thanks, Tamar gcc/ 2018-08-07 Tamar Christina <tamar.christina@arm.com> PR target/86486 * config/aarch64/aarch64.h (STACK_CLASH_MIN_BYTES_OUTGOING_ARGS, STACK_DYNAMIC_OFFSET): New. * config/aarch64/aarch64.c (aarch64_layout_frame): Update outgoing args size. (aarch64_stack_clash_protection_alloca_probe_range, TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): New. gcc/testsuite/ 2018-08-07 Tamar Christina <tamar.christina@arm.com> PR target/86486 * gcc.target/aarch64/stack-check-alloca-1.c: New. * gcc.target/aarch64/stack-check-alloca-10.c: New. * gcc.target/aarch64/stack-check-alloca-2.c: New. * gcc.target/aarch64/stack-check-alloca-3.c: New. * gcc.target/aarch64/stack-check-alloca-4.c: New. * gcc.target/aarch64/stack-check-alloca-5.c: New. * gcc.target/aarch64/stack-check-alloca-6.c: New. * gcc.target/aarch64/stack-check-alloca-7.c: New. * gcc.target/aarch64/stack-check-alloca-8.c: New. * gcc.target/aarch64/stack-check-alloca-9.c: New. * gcc.target/aarch64/stack-check-alloca.h: New. * gcc.target/aarch64/stack-check-14.c: New. * gcc.target/aarch64/stack-check-15.c: New. > -----Original Message----- > From: Jeff Law <law@redhat.com> > Sent: Friday, August 3, 2018 19:05 > To: Tamar Christina <Tamar.Christina@arm.com>; 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: Re: [PATCH][GCC][AArch64] Ensure that outgoing argument size is at > least 8 bytes when alloca and stack-clash. [Patch (3/6)] > > On 07/25/2018 05:09 AM, Tamar Christina wrote: > > Hi All, > > > > Attached an updated patch which documents what the test cases are > expecting as requested. > > > > Ok for trunk? > > > > Thanks, > > Tamar > > > > gcc/ > > 2018-07-25 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, > > STACK_DYNAMIC_OFFSET): New. > > * config/aarch64/aarch64.c (aarch64_layout_frame): > > Update outgoing args size. > > (aarch64_stack_clash_protection_alloca_probe_range, > > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): > New. > > > > gcc/testsuite/ > > 2018-07-25 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * gcc.target/aarch64/stack-check-alloca-1.c: New. > > * gcc.target/aarch64/stack-check-alloca-10.c: New. > > * gcc.target/aarch64/stack-check-alloca-2.c: New. > > * gcc.target/aarch64/stack-check-alloca-3.c: New. > > * gcc.target/aarch64/stack-check-alloca-4.c: New. > > * gcc.target/aarch64/stack-check-alloca-5.c: New. > > * gcc.target/aarch64/stack-check-alloca-6.c: New. > > * gcc.target/aarch64/stack-check-alloca-7.c: New. > > * gcc.target/aarch64/stack-check-alloca-8.c: New. > > * gcc.target/aarch64/stack-check-alloca-9.c: New. > > * gcc.target/aarch64/stack-check-alloca.h: New. > > * gcc.target/aarch64/stack-check-14.c: New. > > * gcc.target/aarch64/stack-check-15.c: New. > > > >> -----Original Message----- > >> From: Tamar Christina > >> Sent: Friday, July 13, 2018 17:36 > >> To: Tamar Christina <Tamar.Christina@arm.com>; > >> 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: RE: [PATCH][GCC][AArch64] Ensure that outgoing argument size > >> is at least 8 bytes when alloca and stack-clash. [Patch (3/6)] > >> > >> Hi All, > >> > >> I'm sending an updated patch which updates a testcase that hits one > >> of our corner cases. > >> This is an assembler scan only update in a testcase. > >> > >> Regards, > >> Tamar > >> > >>> -----Original Message----- > >>> From: Tamar Christina <tamar.christina@arm.com> > >>> Sent: Wednesday, July 11, 2018 12:21 > >>> 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] Ensure that outgoing argument size is > >>> at least 8 bytes when alloca and stack-clash. [Patch (3/6)] > >>> > >>> Hi All, > >>> > >>> This patch adds a requirement that the number of outgoing arguments > >>> for a function is at least 8 bytes when using stack-clash protection. > >>> > >>> By using this condition we can avoid a check in the alloca code and > >>> so have smaller and simpler code there. > >>> > >>> A simplified version of the AArch64 stack frames is: > >>> > >>> +-----------------------+ > >>> | | > >>> | | > >>> | | > >>> +-----------------------+ > >>> |LR | > >>> +-----------------------+ > >>> |FP | > >>> +-----------------------+ > >>> |dynamic allocations | ---- expanding area which will push the > outgoing > >>> +-----------------------+ args down during each allocation. > >>> |padding | > >>> +-----------------------+ > >>> |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > >>> +-----------------------+ > >>> > >>> By always defining an outgoing argument, alloca(0) effectively is > >>> safe to probe at $sp due to the reserved buffer being there. It > >>> will never corrupt the stack. > >>> > >>> This is also safe for alloca(x) where x is 0 or x % page_size == 0. > >>> In the former it is the same case as alloca(0) while the latter is > >>> safe because any allocation pushes the outgoing stack args down: > >>> > >>> |FP | > >>> +-----------------------+ > >>> | | > >>> |dynamic allocations | ---- alloca (x) > >>> | | > >>> +-----------------------+ > >>> |padding | > >>> +-----------------------+ > >>> |outgoing stack args | ---- safety buffer of 8 bytes (aligned) > >>> +-----------------------+ > >>> > >>> Which means when you probe for the residual, if it's 0 you'll again > >>> just probe in the outgoing stack args range, which we know is > >>> non-zero (at > >> least 8 bytes). > >>> > >>> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > >>> Target was tested with stack clash on and off by default. > >>> > >>> Ok for trunk? > >>> > >>> Thanks, > >>> Tamar > >>> > >>> gcc/ > >>> 2018-07-11 Tamar Christina <tamar.christina@arm.com> > >>> > >>> PR target/86486 > >>> * config/aarch64/aarch64.h (STACK_CLASH_OUTGOING_ARGS, > >>> STACK_DYNAMIC_OFFSET): New. > >>> * config/aarch64/aarch64.c (aarch64_layout_frame): > >>> Update outgoing args size. > >>> (aarch64_stack_clash_protection_alloca_probe_range, > >>> TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): > >>> New. > >>> > >>> gcc/testsuite/ > >>> 2018-07-11 Tamar Christina <tamar.christina@arm.com> > >>> > >>> PR target/86486 > >>> * gcc.target/aarch64/stack-check-alloca-1.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-10.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-2.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-3.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-4.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-5.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-6.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-7.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-8.c: New. > >>> * gcc.target/aarch64/stack-check-alloca-9.c: New. > >>> * gcc.target/aarch64/stack-check-alloca.h: New. > >>> * gcc.target/aarch64/stack-check-14.c: New. > >>> * gcc.target/aarch64/stack-check-15.c: New. > >>> > >>> -- > Likewise -- no concerns on my end. AArch64 maintainers have the final say. > > jeff diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 1345f0eb171d05e2b833935c0a32f79c3db03f99..9a51700fa38f2ddd8e003adf209755bcfafa9ee0 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -88,6 +88,10 @@ before probing has to be done for stack clash protection. */ #define STACK_CLASH_CALLER_GUARD 1024 +/* This value represents the minimum amount of bytes we expect the function's + outgoing arguments to be when stack-clash is enabled. */ +#define STACK_CLASH_MIN_BYTES_OUTGOING_ARGS 8 + /* This value controls how many pages we manually unroll the loop for when generating stack clash probes. */ #define STACK_CLASH_MAX_UNROLL_PAGES 4 @@ -1069,4 +1073,17 @@ extern poly_uint16 aarch64_sve_vg; #define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE) +/* Allocate a minimum of STACK_CLASH_MIN_BYTES_OUTGOING_ARGS bytes for the + outgoing arguments if stack clash protection is enabled. This is essential + as the extra arg space allows us to skip a check in alloca. */ +#undef STACK_DYNAMIC_OFFSET +#define STACK_DYNAMIC_OFFSET(FUNDECL) \ + ((flag_stack_clash_protection \ + && cfun->calls_alloca \ + && known_lt (crtl->outgoing_args_size, \ + STACK_CLASH_MIN_BYTES_OUTGOING_ARGS)) \ + ? ROUND_UP (STACK_CLASH_MIN_BYTES_OUTGOING_ARGS, \ + STACK_BOUNDARY / BITS_PER_UNIT) \ + : (crtl->outgoing_args_size + STACK_POINTER_OFFSET)) + #endif /* GCC_AARCH64_H */ diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 19f67112454aeb32ae0905e3511f03dfd803b87f..105750d1c2cba470d14d8079aa61195d4e03978c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4047,6 +4047,10 @@ aarch64_layout_frame (void) cfun->machine->frame.emit_frame_chain = aarch64_needs_frame_chain (); + /* Adjust the outgoing arguments size if required. Keep it in sync with what + the mid-end is doing. */ + crtl->outgoing_args_size = STACK_DYNAMIC_OFFSET (cfun); + #define SLOT_NOT_REQUIRED (-2) #define SLOT_REQUIRED (-1) @@ -4814,6 +4818,16 @@ aarch64_set_handled_components (sbitmap components) cfun->machine->reg_is_wrapped_separately[regno] = true; } +/* On AArch64 we have an ABI defined safe buffer. This constant is used to + determining the probe offset for alloca. */ + +static HOST_WIDE_INT +aarch64_stack_clash_protection_alloca_probe_range (void) +{ + return STACK_CLASH_CALLER_GUARD; +} + + /* Allocate POLY_SIZE bytes of stack space using TEMP1 and TEMP2 as scratch registers. If POLY_SIZE is not large enough to require a probe this function will only adjust the stack. When allocating the stack space @@ -18382,6 +18396,10 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_CONSTANT_ALIGNMENT #define TARGET_CONSTANT_ALIGNMENT aarch64_constant_alignment +#undef TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE +#define TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE \ + aarch64_stack_clash_protection_alloca_probe_range + #undef TARGET_COMPUTE_PRESSURE_CLASSES #define TARGET_COMPUTE_PRESSURE_CLASSES aarch64_compute_pressure_classes diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-14.c b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c new file mode 100644 index 0000000000000000000000000000000000000000..a573d108a0ce0243e038411aa01eb65dfd81a460 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (4050); + x = t1 (x); + return p[x]; +} + + +/* This test has a constant sized alloca that is smaller than the + probe interval. Only one probe is required since the value is larger + than 1024 bytes but smaller than 63k. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 1 } } */ + + + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-15.c b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c new file mode 100644 index 0000000000000000000000000000000000000000..497d83e6596a3cbfce898ca6cda1a65d5fe76d0b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (x); + x = t1 (x); + return p[x]; +} + + +/* This test has a variable sized alloca. It requires 3 probes. + One in the loop, one for the residual, one for when it's < 1024 and one for + when it's not. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 3 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c new file mode 100644 index 0000000000000000000000000000000000000000..7fc189f6210d97fe87af489f11c94a7a98c9ac94 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE y +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp\]} 1 } } */ + +/* Dynamic alloca, expect loop, and 2 probes with 1kB offset and 1 at sp. + 1st probe is inside the loop for the full guard-size allocations, second + probe is for the case where residual is zero and the final probe for when + residiual is > 1024 bytes. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c new file mode 100644 index 0000000000000000000000000000000000000000..7c42206d3158d624815a5cc13b2e619c4a432dda --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127.5 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ + +/* Large alloca of an amount which isn't a multiple of a guard-size, and + residiual is more than 1kB. Loop expected with one 1Kb probe offset and + one residual probe at offset 1kB. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c new file mode 100644 index 0000000000000000000000000000000000000000..69fdd16e35a296276acaf76bb0e0954ffba5a421 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 0 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-not {str\s+xzr,} } } */ + +/* Alloca of 0 should emit no probes, boundary condition. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c new file mode 100644 index 0000000000000000000000000000000000000000..fba3a7a25b726558d4167f3d71c9490d1c89225d --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 100 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ + +/* Alloca is less than 1kB, 1 probe expected at word offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c new file mode 100644 index 0000000000000000000000000000000000000000..d53f30a4133b7b2af74823d87ce88ff0d2116550 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 2 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at + 1kB offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c new file mode 100644 index 0000000000000000000000000000000000000000..e0ff99ffbe1504d28aff314eeefdcac027321c5d --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at + 1kB offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c new file mode 100644 index 0000000000000000000000000000000000000000..c4bad9a2f46b405a7f01a71733f4534c66515742 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63.5 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is more than 1kB, but less than guard-size, 1 probe expected at 1kB + offset. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c new file mode 100644 index 0000000000000000000000000000000000000000..cba9ff89c1a41da1f6b439239114b25351cab1d0 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Alloca is exactly one guard-size, 1 probe expected at 1kB offset. + Boundary condition. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c new file mode 100644 index 0000000000000000000000000000000000000000..5a35411b34483231bd05859b963a031d0011ad2b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 65 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ + +/* Alloca is more than one guard-page, and residual is exactly 1Kb. 2 probes + expected. One at 1kB offset for the guard-size allocation and one at word + offset for the residual. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c new file mode 100644 index 0000000000000000000000000000000000000000..5773d8052bc2d89842ad591b12807056a0247ac5 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + +/* Large alloca of a constant amount which is a multiple of a guard-size, + no residiual. Loop expected with one 1Kb probe offset and no residual probe + because residual is at compile time known to be zero. */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h new file mode 100644 index 0000000000000000000000000000000000000000..a4f7fa2dd351cc7b76784103fc645a066c5ba6e3 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h @@ -0,0 +1,13 @@ +#include <alloca.h> + +__attribute__((noinline, noipa)) +void g (char* ptr, int y) +{ + ptr[y] = '\0'; +} + +void f_caller (int y) +{ + char* pStr = alloca(SIZE); + g (pStr, y); +} \ No newline at end of file
On Tue, Aug 07, 2018 at 05:09:34AM -0500, Tamar Christina wrote: > Hi All, > > This is a re-spin to address review comments. No code change aside from a variable rename. > > Ok for trunk? OK. Thanks, James > gcc/ > 2018-08-07 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * config/aarch64/aarch64.h (STACK_CLASH_MIN_BYTES_OUTGOING_ARGS, > STACK_DYNAMIC_OFFSET): New. > * config/aarch64/aarch64.c (aarch64_layout_frame): > Update outgoing args size. > (aarch64_stack_clash_protection_alloca_probe_range, > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): New. > > gcc/testsuite/ > 2018-08-07 Tamar Christina <tamar.christina@arm.com> > > PR target/86486 > * gcc.target/aarch64/stack-check-alloca-1.c: New. > * gcc.target/aarch64/stack-check-alloca-10.c: New. > * gcc.target/aarch64/stack-check-alloca-2.c: New. > * gcc.target/aarch64/stack-check-alloca-3.c: New. > * gcc.target/aarch64/stack-check-alloca-4.c: New. > * gcc.target/aarch64/stack-check-alloca-5.c: New. > * gcc.target/aarch64/stack-check-alloca-6.c: New. > * gcc.target/aarch64/stack-check-alloca-7.c: New. > * gcc.target/aarch64/stack-check-alloca-8.c: New. > * gcc.target/aarch64/stack-check-alloca-9.c: New. > * gcc.target/aarch64/stack-check-alloca.h: New. > * gcc.target/aarch64/stack-check-14.c: New. > * gcc.target/aarch64/stack-check-15.c: New.
Hi All, I'm looking for permission to backport this patch to the GCC-8 branch to fix PR86486. OK for backport? Thanks, Tamar > -----Original Message----- > From: James Greenhalgh <james.greenhalgh@arm.com> > Sent: Tuesday, August 7, 2018 17:18 > To: Tamar Christina <Tamar.Christina@arm.com> > Cc: Jeff Law <law@redhat.com>; 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] Ensure that outgoing argument size is at > least 8 bytes when alloca and stack-clash. [Patch (3/6)] > > On Tue, Aug 07, 2018 at 05:09:34AM -0500, Tamar Christina wrote: > > Hi All, > > > > This is a re-spin to address review comments. No code change aside from a > variable rename. > > > > Ok for trunk? > > OK. > > Thanks, > James > > > gcc/ > > 2018-08-07 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * config/aarch64/aarch64.h > (STACK_CLASH_MIN_BYTES_OUTGOING_ARGS, > > STACK_DYNAMIC_OFFSET): New. > > * config/aarch64/aarch64.c (aarch64_layout_frame): > > Update outgoing args size. > > (aarch64_stack_clash_protection_alloca_probe_range, > > TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE): > New. > > > > gcc/testsuite/ > > 2018-08-07 Tamar Christina <tamar.christina@arm.com> > > > > PR target/86486 > > * gcc.target/aarch64/stack-check-alloca-1.c: New. > > * gcc.target/aarch64/stack-check-alloca-10.c: New. > > * gcc.target/aarch64/stack-check-alloca-2.c: New. > > * gcc.target/aarch64/stack-check-alloca-3.c: New. > > * gcc.target/aarch64/stack-check-alloca-4.c: New. > > * gcc.target/aarch64/stack-check-alloca-5.c: New. > > * gcc.target/aarch64/stack-check-alloca-6.c: New. > > * gcc.target/aarch64/stack-check-alloca-7.c: New. > > * gcc.target/aarch64/stack-check-alloca-8.c: New. > > * gcc.target/aarch64/stack-check-alloca-9.c: New. > > * gcc.target/aarch64/stack-check-alloca.h: New. > > * gcc.target/aarch64/stack-check-14.c: New. > > * gcc.target/aarch64/stack-check-15.c: New.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 1345f0eb171d05e2b833935c0a32f79c3db03f99..e9560b53bd8b5761855561dbf82d9c90cc1c282a 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -88,6 +88,10 @@ before probing has to be done for stack clash protection. */ #define STACK_CLASH_CALLER_GUARD 1024 +/* This value represents the minimum amount of bytes we expect the function's + outgoing arguments to be when stack-clash is enabled. */ +#define STACK_CLASH_OUTGOING_ARGS 8 + /* This value controls how many pages we manually unroll the loop for when generating stack clash probes. */ #define STACK_CLASH_MAX_UNROLL_PAGES 4 @@ -1069,4 +1073,15 @@ extern poly_uint16 aarch64_sve_vg; #define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE) +/* Allocate the minimum of STACK_CLASH_OUTGOING_ARGS if stack clash protection + is enabled for the outgoing arguments. This is essential as the extra args + space allows if to skip a check in alloca. */ +#undef STACK_DYNAMIC_OFFSET +#define STACK_DYNAMIC_OFFSET(FUNDECL) \ + ((flag_stack_clash_protection \ + && cfun->calls_alloca \ + && known_lt (crtl->outgoing_args_size, STACK_CLASH_OUTGOING_ARGS)) \ + ? ROUND_UP (STACK_CLASH_OUTGOING_ARGS, STACK_BOUNDARY / BITS_PER_UNIT) \ + : (crtl->outgoing_args_size + STACK_POINTER_OFFSET)) + #endif /* GCC_AARCH64_H */ diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 2f90ec624df89488489ce203022619d5484674e8..ae3c2fb85256b1e95e2242f3f16a027e918ba368 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4043,6 +4043,10 @@ aarch64_layout_frame (void) cfun->machine->frame.emit_frame_chain = aarch64_needs_frame_chain (); + /* Adjust the outgoing arguments size if required. Keep it in sync with what + the mid-end is doing. */ + crtl->outgoing_args_size = STACK_DYNAMIC_OFFSET (cfun); + #define SLOT_NOT_REQUIRED (-2) #define SLOT_REQUIRED (-1) @@ -4810,6 +4814,16 @@ aarch64_set_handled_components (sbitmap components) cfun->machine->reg_is_wrapped_separately[regno] = true; } +/* On AArch64 we have an ABI defined safe buffer for which we have to do no + probing. */ + +static HOST_WIDE_INT +aarch64_stack_clash_protection_alloca_probe_range (void) +{ + return STACK_CLASH_CALLER_GUARD; +} + + /* Allocate POLY_SIZE bytes of stack space using TEMP1 and TEMP2 as scratch registers. If POLY_SIZE is not large enough to require a probe this function will only adjust the stack. When allocation the stack space @@ -18359,6 +18373,10 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_CONSTANT_ALIGNMENT #define TARGET_CONSTANT_ALIGNMENT aarch64_constant_alignment +#undef TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE +#define TARGET_STACK_CLASH_PROTECTION_ALLOCA_PROBE_RANGE \ + aarch64_stack_clash_protection_alloca_probe_range + #undef TARGET_COMPUTE_PRESSURE_CLASSES #define TARGET_COMPUTE_PRESSURE_CLASSES aarch64_compute_pressure_classes diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-14.c b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c new file mode 100644 index 0000000000000000000000000000000000000000..a573d108a0ce0243e038411aa01eb65dfd81a460 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-14.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (4050); + x = t1 (x); + return p[x]; +} + + +/* This test has a constant sized alloca that is smaller than the + probe interval. Only one probe is required since the value is larger + than 1024 bytes but smaller than 63k. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 1 } } */ + + + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-15.c b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c new file mode 100644 index 0000000000000000000000000000000000000000..497d83e6596a3cbfce898ca6cda1a65d5fe76d0b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-15.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +int t1(int); + +int t2(int x) +{ + char *p = __builtin_alloca (x); + x = t1 (x); + return p[x]; +} + + +/* This test has a variable sized alloca. It requires 3 probes. + One in the loop, one for the residual, one for when it's < 1024 and one for + when it's not. + + The form can change quite a bit so we just check for two + probes without looking at the actual address. */ +/* { dg-final { scan-assembler-times "str\\txzr," 3 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c new file mode 100644 index 0000000000000000000000000000000000000000..524713a2c17f51eef0c58851661f9275f24212c9 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE y +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c new file mode 100644 index 0000000000000000000000000000000000000000..24553daf9eedd372c1766b884fd48bebd42de9b2 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-10.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127.5 * 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c new file mode 100644 index 0000000000000000000000000000000000000000..086fa86a4d2d6bffdca6474afa93c6a508000d58 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-2.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 0 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-not {str\s+xzr,} } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c new file mode 100644 index 0000000000000000000000000000000000000000..f5c20dad7ace049da0da6bfaafb5ec5e0b985e6e --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-3.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 100 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 8\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c new file mode 100644 index 0000000000000000000000000000000000000000..729d1184bc981a7fb00bc14d118ef526f96360b8 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-4.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 2 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c new file mode 100644 index 0000000000000000000000000000000000000000..2c08aa13dcf5cee5fedbd931535ae2776e422ad6 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-5.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c new file mode 100644 index 0000000000000000000000000000000000000000..2881e9666cebb1f5f3d5867d23ade5d4b9d8366c --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-6.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 63.5 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c new file mode 100644 index 0000000000000000000000000000000000000000..60f91c1c4dbc13c6f281e5d9ff2d89688dc942c7 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-7.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 64 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 1 } } */ + diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c new file mode 100644 index 0000000000000000000000000000000000000000..f1da22ccfecda1deb8a8611618b03debedf14426 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-8.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 65 * 1024 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c new file mode 100644 index 0000000000000000000000000000000000000000..4b4da189b782c61438715bf5a8f0b791de4e29dd --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca-9.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fstack-clash-protection --param stack-clash-protection-guard-size=16" } */ +/* { dg-require-effective-target supports_stack_clash_protection } */ + +#define SIZE 127 * 64 * 2014 +#include "stack-check-alloca.h" + +/* { dg-final { scan-assembler-times {str\s+xzr, \[sp, 1024\]} 2 } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h new file mode 100644 index 0000000000000000000000000000000000000000..a4f7fa2dd351cc7b76784103fc645a066c5ba6e3 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-check-alloca.h @@ -0,0 +1,13 @@ +#include <alloca.h> + +__attribute__((noinline, noipa)) +void g (char* ptr, int y) +{ + ptr[y] = '\0'; +} + +void f_caller (int y) +{ + char* pStr = alloca(SIZE); + g (pStr, y); +} \ No newline at end of file