[PATCH] D84414: [RISCV] Support Shadow Call Stack

Tue Aug 25 10:50:01 PDT 2020

zzheng added a comment.

In D84414#2234267 <https://reviews.llvm.org/D84414#2234267>, @lenary wrote:

> Ok, so any compilation units without `-fsanitize=shadow-call-stack` should be compiled with `-ffixed-x18` if you want to link those together. That is reasonable. My question was whether we can ensure that `-fsanitize=shadow-call-stack` can imply `-ffixed-x18` rather than having to pass both options.
>
> It is my understanding that all functions in a CU with `-fsanitize=shadow-call-stack` will get the SCS function attribute, so why can't we use that attribute to work out whether x18 should be reserved, rather than using `-ffixed-x18`? You'll see `RISCVRegisterInfo::getReservedRegs` reserves `fp` and `bp` only if they're needed by the function (which can be based on attributes or other info), rather than using `isRegisterReservedByUser` - which seems cleaner to me.

In D84414#2234327 <https://reviews.llvm.org/D84414#2234327>, @pcc wrote:

> FWIW, on aarch64 we decided to make `-fsanitize=shadow-call-stack` require the x18 reservation (instead of implying it) to try to avoid ABI mismatch problems. That is, it should be safe to mix and match code compiled with and without `-fsanitize=shadow-call-stack`. If we make `-fsanitize=shadow-call-stack` imply the x18 reservation, it makes it more likely that someone will accidentally build and link in incompatible code that does not reserve x18.

Yes, binding '-fsanitize=shadow-call-stack' with '-ffixed-x18' is to ensure user don't accidentally link SCS-enabled code with non-SCS code that uses/overwrites x18. We can always infer that x18 is reserved when SCS in on.

There's no need to check `RISCVRegisterInfo::getReservedRegs` for x18 by looking into function attr. It forms a circular chain: is x18 reserved for SCS? <--> SCS is enabled, so x18 should be reserved.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414