From: Tamar Christina <tamar.christina@arm.com>
To: gcc-patches@gcc.gnu.org
Cc: nd@arm.com, james.greenhalgh@arm.com, Richard.Earnshaw@arm.com,
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)]
Date: Wed, 11 Jul 2018 11:21:00 -0000 [thread overview]
Message-ID: <20180711112121.GA15812@arm.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 3508 bytes --]
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.
--
[-- Attachment #2: rb9297.patch --]
[-- Type: text/x-diff, Size: 12189 bytes --]
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
next reply other threads:[~2018-07-11 11:21 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-11 11:21 Tamar Christina [this message]
2018-07-13 16:35 ` Tamar Christina
2018-07-25 11:09 ` Tamar Christina
2018-08-03 18:05 ` Jeff Law
2018-08-07 10:09 ` Tamar Christina
2018-08-07 16:18 ` James Greenhalgh
2018-10-09 6:38 ` Tamar Christina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180711112121.GA15812@arm.com \
--to=tamar.christina@arm.com \
--cc=Marcus.Shawcroft@arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=james.greenhalgh@arm.com \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).