This is an archive of the discontinued LLVM Phabricator instance.

[x86/SLH] Teach SLH to harden indirect branches and switches without retpolines.
ClosedPublic

Authored by chandlerc on Aug 21 2018, 8:26 PM.

Details

Summary

This implements the core design of tracing the intended target into the
target, checking it, and using that to update the predicate state. It
takes advantage of a few interesting aspects of SLH to make it a bit
easier to implement:

  • We already split critical edges with conditional branches, so we can

assume those are gone.

  • We already unfolded any memory access in the indirect branch

instruction itself.

I've left hard errors in place to catch if any of these somewhat subtle
invariants get violated.

There is some code that I can factor out and share with D50837 when it
lands, but I didn't want to couple landing the two patches, so I'll do
that in a follow-up cleanup commit if alright.

Factoring out the code to handle different scenarios of materializing an
address remains frustratingly hard. In a bunch of cases you want to fold
one of the cases into an immediate operand of some other instruction,
and you also have both symbols and basic blocks being used which require
different methods on the MI builder (and different operand kinds).
Still, I'll take a stab at sharing at least some of this code in
a follow-up if I can figure out how.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc created this revision.Aug 21 2018, 8:26 PM
chandlerc updated this revision to Diff 161906.Aug 22 2018, 2:23 AM

Fix an obvious think-o (that I made twice) and would be caught if we actually
tried to encode the output here...

rnk added inline comments.Aug 23 2018, 3:44 PM
llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
945–947 ↗(On Diff #161906)

This sentence doesn't seem as precise as it could be. retpolines don't remove indirect branches, they replace them with indirect branches that always misspeculate. Maybe the right way to state this is that retpolines block all indirect branch speculation, whereas this mitigation allows for more correct indirect branch speculation.

965 ↗(On Diff #161906)

s/token/taken/

llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
2 ↗(On Diff #161906)

You might want to add a test for PIC label materialization. I don't see any new LEAs here.

chandlerc updated this revision to Diff 162320.Aug 23 2018, 8:00 PM
chandlerc marked 3 inline comments as done.

Address review comments.

Thanks for the review, should all be addressed.

llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
945–947 ↗(On Diff #161906)

Tried to improve comment. The tricky thing is that we do in practice just "eliminate" indirect branches by avoiding jump table lowerings.

Anyways, I' think I've gotten the comment more clear, but please let me know if there is a better way to explain all of this.

rnk accepted this revision.Aug 24 2018, 4:53 PM

lgtm

llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
353 ↗(On Diff #162320)

It would be nice if the test checked that it was using the correct label. I consider this {{.*}} scrub a bug in update_llc_test_checks.py. For every test I've added, I run the updater with --no_x86_scrip_rip or whatever the argument is.

This revision is now accepted and ready to land.Aug 24 2018, 4:53 PM
chandlerc marked an inline comment as done.Sep 4 2018, 3:41 AM

Thanks for the review, landing.

llvm/test/CodeGen/X86/speculative-load-hardening-indirect.ll
353 ↗(On Diff #162320)

Good suggestion, done.

This revision was automatically updated to reflect the committed changes.
chandlerc marked an inline comment as done.