ardb 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;
----------------
nickdesaulniers wrote:
> 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=`).
> 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?
>
GCC does not implement this for ARM yet. For AArch64, it requires all three
options: guard, guard-reg and guard-offset.
> Seems to me like `0` would be a good default offset, if unspecified.
>
The default is INT_MAX at the moment, and I wasn't sure if we can simply change
that without breaking users.
> 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=`).
Yes, I think it makes sense to check this for AAch64 as well.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3190-3191
+ }
+ CmdArgs.push_back("-target-feature");
+ CmdArgs.push_back("+read-tp-hard");
+ }
----------------
nickdesaulniers wrote:
> Isn't this redundant/set elsewhere?
No, it is not. The alternative is requiring the user to pass -mtp=cp15 when
using the TLS stack protector, which is silly because it is implied.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112768/new/
https://reviews.llvm.org/D112768
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
