[PATCH] D112768: [ARM] implement support for TLS register based stack protector

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3177-3179
+      if (!Args.hasArg(options::OPT_mstack_protector_guard_offset_EQ)) {
+        D.Diag(diag::err_drv_ssp_missing_offset_argument)
+            << A->getOption().getName() << Value;
----------------
Does GCC require these flags to be paired (`-mstack-protector-guard=` and `-mstack-protector-guard-offset`) ? Only for ARM and THUMB?  Doesn't the offset only make sense for sysreg and tls, or global, too?

Seems to me like `0` would be a good default offset, if unspecified.

Perhaps this would good to check then warn on for //all// supported architectures, and perhaps as another child patch? (or just default to `0` and not require `-mstack-protector-offset=`).


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3190-3191
+      }
+      CmdArgs.push_back("-target-feature");
+      CmdArgs.push_back("+read-tp-hard");
+    }
----------------
Isn't this redundant/set elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112768