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

[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