subject:"\[PATCH\] D100919\: \[AArch64\] Support customizing stack protector guard"

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread Nick Desaulniers via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f417789192e: [AArch64] Support customizing stack protector 
guard (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,105 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-257-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-MINUS-257-OFFSET %s
+
+; XFAIL
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=32761 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-4096 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=4097 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-257-OFFSET:  add x8, x8, #257
+; CHECK-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-MINUS-257-OFFSET:  sub x8, x8, #257
+; CHECK-MINUS-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-257-OFFSET:  add x9, x9, #257
+; CHECK-257-OFFSET-NEXT: ldr x9, [x9]
+; CHECK-MINUS-257-OFFSET:

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

Appreciate the reviews; ++beers_owed;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1954
+  // than 23760.
+  // It might be nice to use AArch64::MOVi32imm here, which would get
+  // expanded in PreSched2 after PostRA, but our lone scratch Reg already

dmgreen wrote:
> It may be possible to do something earlier, where a we add the MOVi32imm 
> earlier before registry allocation and use that as the register for the ldr 
> offset. That would keep it simpler than trying to find scratch registers, and 
> may be optimized better by the pipeline.
Sure; added that as a comment; hopefully we wont need it, but "never say never 
(again)."


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 345949.
nickdesaulniers added a comment.

- braces, typo fix, add comment about additional approaches


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,105 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-257-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-MINUS-257-OFFSET %s
+
+; XFAIL
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=32761 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-4096 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=4097 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-257-OFFSET:  add x8, x8, #257
+; CHECK-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-MINUS-257-OFFSET:  sub x8, x8, #257
+; CHECK-MINUS-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-257-OFFSET:  add x9, x9, #257
+; CHECK-257-OFFSET-NEXT: ldr x9, [x9]
+; CHECK-MINUS-257-OFFSET:  sub x9, x9, #257
+; CHECK-MINUS-257-OFFSET-NEXT: ldr x9, [x9]
+; CHECK-NEXT:cmp x9, x8

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM with one nit.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1919
+int Offset = Options.StackProtectorGuardOffset;
+if (Offset >= 0 && Offset <= 32760 && Offset % 8 == 0)
+  // ldr xN, [xN, #offset]

For all the if/else here I'd include the braces. I know that once the 
preprocessor is done it'll be a one line if but visually it's confusing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-17 Thread Dave Green via Phabricator via cfe-commits

dmgreen accepted this revision.
dmgreen added a comment.

Thanks. This sounds good to me, if David agrees.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1912
+if (!SrcReg)
+  report_fatal_error("Unknow SysReg for Stack Protector Guard Register");
+

-> Unknown



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1954
+  // than 23760.
+  // It might be nice to use AArch64::MOVi32imm here, which would get
+  // expanded in PreSched2 after PostRA, but our lone scratch Reg already

It may be possible to do something earlier, where a we add the MOVi32imm 
earlier before registry allocation and use that as the register for the ldr 
offset. That would keep it simpler than trying to find scratch registers, and 
may be optimized better by the pipeline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-14 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 345587.
nickdesaulniers added a comment.
Herald added a subscriber: dang.

- support up to +/- 4096 non-multiples of 8. We could get crazy with psuedo 
instructions and register scavenging, but this is really overkill; the very 
first case of positive multiple's of 8 < 32670 will be 99% of the kernel's use 
case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,105 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-257-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-257 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-MINUS-257-OFFSET %s
+
+; XFAIL
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=32761 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-4096 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+; RUN: not --crash llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=4097 -o - 2>&1 | \
+; RUN: FileCheck --check-prefix=CHECK-BAD-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-257-OFFSET:  add x8, x8, #257
+; CHECK-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-MINUS-257-OFFSET:  sub x8, x8, #257
+; CHECK-MINUS-257-OFFSET-NEXT: ldr x8, [x8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldur x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9,

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1917
+.addDef(Reg)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else

dmgreen wrote:
> nickdesaulniers wrote:
> > dmgreen wrote:
> > > What is StackProtectorGuardOffset, and could is be out of range for the 
> > > load? That is something that verify-machineinstrs won't currently check 
> > > for.
> > It's passed from the front end; it's an offset added to the pointer load.  
> > The Linux kernel uses a value in 1100-1200 range. GCC will let you use 
> > whatever large value and keep appending adds for constants greater than 
> > 32200: https://godbolt.org/z/1xhbEPbKG, but YAGNI.
> OK. It might be worth at least issuing a report_fatal_error if it is out of 
> the range of the instructions handled here. Otherwise the error might be 
> non-obvious, it just truncating the immediate during codegen.
Indeed, in particular it looks like the range of LDR is 0-32760 which is 
sufficient for the kernel, but the range for LDUR is only -256-255, which is 
not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread Dave Green via Phabricator via cfe-commits

dmgreen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1917
+.addDef(Reg)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else

nickdesaulniers wrote:
> dmgreen wrote:
> > What is StackProtectorGuardOffset, and could is be out of range for the 
> > load? That is something that verify-machineinstrs won't currently check for.
> It's passed from the front end; it's an offset added to the pointer load.  
> The Linux kernel uses a value in 1100-1200 range. GCC will let you use 
> whatever large value and keep appending adds for constants greater than 
> 32200: https://godbolt.org/z/1xhbEPbKG, but YAGNI.
OK. It might be worth at least issuing a report_fatal_error if it is out of the 
range of the instructions handled here. Otherwise the error might be 
non-obvious, it just truncating the immediate during codegen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D100919#2750125 , @dmgreen wrote:

> It's worth adding -verify-machineinstrs to the tests, to check the code 
> passes the internal checks.

Done.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1917
+.addDef(Reg)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else

dmgreen wrote:
> What is StackProtectorGuardOffset, and could is be out of range for the load? 
> That is something that verify-machineinstrs won't currently check for.
It's passed from the front end; it's an offset added to the pointer load.  The 
Linux kernel uses a value in 1100-1200 range. GCC will let you use whatever 
large value and keep appending adds for constants greater than 32200: 
https://godbolt.org/z/1xhbEPbKG, but YAGNI.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 344549.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- reorder USE/DEF, add -verify-machineinstrs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -verify-machineinstrs -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addDef(Reg, RegState::Renamable)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addDef(Reg)
+.addUse(Reg, RegState::Kill)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread Dave Green via Phabricator via cfe-commits

dmgreen added a comment.

I don't know a huge amount about stack protectors.

It's worth adding -verify-machineinstrs to the tests, to check the code passes 
the internal checks.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1917
+.addDef(Reg)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else

What is StackProtectorGuardOffset, and could is be out of range for the load? 
That is something that verify-machineinstrs won't currently check for.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1920
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addUse(Reg, RegState::Kill)
+.addDef(Reg)

Defs come before Uses in machine instructions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-11 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett requested changes to this revision.
DavidSpickett added a comment.
This revision now requires changes to proceed.

This is probably not showing up in review queues as it got accepted 
accidentally at some point. I'm going to "Request Changes" then I think you'll 
need to refresh the diff Nick to get it back into active review.
(I don't know of any other way to "give up" an accept revision)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-10 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

bumping for review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-05 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett added a reviewer: ostannard.
DavidSpickett added a subscriber: ostannard.
DavidSpickett added a comment.

All my comments have been answered. Perhaps @ostannard can check 
AArch64InstrInfo.cpp ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1914-1915
+  BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+  .addReg(Reg, getKillRegState(false))
+  .addReg(Reg, RegState::Define)
+  .addImm(Options.StackProtectorGuardOffset >> 3);

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > @efriedma can you please check that I have these flags correct? I'm not 
> > > sure that I do.  Also, I suspect that I can't allocated a virtual 
> > > register for the result of the LDR, since this is post-RA?
> > @efriedma bumping for review; otherwise can you suggest another reviewer?
> In particular, I'm most curious about the use of `RegState::Renamable`; the 
> result of reading via `mrs` is a temporary; but this is post-RA, so isn't it 
> too late for further register renaming?
Additionally, if it's a temporary, should it be `RegState::Implicit`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1914-1915
+  BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+  .addReg(Reg, getKillRegState(false))
+  .addReg(Reg, RegState::Define)
+  .addImm(Options.StackProtectorGuardOffset >> 3);

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > @efriedma can you please check that I have these flags correct? I'm not 
> > sure that I do.  Also, I suspect that I can't allocated a virtual register 
> > for the result of the LDR, since this is post-RA?
> @efriedma bumping for review; otherwise can you suggest another reviewer?
In particular, I'm most curious about the use of `RegState::Renamable`; the 
result of reading via `mrs` is a temporary; but this is post-RA, so isn't it 
too late for further register renaming?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 342921.
nickdesaulniers added a comment.

- prefer addDef, explicit register kills


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addDef(Reg, RegState::Renamable)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addUse(Reg, RegState::Kill)
+.addDef(Reg)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addUse(Reg, RegState::Kill)
+.addDef(Reg)
+

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1914-1915
+  BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+  .addReg(Reg, getKillRegState(false))
+  .addReg(Reg, RegState::Define)
+  .addImm(Options.StackProtectorGuardOffset >> 3);

nickdesaulniers wrote:
> @efriedma can you please check that I have these flags correct? I'm not sure 
> that I do.  Also, I suspect that I can't allocated a virtual register for the 
> result of the LDR, since this is post-RA?
@efriedma bumping for review; otherwise can you suggest another reviewer?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll:20
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+

DavidSpickett wrote:
> What does NPOT stand for here, something to do with the offset not being a 
> multiple of 8 bytes?
Non Power Of Two


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-05-04 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 342905.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- rebase, update comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg, RegState::Define)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addReg(Reg, getKillRegState(false))
+

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-28 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:385
   /// On x86 this can be "fs" or "gs".
+  /// On AArch64 this can be any value of llvm::AArch64SysReg::SysReg.
   std::string StackProtectorGuardReg;

This is now used for TLS on x86 and sysreg on AArch64 so how about:
```
  /// The TLS base register when StackProtectorGuard is "tls", or register
  /// used to store the stack canary for "sysreg".
  /// On x86 this can be "fs" or "gs".
  /// On AArch64 this can only be "sp_el0".
```




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3110
+}
+if (EffectiveTriple.isAArch64() && Value != "sysreg" && Value != "global") 
{
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> DavidSpickett wrote:
> > Shouldn't this also allow "tls"? At least that's what the previous code 
> > works out to, I don't know if that actually works on AArch64 or if it just 
> > didn't error.
> I don't think so; GCC seems to support `tls` for x86 but not for aarch64.
> https://godbolt.org/z/6WjEPfhT5
Fair enough, seems unlikely it would work in any case.



Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll:20
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+

What does NPOT stand for here, something to do with the offset not being a 
multiple of 8 bytes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 340932.
nickdesaulniers added a comment.

- rebase on D101387 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg, RegState::Define)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg,

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers marked an inline comment as not done.
nickdesaulniers added inline comments.



Comment at: clang/test/Driver/stack-protector-guard.c:59
+// INVALID-VALUE-AARCH64: error: invalid value 'tls' in 
'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' 
are:sysreg global'
+// INVALID-REG-AARCH64: error: invalid value 'foo' in 
'mstack-protector-guard-reg=','for AArch64'

nickdesaulniers wrote:
> DavidSpickett wrote:
> > I'm not sure if this is due to your code or the error machinery itself but 
> > these errors are strangely written.
> > 
> > I'd expect:
> > ```
> > error: invalid value 'tls' in 'mstack-protector-guard='tls', valid 
> > arguments to '-mstack-protector-guard=' are:sysreg global'
> > ```
> > 
> > Maybe it's assuming that there could be multiple values and `','` means 
> > that list option treats the value as a list? Or it's not using the right 
> > value and the comma is meant to be after the `=''` as in my example.
> Sure, `err_drv_invalid_value` would be better.
Ah, `err_drv_invalid_value_with_suggestion` puts single quotes around the third 
parameter `%2`.  Let me send a patch cleaning that up, then I'll rebase this on 
that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

DavidSpickett wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > nickdesaulniers wrote:
> > > > > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the 
> > > > > > frontend?
> > > > > I don't think so because 
> > > > > `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is under lib/ not 
> > > > > include/. Not sure if I should just remove reg validation?
> > > > Guidance provided by @echristo and @jyknight was that we should avoid 
> > > > such linkage requirements on Target/, so instead I'll work on adding a 
> > > > helper to clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates 
> > > > logic from `AArch64SysReg::lookupSysRegByName`.
> > > It looks like there's ~1000 possible sysregs for aarch64 ATM; do we 
> > > really want to add all of those to clang?
> > I'm going to post that as a separate commit/review on top of this series, 
> > that way it doesn't pollute this code review. This is ready to be reviewed.
> If the number of different registers people actually use with this option is 
> somewhere < 10 I'd just hardcode the names here as needed. (a large amount of 
> those sysregs won't be suitable for this purpose anyway)
Right, I figure that can be a separate decision from the rest of the 
implementation, I've forked that off in https://reviews.llvm.org/D101327. 
Whether it lands or not doesn't matter to me.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3110
+}
+if (EffectiveTriple.isAArch64() && Value != "sysreg" && Value != "global") 
{
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

DavidSpickett wrote:
> Shouldn't this also allow "tls"? At least that's what the previous code works 
> out to, I don't know if that actually works on AArch64 or if it just didn't 
> error.
I don't think so; GCC seems to support `tls` for x86 but not for aarch64.
https://godbolt.org/z/6WjEPfhT5



Comment at: clang/test/Driver/stack-protector-guard.c:59
+// INVALID-VALUE-AARCH64: error: invalid value 'tls' in 
'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' 
are:sysreg global'
+// INVALID-REG-AARCH64: error: invalid value 'foo' in 
'mstack-protector-guard-reg=','for AArch64'

DavidSpickett wrote:
> I'm not sure if this is due to your code or the error machinery itself but 
> these errors are strangely written.
> 
> I'd expect:
> ```
> error: invalid value 'tls' in 'mstack-protector-guard='tls', valid arguments 
> to '-mstack-protector-guard=' are:sysreg global'
> ```
> 
> Maybe it's assuming that there could be multiple values and `','` means that 
> list option treats the value as a list? Or it's not using the right value and 
> the comma is meant to be after the `=''` as in my example.
Sure, `err_drv_invalid_value` would be better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 340905.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.

- prefer err_drv_invalid_value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg, RegState::Define)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addReg(Reg, getKillRegState(false))

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett added inline comments.



Comment at: clang/test/Driver/stack-protector-guard.c:59
+// INVALID-VALUE-AARCH64: error: invalid value 'tls' in 
'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' 
are:sysreg global'
+// INVALID-REG-AARCH64: error: invalid value 'foo' in 
'mstack-protector-guard-reg=','for AArch64'

I'm not sure if this is due to your code or the error machinery itself but 
these errors are strangely written.

I'd expect:
```
error: invalid value 'tls' in 'mstack-protector-guard='tls', valid arguments to 
'-mstack-protector-guard=' are:sysreg global'
```

Maybe it's assuming that there could be multiple values and `','` means that 
list option treats the value as a list? Or it's not using the right value and 
the comma is meant to be after the `=''` as in my example.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-27 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3110
+}
+if (EffectiveTriple.isAArch64() && Value != "sysreg" && Value != "global") 
{
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

Shouldn't this also allow "tls"? At least that's what the previous code works 
out to, I don't know if that actually works on AArch64 or if it just didn't 
error.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the 
> > > > > frontend?
> > > > I don't think so because 
> > > > `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is under lib/ not 
> > > > include/. Not sure if I should just remove reg validation?
> > > Guidance provided by @echristo and @jyknight was that we should avoid 
> > > such linkage requirements on Target/, so instead I'll work on adding a 
> > > helper to clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates 
> > > logic from `AArch64SysReg::lookupSysRegByName`.
> > It looks like there's ~1000 possible sysregs for aarch64 ATM; do we really 
> > want to add all of those to clang?
> I'm going to post that as a separate commit/review on top of this series, 
> that way it doesn't pollute this code review. This is ready to be reviewed.
If the number of different registers people actually use with this option is 
somewhere < 10 I'd just hardcode the names here as needed. (a large amount of 
those sysregs won't be suitable for this purpose anyway)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-26 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?
> > > I don't think so because 
> > > `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is under lib/ not 
> > > include/. Not sure if I should just remove reg validation?
> > Guidance provided by @echristo and @jyknight was that we should avoid such 
> > linkage requirements on Target/, so instead I'll work on adding a helper to 
> > clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates logic from 
> > `AArch64SysReg::lookupSysRegByName`.
> It looks like there's ~1000 possible sysregs for aarch64 ATM; do we really 
> want to add all of those to clang?
I'm going to post that as a separate commit/review on top of this series, that 
way it doesn't pollute this code review. This is ready to be reviewed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-26 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 340652.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- add support for NPOT and negative offsets


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg, RegState::Define)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addReg(Reg,

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-26 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?
> > I don't think so because `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` 
> > is under lib/ not include/. Not sure if I should just remove reg validation?
> Guidance provided by @echristo and @jyknight was that we should avoid such 
> linkage requirements on Target/, so instead I'll work on adding a helper to 
> clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates logic from 
> `AArch64SysReg::lookupSysRegByName`.
It looks like there's ~1000 possible sysregs for aarch64 ATM; do we really want 
to add all of those to clang?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-26 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added subscribers: jyknight, echristo.
nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?
> I don't think so because `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is 
> under lib/ not include/. Not sure if I should just remove reg validation?
Guidance provided by @echristo and @jyknight was that we should avoid such 
linkage requirements on Target/, so instead I'll work on adding a helper to 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates logic from 
`AArch64SysReg::lookupSysRegByName`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3114
 
   if (Arg *A = Args.getLastArg(options::OPT_mstack_protector_guard_offset_EQ)) 
{
 StringRef Value = A->getValue();

nickdesaulniers wrote:
> TODO: GCC treats this as mutually exclusive when 
> OPT_mstack_protector_guard_EQ == global.
And it does so inconsistently; it errors for aarch64, but does literally 
nothing (no warning, no change to codegen) for x86. I don't plan to be bug 
compatible here.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?
I don't think so because `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is 
under lib/ not include/. Not sure if I should just remove reg validation?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 340212.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- combine tests, clean up Clang drivers slightly


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,55 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,24 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+  .addReg(Reg, getKillRegState(false))
+  .addReg(Reg, RegState::Define)
+  .addImm(Options.StackProtectorGuardOffset >> 3);
+  MBB.erase(MI);
+  return true;
+}
+  }
+
   const GlobalValue *GV =
   cast((*MI.memoperands_begin())->getValue());
   const TargetMachine  = MBB.getParent()->getTarget();
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -503,6 +503,8 @@
 return StackProtectorGuards::TLS;
   if (getStackProtectorGuard() == "global")
 return StackProtectorGuards::Global;
+  if (getStackProtectorGuard() == "sysreg")
+return StackProtectorGuards::SysReg;
   if (getStackProtectorGuard() != "none") {
 ErrorOr> MBOrErr =
 MemoryBuffer::getFile(getStackProtectorGuard());
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -73,11 +73,7 @@
 None// Do not use Basic Block Sections.
   };
 
-  enum class StackProtectorGuards {
-None,
-TLS,
-Global
-  };
+  enum

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1914-1915
+  BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+  .addReg(Reg, getKillRegState(false))
+  .addReg(Reg, RegState::Define)
+  .addImm(Options.StackProtectorGuardOffset >> 3);

@efriedma can you please check that I have these flags correct? I'm not sure 
that I do.  Also, I suspect that I can't allocated a virtual register for the 
result of the LDR, since this is post-RA?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg-nonzero-offset.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s --stack-protector-guard=sysreg \

This can be combined with the other test by using different CHECK prefixes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 340192.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- fix 32b ARM triple, add test for non-zero offset, actually load ptr


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg-nonzero-offset.ll
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,49 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NEXT:ldr x8, [x8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NEXT:ldr x9, [x9]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg-nonzero-offset.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg-nonzero-offset.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | FileCheck %s
+; TODO: add test for negative offset; StackProtectorGuardOffset should probably
+; be unsigned.
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NEXT:ldr x8, [x8, #8]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NEXT:ldr x9, [x9, #8]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,24 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.
Herald added a subscriber: tmatheson.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3104
   << A->getAsString(Args) << TripleStr;
-if (Value != "tls" && Value != "global") {
+if (Value != "tls" && Value != "global" && Value != "sysreg") {
   D.Diag(diag::err_drv_invalid_value_with_suggestion)

TODO: tls is not supported on aarch64 in GCC.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3114
 
   if (Arg *A = Args.getLastArg(options::OPT_mstack_protector_guard_offset_EQ)) 
{
 StringRef Value = A->getValue();

TODO: GCC treats this as mutually exclusive when OPT_mstack_protector_guard_EQ 
== global.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-21 Thread Kristof Beyls via Phabricator via cfe-commits

kristof.beyls added inline comments.



Comment at: clang/test/Driver/stack-protector-guard.c:26
 
-// RUN: not %clang -target aarch64-linux-gnu -mstack-protector-guard-offset=10 
%s 2>&1 | \
+// RUN: not %clang -target arm-linux-gnuebi -mstack-protector-guard-offset=10 
%s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-ARCH3 %s

I guess you meant to type "-target arm-linux-gnueabi"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-20 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers planned changes to this revision.
nickdesaulniers added a comment.

Pretty sure I need to do something with non-zero values of 
`-mstack-protector-guard-offset=0`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-20 Thread Xiang Zhang via Phabricator via cfe-commits

xiangzhangllvm accepted this revision.
xiangzhangllvm added a comment.
This revision is now accepted and ready to land.

I didn't find any problem in the main context of the patch, +1 first.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-20 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-20 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

I still have some todos that I will get to tomorrow, but posting this early for 
review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100919/new/

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

2021-04-20 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers created this revision.
nickdesaulniers added reviewers: xiangzhangllvm, efriedma, MaskRay.
Herald added subscribers: dexonsmith, danielkiss, hiraditya, kristof.beyls.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Follow up to D88631  but for aarch64; the 
Linux kernel uses the command
line flags:

1. -mstack-protector-guard=sysreg
2. -mstack-protector-guard-reg=sp_el0
3. -mstack-protector-guard-offset=0

to use the system register sp_el0 for the stack canary, enabling the
kernel to have a unique stack canary per task (like a thread, but not
limited to userspace as the kernel can preempt itself).

Address pr/47341 for aarch64.

Fixes: https://github.com/ClangBuiltLinux/linux/issues/289
Signed-off-by: Nick Desaulniers 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,47 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1752,6 +1752,21 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  // TODO: if we're given a non-sense StackProtectorGuardReg, so
+  // SrcReg is nullptr, what should we do?
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, getKillRegState(false))
+  .addImm(SrcReg->Encoding);
+  MBB.erase(MI);
+  return true;
+}
+  }
+
   const GlobalValue *GV =
   cast((*MI.memoperands_begin())->getValue());
   const TargetMachine  = MBB.getParent()->getTarget();
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -502,6 +502,8 @@
 return StackProtectorGuards::TLS;
   if (getStackProtectorGuard() == "global")
 return StackProtectorGuards::Global;
+  if (getStackProtectorGuard() == "sysreg")
+return StackProtectorGuards::SysReg;
   if (getStackProtectorGuard() != "none") {
 ErrorOr> MBOrErr =
 MemoryBuffer::getFile(getStackProtectorGuard());
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -73,11 +73,7 @@
 None// Do not use

44 matches

Mail list logo