[ARM] harden-sls-blr: avoid r12 and lr in indirect calls
ClosedPublic
Actions

Authored by kristof.beyls on Dec 2 2020, 1:56 AM.

Details

Reviewers

ostannard

Commits

rGdf8ed3928377: [ARM] harden-sls-blr: avoid r12 and lr in indirect calls.

Summary

As a linker is allowed to clobber r12 on function calls, the
code transformation that hardens indirect calls is not correct in case a
linker does so. Similarly, the transformation is not correct when
register lr is used.

This patch makes sure that r12 or lr are not used for indirect calls
when harden-sls-blr is enabled.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

kristof.beyls created this revision.Dec 2 2020, 1:56 AM

Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2020, 1:56 AM

Herald added subscribers: llvm-commits, danielkiss, hiraditya. · View Herald Transcript

kristof.beyls requested review of this revision.Dec 2 2020, 1:56 AM

kristof.beyls added a parent revision: D92468: [ARM] Harden indirect calls against SLS.

Harbormaster completed remote builds in B80790: Diff 308915.Dec 2 2020, 1:57 AM

kristof.beyls added a child revision: D93221: [ARM] Add clang command line support for -mharden-sls=.Dec 14 2020, 8:18 AM

ostannard added inline comments.Dec 17 2020, 2:27 AM

llvm/lib/Target/ARM/ARMInstrInfo.td
2496	Wouldn't this also prevent use of the BLX instructions in inline assembly? I think it would be better to leave the requirements on the instructions as they are, and move the isel patterns out into separate `Pattern` records, which can have their own target requirements. I looks like that is already how the AArch64 backend does this.
llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
195	A slightly less fragile way to do this would be to pass the function pointer into and out an inline asm statement, with an output constraint which forces it into r12/lr. The compiler can still move it to a different register (and will need to do that with slsblr enabled), but it won't be dependent on register allocation order.

Updated patch based on review comments + rebased to ToT

kristof.beyls marked 2 inline comments as done.Dec 18 2020, 6:09 AM

kristof.beyls added inline comments.

llvm/lib/Target/ARM/ARMInstrInfo.td
2496	Agreed, I have separated out the patterns here and also in the Thumb instruction variants. I also added a test to the test file that checks that "blx r12" is accepted if used in inline assembly.
llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
195	Thanks for the suggestion, that is indeed better. Now implemented.