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

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 SINC

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

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 ___

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

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

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

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:

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

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 && Of

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

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"); + ---

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

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 m

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

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: > >

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

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 StackProtectorGuar

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

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.

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

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://

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

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) +

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

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 d

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

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 http

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

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.

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

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.StackPr

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

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.StackPr

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

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

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

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.StackPr

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

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

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

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/D10

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

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

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

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

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

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='

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

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 wro

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

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.o

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

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: inv

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

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 al

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

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 w

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

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://re

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

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 w

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

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)

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

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

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

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

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

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.StackPr

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

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

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

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.or

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

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" && Val

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

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 | \

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

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://rev

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

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/

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

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::lookup

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

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 __

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

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: llv