Page MenuHomePhabricator

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

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

Details

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

Event Timeline

kristof.beyls created this revision.Dec 2 2020, 1:56 AM
kristof.beyls requested review of this revision.Dec 2 2020, 1:56 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.

This revision is now accepted and ready to land.Dec 18 2020, 6:20 AM
This revision was automatically updated to reflect the committed changes.
kristof.beyls marked 2 inline comments as done.