[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148034#4264996 <https://reviews.llvm.org/D148034#4264996>, @asb wrote:

> In D148034#4262991 <https://reviews.llvm.org/D148034#4262991>, @MaskRay wrote:
>
>> In D148034#4260376 <https://reviews.llvm.org/D148034#4260376>, @asb wrote:
>>
>>> Will `--[no-]relax-gp` make its way into a minor gcc point release or do we need to wait for the next major release?
>>>
>>> In terms of this breaking GNU users - isn't it the case that without this option, they may get silently broken code when using the shadow call stack? Breaking loudly and early seems preferable, though of course it would be best if it's easily fixable by e.g. updating to a newer released binutils.
>>
>> Yes, `-fsanitize=shadow-call-stack` using gp users will get silently broken code if linked with GNU ld, unless GNU ld is specified, or `-Wl,--no-relax` or `-Wl,--no-relax-gp` is specified.
>> This is an instance of the guideline proposed in https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/371/commits/bb0df41a4f2626fa039173c2a975039905dce99c
>>
>>> For such platforms, care must be taken to ensure all code (compiler generated or otherwise) avoids using gp in a way incompatible with the platform specific purpose, and that global pointer relaxation is disabled in the toolchain.
>>
>> Personally I think most `-fsanitize=shadow-call-stack`  users do not use GNU ld, so this incompatibility is actually minor.
>>
>> `-fsanitize=shadow-call-stack` is already a quite specific configuration. For GNU ld users, I think placing the burden more on user education is fine (sorry, just that we don't have better options).
>
> I just wanted to make sure I'm following your suggestion here. Have you come round to the idea of adding `--no-relax-gp` when shadow call stack is enabled and letting it error for users of old GNU tools on the basis that there aren't better options?

I added `--no-relax-gp` because I imagined many use cases not wanting GNU ld's default global pointer relaxation behavior.
(I had had the patch long before it finally landed. I did not understand Andrew's stance and objection and what he would accept. If anyone is curious, please dig up the original binutils thread.)

I don't think enabling software shadow call stack should pass `--no-relax-gp` to the linker: the driver isn't in a good position to know whether the linker is a latest GNU ld that supports `--no-relax-gp`.
GNU ld users may specify `-Wl,--no-relax` and don't need `--no-relax-gp`. Clang Driver should not inspect `-Wl,` options to make a "smart" decision.

The compatibility burden lies on the user side. Users should be aware of the GNU ld's default behavior possibly causing problems.
Clang Driver being smart is more likely to get in the way. As mentioned, `clang -fsanitize=shadow-call-stack` users likely don't use GNU ld, so the pitfall situation is not great but I think is acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148034