This is an archive of the discontinued LLVM Phabricator instance.

[X86] Use EDI for retpoline when no scratch regs are left
ClosedPublic

Authored by rnk on Feb 12 2018, 4:08 PM.

Details

Summary

Instead of solving the hard problem of how to pass the callee to the indirect
jump thunk without a register, just use a CSR. At a call boundary, there's
nothing stopping us from using a CSR to hold the callee as long as we save and
restore it in the prologue.

Also, add tests for this mregparm=3 case. I wrote execution tests for
__llvm_retpoline_push, but they never got committed as lit tests, either
because I never rewrote them or because they got lost in merge conflicts.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk created this revision.Feb 12 2018, 4:08 PM
chandlerc accepted this revision.Feb 13 2018, 1:33 AM

So, if I understand correctly, this prevents the use of -mregparm=3, 32-bit x86, and a caller-save-all calling convention. And the reason this is reliable is because LLVM doesn't have such a calling convention (except maybe AnyReg) and if someone even managed to craft such IR they would just hit the assert and have to extend this? And we believe Clang definitively cannot create such a calling convention?

I'm not doubting anything here, I'm repeating to check my understanding. =]

Provided I've understood correctly, LGTM, let's get this in testing.

This revision is now accepted and ready to land.Feb 13 2018, 1:33 AM
rnk added a comment.Feb 13 2018, 12:48 PM

So, if I understand correctly, this prevents the use of -mregparm=3, 32-bit x86, and a caller-save-all calling convention. And the reason this is reliable is because LLVM doesn't have such a calling convention (except maybe AnyReg) and if someone even managed to craft such IR they would just hit the assert and have to extend this? And we believe Clang definitively cannot create such a calling convention?

I'm not doubting anything here, I'm repeating to check my understanding. =]

Yes, there may be 32-bit CCs that can use all of EAX, EDX, ECX, and EDI, but I don't think it's worth trying to support retpolining indirect calls of such conventions. The same problem may exist on x64 if someone adds a convention that uses R11. We're just assuming they wouldn't because such a convention won't work when there's a PLT trip involved. I strengthened the assert to a conditional report_fatal_error, so we will fail reliably in these unsupported scenarios.

Provided I've understood correctly, LGTM, let's get this in testing.

Sounds good. I will try to merge this to 6.0 and 5.0.

This revision was automatically updated to reflect the committed changes.