[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

This revision was automatically updated to reflect the committed changes. Closed by commit rGaa1d2693c256: [CodeGen][RISCV] Change Shadow Call Stack Register to X3 (authored by paulkirth). Changed prior to commit: https://reviews.llvm.org/D146463?vs=512489&id=512972#toc Repository: rG LLVM G

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 512489. paulkirth marked 2 inline comments as done. paulkirth added a comment. Add spaces to parantheticals. Shorten link to discussion on psABI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ http

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 512277. paulkirth added a comment. Remove dead declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D146463 Files: clang/docs/ShadowCallStack.rst clang/lib/Driv

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:573 + Kinds & SanitizerKind::ShadowCallStack) + << "-msmall-data-limit=0"; +} jrtc27 wrote: > jrtc27 wrote: > > paulkirth wrote: > > > jrt

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 512276. paulkirth marked 7 inline comments as done. paulkirth added a comment. Remove `-msmall-data-limit` related changes. There was a misunderstanding on my part about default linker behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

jrtc27 added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:573 + Kinds & SanitizerKind::ShadowCallStack) + << "-msmall-data-limit=0"; +} jrtc27 wrote: > paulkirth wrote: > > jrtc27 wrote: > > > Why is

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

jrtc27 added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:573 + Kinds & SanitizerKind::ShadowCallStack) + << "-msmall-data-limit=0"; +} paulkirth wrote: > jrtc27 wrote: > > Why is this an error? It m

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:307 +bool shadowCallStackDSLConflicts(const llvm::opt::ArgList &Args, + const llvm::Triple &TT) { mcgrathr wrote: > I'm guessing that "DSL" stands fo

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

jrtc27 added inline comments. Comment at: clang/docs/ShadowCallStack.rst:157 +linker. This can be done with the ``--no-relax-gp`` flag in GNU ld. It may also +be useful to compile with ``-msmall-data-limit=0``. This doesn't really achieve anything other than no

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr accepted this revision. mcgrathr added a comment. lgtm with some nits and a few more test cases. Comment at: clang/docs/ShadowCallStack.rst:61 +The instrumentation makes use of the platform register ``x18`` on AArch64 and +``x3`` on RISC-V. For simplicity we will refer

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

asb accepted this revision. asb added a comment. In D146463#4255927 , @paulkirth wrote: > @asb Are we happy with the state of consensus w.r.t. using `x3`? I think the > lingering concerns from the psABI discussion have been resolved. Yes, all LGTM now.

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth added a comment. @asb Are we happy with the state of consensus w.r.t. using `x3`? I think the lingering concerns from the psABI discussion have been resolved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llv

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 511704. paulkirth marked an inline comment as done. paulkirth added a comment. Remove unrelated whitespace changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D146463

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 511575. paulkirth marked an inline comment as done. paulkirth added a comment. Add default small-data-limit for Android and Fuchsia. - update tests - update clang driver Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 511466. paulkirth marked 2 inline comments as done. paulkirth added a comment. Remove extra line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D146463 Files: clang/doc

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 511458. paulkirth added a comment. Fix formatting and update documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D146463 Files: clang/docs/ShadowCallStack.rst

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

asb added a comment. Ok, looks like the consensus I thought we had isn't quite there on the psABI thread - Andrew Waterman has some concerns. We should let that discussion play out some more. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/ne

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

asb added a comment. In D146463#4247050 , @paulkirth wrote: > @asb, @craig.topper, @jrtc27 Are there any remaining considerations for us > here? From the discussions in psABI and sig-toolchain, I think we have a > consensus that this is the approach we

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

jrtc27 added a comment. From an ABI/codegen perspective the small data limit doesn’t matter here, all it does is govern whether things go in .sdata or not. The linker is what chooses whether to use GP, which happens regardless of whether something is in .sdata, just things in .sdata are more li

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth added a comment. whe Comment at: clang/lib/Driver/SanitizerArgs.cpp:546 - if ((Kinds & SanitizerKind::ShadowCallStack) && - ((TC.getTriple().isAArch64() && -!llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) || - (TC.getTriple().isRISCV() &&

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. I don't see changes here to make the Fuchsia & Android targets stop doing implicit `-ffixed-x18` and to make them start doing implicit `-msmall-data-limit=0` for non-PIC modes. Comment at: clang/lib/Driver/SanitizerArgs.cpp:546 - if ((Kinds & Sani

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

craig.topper accepted this revision. craig.topper added a comment. LGTM other than that one comment. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:86 InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) { - if (RISCV::isX18ReservedByDefault(TT)) -UserRes

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth added a comment. @asb, @craig.topper, @jrtc27 Are there any remaining considerations for us here? From the discussions in psABI and sig-toolchain, I think we have a consensus that this is the approach we'll be taking for RISC-V. We'd prefer to correct this ASAP, so as to prevent futu

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 509811. paulkirth added a comment. Remove blank lines added to test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D146463 Files: clang/lib/Driver/SanitizerArgs.cp

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. In D146463#4232252 , @craig.topper wrote: > In D146463#4232221 , @mcgrathr > wrote: > >> In D146463#4232214 , @craig.topper >> wrote: >> >>> D

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

craig.topper added a comment. In D146463#4232221 , @mcgrathr wrote: > In D146463#4232214 , @craig.topper > wrote: > >> Does the kernel populate the initial value of the register for the shadow >> stack or is tha

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. In D146463#4232214 , @craig.topper wrote: > Does the kernel populate the initial value of the register for the shadow > stack or is that done by a runtime inside the application? This is a change to code generation and is orth

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

craig.topper added a comment. Does the kernel populate the initial value of the register for the shadow stack or is that done by a runtime inside the application? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146463/new/ https://reviews.llvm.org/D

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

paulkirth updated this revision to Diff 509508. paulkirth retitled this revision from "[CodeGen][RISCV] Change Shadow Call Stack Register to S11" to "[CodeGen][RISCV] Change Shadow Call Stack Register to X3". paulkirth edited the summary of this revision. paulkirth added a comment. update to use