[PATCH] D78471: [x86/SLH] Pin function address in physical register after it been hardened.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added subscribers: zbrid, mattdr.
chandlerc added a comment.

In D78471#2033046 <https://reviews.llvm.org/D78471#2033046>, @craig.topper wrote:

> In D78471#2033001 <https://reviews.llvm.org/D78471#2033001>, @pengfei wrote:
>
> > In D78471#2032415 <https://reviews.llvm.org/D78471#2032415>, @craig.topper wrote:
> >
> > > Is the problem here really that foldMemoryOperand doesn't know to preserve the post instr symbol?
> >
> >
> > Yes and no. It's the problem I wanted to fix firstly. But after I read the code of hardening, I think the spill is also a problem. Because that pass always load the function address to register at the beginning. I think the spilling violates the objective the pass does. Beside, according to calling conversation, there are volatile registers before calling. So I think it's not a cost if we pin the address register.
>
>
> I think we should be calling NewMI->cloneInstrSymbols  in the same spot we copy memory references in TargetInstrInfo::foldMemoryOperand. Or we shouldn't disable folding if any symbols are present. We're effectively cloning an instruction when we fold the memory and we should make sure we don't drop things when we do that.


+1 -- I'd definitely prefer to *enable* the spill, but correctly handle symbols attached.

> Whether SLH should spill or not is something we can still address, but that's a detail of what the right thing to do for SLH is. I view it separately from incorrectly cloning the instruction in folding.

Spills should be fine.

SLH very specifically runs *before* register allocation because hardening speculative *reloads* from spilled registers is not necessary in its security model. We don't harden other spills, and we don't need to avoid this one.

The reason the SLH pass forces the call target to be in a register (more specifically, a *virtual* register, because RA hasn't happened yet) is to ensure that the load and call are two separate instructions. When they're folded together, we don't have a chance to harden the target of the call *after* the load. All we need is to *unfold* this source load, we don't need to prevent further spills. It will spill the hardened value and reload it, which is within the security model of SLH.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78471/new/

https://reviews.llvm.org/D78471