This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use OS-specific SafeStack ABI for Fuchsia
AcceptedPublic

Authored by mcgrathr on Feb 8 2023, 6:08 PM.

Details

Summary

Fuchsia provides a slot relative to tp for the unsafe stack pointer.

Diff Detail

Event Timeline

mcgrathr created this revision.Feb 8 2023, 6:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2023, 6:08 PM
mcgrathr requested review of this revision.Feb 8 2023, 6:08 PM
jrtc27 added inline comments.Feb 8 2023, 6:12 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
13272

Leave unrelated functions alone

llvm/test/Transforms/SafeStack/RISCV/abi.ll
4

This is way too complicated and confusing to read. Just use --check-prefix=FUCHSIA and use update_test_checks.py to generate the CHECK lines. And probably this should have a non-Fuschia test too.

jrtc27 added a comment.Feb 8 2023, 6:13 PM

And *please* add RISCV backend maintainers and regular reviewers/committers as reviewers on such patches

paulkirth accepted this revision.Feb 8 2023, 6:41 PM

Mostly this is LGTM modulo unrelated formatting. I'll let @jrtc27 decide if there's anything more required for RISCV, but the implementation and testing is consistent w/ ARM and AArch64.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
13272

nit: unrelated

13276

ditto.

This revision is now accepted and ready to land.Feb 8 2023, 6:41 PM
asb added a comment.Feb 10 2023, 7:46 AM

Thanks @mcgrathr - I've left an inline query.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
14249

I've not looked at the safe stack pointer code before, but I'd expect this to call TargetLowering::getSafeStackPointerLocation(IRB). Why doesn't it?