diff mbox

[AArch64] : Add constraint letter for stack_protect_test pattern)

Message ID 1410964981-32412-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Sept. 17, 2014, 2:43 p.m. UTC
On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote:
> "=&r" is correct for an early-clobbered scratch.
>
> R.

In that case...

How is the attached patch for trunk? I've bootstrapped it on AArch64
with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS
without seeing any issues.

OK?

Thanks,
James

---
gcc/

2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark
	scratch register as an output to placate register renaming.

gcc/testsuite/

2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>

	* gcc.target/aarch64/stack_protector_set_1.c: New.
	* gcc.target/aarch64/stack_protector_set_2.c: Likewise.

Comments

Andrew Pinski Sept. 17, 2014, 2:50 p.m. UTC | #1
> On Sep 17, 2014, at 7:43 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> 
> 
>> On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote:
>> "=&r" is correct for an early-clobbered scratch.
>> 
>> R.
> 
> In that case...
> 
> How is the attached patch for trunk? I've bootstrapped it on AArch64
> with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS
> without seeing any issues.
> 
> OK?
> 
> Thanks,
> James
> 
> ---
> gcc/
> 
> 2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>
> 
>   * config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark
>   scratch register as an output to placate register renaming.
> 
> gcc/testsuite/
> 
> 2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>
> 
>   * gcc.target/aarch64/stack_protector_set_1.c: New.
>   * gcc.target/aarch64/stack_protector_set_2.c: Likewise.

There is nothing aarch64 specific about this testcase so I would place them under gcc.dg and add the extra marker which says this testcase requires stack protector.   And maybe even use compile instead of just assemble too. 

Thanks,
Andrew

> <0001-Re-PATCH-AArch64-Add-constraint-letter-for-stack_pro.patch>
Marcus Shawcroft Sept. 18, 2014, 8:18 a.m. UTC | #2
On 17 September 2014 15:43, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>
> On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote:
>> "=&r" is correct for an early-clobbered scratch.
>>
>> R.
>
> In that case...
>
> How is the attached patch for trunk? I've bootstrapped it on AArch64
> with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS
> without seeing any issues.
>
> OK?
>
> Thanks,
> James
>
> ---
> gcc/
>
> 2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark
>         scratch register as an output to placate register renaming.


OK for this part.


> gcc/testsuite/
>
> 2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * gcc.target/aarch64/stack_protector_set_1.c: New.
>         * gcc.target/aarch64/stack_protector_set_2.c: Likewise.

I agree with Andrew, these don't need to be aarch64 specific.

/Marcus
Jakub Jelinek Sept. 18, 2014, 9:05 a.m. UTC | #3
On Thu, Sep 18, 2014 at 09:18:53AM +0100, Marcus Shawcroft wrote:
> > gcc/testsuite/
> >
> > 2014-09-15  James Greenhalgh  <james.greenhalgh@arm.com>
> >
> >         * gcc.target/aarch64/stack_protector_set_1.c: New.
> >         * gcc.target/aarch64/stack_protector_set_2.c: Likewise.
> 
> I agree with Andrew, these don't need to be aarch64 specific.

Well, guess the 16 needs to be replaced with sizeof buffer, because
sizeof (unsigned int) is not 4 on all architectures.
And
/* { dg-require-effective-target fstack_protector } */
is needed, not all targets support -fstack-protector*.

	Jakub
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c60038a9015d614f40f6d9e3fd228ad3e2b247a8..f15a516bb0559c86bea7512f91d60dc179ec9149 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4031,7 +4031,7 @@  (define_insn "stack_protect_test_<mode>"
 	(unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")
 		     (match_operand:PTR 2 "memory_operand" "m")]
 	 UNSPEC_SP_TEST))
-   (clobber (match_scratch:PTR 3 "&r"))]
+   (clobber (match_scratch:PTR 3 "=&r"))]
   ""
   "ldr\t%<w>3, %x1\;ldr\t%<w>0, %x2\;eor\t%<w>0, %<w>3, %<w>0"
   [(set_attr "length" "12")
diff --git a/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c
new file mode 100644
index 0000000..df0d26b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_1.c
@@ -0,0 +1,15 @@ 
+/* { dg-do assemble } */
+/* { dg-options "-fstack-protector-strong -O1 -frename-registers" } */
+
+extern int bar (const char *s, int *argc);
+extern int baz (const char *s);
+
+char
+foo (const char *s)
+{
+  int argc;
+  int ret;
+  if ( !bar (s, &argc))
+    ret = baz (s);
+  return *s;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c
new file mode 100644
index 0000000..b94a2d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack_protector_set_2.c
@@ -0,0 +1,17 @@ 
+/* { dg-do assemble } */
+/* { dg-options "-fstack-protector-strong -O1 -frename-registers" } */
+
+typedef unsigned int uint32_t;
+struct ctx
+{
+  uint32_t A;
+};
+
+void *
+buffer_copy (const struct ctx *ctx, void *resbuf)
+{
+  uint32_t buffer[4];
+  buffer[0] = (ctx->A);
+  __builtin_memcpy (resbuf, buffer, 16);
+  return resbuf;
+}