[X86] Fix liveness information when expanding X86::EH_SjLj_LongJmp64

[thegameg]

Summary

test/CodeGen/X86/shadow-stack.ll has the following machine verifier errors:

*** Bad machine code: Using a killed virtual register ***
- function:    bar
- basic block: %bb.6 entry (0x7fdc81857818)
- instruction: %3:gr64 = MOV64rm killed %2:gr64, 1, $noreg, 8, $noreg
- operand 1:   killed %2:gr64

*** Bad machine code: Using a killed virtual register ***
- function:    bar
- basic block: %bb.6 entry (0x7fdc81857818)
- instruction: $rsp = MOV64rm killed %2:gr64, 1, $noreg, 16, $noreg
- operand 1:   killed %2:gr64

*** Bad machine code: Virtual register killed in block, but needed live out. ***
- function:    bar
- basic block: %bb.2 entry (0x7fdc818574f8)
Virtual register %2 is used after the block.

The fix here is to only copy the machine operand's register without the
kill flags for all the instructions except the very last one of the
sequence.

I had to insert dummy PHIs in the test case to force the NoPHI function
property to be set to false. More on this here: https://llvm.org/PR38439

[thegameg]

Ping

[aivchenk]

Thanks for the fix. LGTM

lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::emitLongJmpShadowStackFix(MachineInstr &MI,
 
   // Reload the previously saved SSP register value.
   unsigned PrevSSPReg = MRI.createVirtualRegister(PtrRC);
   unsigned PtrLoadOpc = (PVT == MVT::i64) ? X86::MOV64rm : X86::MOV32rm;
   const int64_t SPPOffset = 3 * PVT.getStoreSize();
   MachineInstrBuilder MIB =
       BuildMI(fallMBB, DL, TII->get(PtrLoadOpc), PrevSSPReg);
   for (unsigned i = 0; i < X86::AddrNumOperands; ++i) {
+    const MachineOperand &MO = MI.getOperand(i);
     if (i == X86::AddrDisp)
-      MIB.addDisp(MI.getOperand(i), SPPOffset);
+      MIB.addDisp(MO, SPPOffset);
+    else if (MO.isReg()) // Don't add the whole operand, we don't want to
+                         // preserve kill flags.
+      MIB.addReg(MO.getReg());
     else
-      MIB.add(MI.getOperand(i));
+      MIB.add(MO);
   }
   MIB.setMemRefs(MMOBegin, MMOEnd);
 
   // Subtract the current SSP from the previous SSP.
   unsigned SspSubReg = MRI.createVirtualRegister(PtrRC);
   unsigned SubRROpc = (PVT == MVT::i64) ? X86::SUB64rr : X86::SUB32rr;
   BuildMI(fallMBB, DL, TII->get(SubRROpc), SspSubReg)
       .addReg(PrevSSPReg)
X86TargetLowering::emitEHSjLjLongJmp(MachineInstr &MI,
 
   // When CET and shadow stack is enabled, we need to fix the Shadow Stack.
   if (MF->getMMI().getModule()->getModuleFlag("cf-protection-return")) {
     thisMBB = emitLongJmpShadowStackFix(MI, thisMBB);
   }
 
   // Reload FP
   MIB = BuildMI(*thisMBB, MI, DL, TII->get(PtrLoadOpc), FP);
-  for (unsigned i = 0; i < X86::AddrNumOperands; ++i)
-    MIB.add(MI.getOperand(i));
+  for (unsigned i = 0; i < X86::AddrNumOperands; ++i) {
+    const MachineOperand &MO = MI.getOperand(i);
+    if (MO.isReg()) // Don't add the whole operand, we don't want to
+                    // preserve kill flags.
+      MIB.addReg(MO.getReg());
+    else
+      MIB.add(MO);
+  }
   MIB.setMemRefs(MMOBegin, MMOEnd);
 
   // Reload IP
   MIB = BuildMI(*thisMBB, MI, DL, TII->get(PtrLoadOpc), Tmp);
   for (unsigned i = 0; i < X86::AddrNumOperands; ++i) {
+    const MachineOperand &MO = MI.getOperand(i);
     if (i == X86::AddrDisp)
-      MIB.addDisp(MI.getOperand(i), LabelOffset);
+      MIB.addDisp(MO, LabelOffset);
+    else if (MO.isReg()) // Don't add the whole operand, we don't want to
+                         // preserve kill flags.
+      MIB.addReg(MO.getReg());
     else
-      MIB.add(MI.getOperand(i));
+      MIB.add(MO);
   }
   MIB.setMemRefs(MMOBegin, MMOEnd);
 
   // Reload SP
   MIB = BuildMI(*thisMBB, MI, DL, TII->get(PtrLoadOpc), SP);
   for (unsigned i = 0; i < X86::AddrNumOperands; ++i) {
     if (i == X86::AddrDisp)
       MIB.addDisp(MI.getOperand(i), SPOffset);
     else
-      MIB.add(MI.getOperand(i));
+      MIB.add(MI.getOperand(i)); // We can preserve the kill flags here, it's
+                                 // the last instruction of the expansion.
   }
   MIB.setMemRefs(MMOBegin, MMOEnd);
 
   // Jump
   BuildMI(*thisMBB, MI, DL, TII->get(IJmpOpc)).addReg(Tmp);
 
   MI.eraseFromParent();
   return thisMBB;

test/CodeGen/X86/sjlj-shadow-stack-liveness.mir

+# RUN: llc -mtriple=x86_64-- -run-pass=expand-isel-pseudos -verify-machineinstrs -o - %s | FileCheck %s
+# Check that we're not copying the kill flags with the operands from the pseudo
+# instruction.
+--- |
+  define void @bar() { ret void }
+
+  !llvm.module.flags = !{!0}
+
+  !0 = !{i32 4, !"cf-protection-return", i32 1}
+...
+---
+name:            bar
+# CHECK-LABEL: name: bar
+alignment:       4
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    %0:gr64 = IMPLICIT_DEF
+    ; CHECK: %0:gr64 = IMPLICIT_DEF
+    EH_SjLj_LongJmp64 killed %0, 1, $noreg, 0, $noreg
+    ; CHECK: bb.3:
+    ; CHECK: MOV64rm %0
+    ; CHECK-NOT: MOV64rm killed %0
+    ; CHECK: bb.7:
+    ; CHECK-NEXT: MOV64rm %0
+    ; CHECK-NOT: MOV64rm killed %0
+    ; CHECK-NEXT: MOV64rm %0
+    ; CHECK-NOT: MOV64rm killed %0
+    ; CHECK-NEXT: MOV64rm killed %0
+
+  ; FIXME: Dummy PHI to set the property NoPHIs to false. PR38439.
+  bb.2:
+    %1:gr64 = PHI undef %1, %bb.2, undef %1, %bb.2
+    JMP_1 %bb.2
+...