Page MenuHomePhabricator

[codegen][riscv] Emit CFI directives when using shadow call stack

Authored by paulkirth on Mar 2 2023, 4:29 PM.



Currently we don't emit any CFI instructions for the SCS register when
enabling SCS on RISCV. This causes problems when unwinding, since the
SCS register isn't being handled properly.

Diff Detail

Event Timeline

paulkirth created this revision.Mar 2 2023, 4:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 4:29 PM
paulkirth requested review of this revision.Mar 2 2023, 4:29 PM
paulkirth edited the summary of this revision. (Show Details)Mar 2 2023, 4:39 PM
paulkirth added reviewers: jrtc27, asb.
mcgrathr accepted this revision.Mar 6 2023, 11:32 AM
mcgrathr added inline comments.

This is only safe to do with a one-byte buffer since you know the value fits.
It should at the very least have an assertion that encodeSLEB128 returned 1.
But just using -SlotSize & 0x7f would be more clearly safe.

This revision is now accepted and ready to land.Mar 6 2023, 11:32 AM
paulkirth updated this revision to Diff 502756.Mar 6 2023, 12:24 PM
paulkirth marked an inline comment as done.
paulkirth edited the summary of this revision. (Show Details)

Directly compute the SLEB128 value.

@jrtc27 @asb Any feedback on this? I'd like to land it, but wanted RISC-V maintainers to take a look. If there are no objections, I'll land this tomorrow.

jrtc27 added inline comments.Mar 8 2023, 2:30 PM

getDwarfRegNum with SCSPReg


Similarly don't hard-code (up to you whether you unconditionally use bregx or branch on whether reg num is < 32)


AArch64 does the cast before the mask, should be consistent


Also don't hard-code

mcgrathr added inline comments.Mar 8 2023, 6:21 PM

Definitely don't use bregx. That takes another byte and CFI size is precious. Just use DW_OP_breg0 + n after an assertion that n < 32 because it actually is (it's 18 and will only ever be 18).

paulkirth updated this revision to Diff 503817.Mar 9 2023, 9:56 AM
paulkirth marked 4 inline comments as done.

Address comments


I don't see a good way to not directly encode the length of the following expression without making this much more complex.