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

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 Differential Revision: https://reviews.llvm.org/D50260 llvm-svn: 340033
llvm · Aug 17, 2018 · 8bff832 · 8bff832
1 parent 9e50e91
commit 8bff832
Showing 2 changed files with 57 additions and 7 deletions.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -28262,10 +28262,14 @@ X86TargetLowering::emitLongJmpShadowStackFix(MachineInstr &MI,
   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(MMOs);
 
@@ -28383,17 +28387,27 @@ X86TargetLowering::emitEHSjLjLongJmp(MachineInstr &MI,
 
   // 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(MMOs);
 
   // 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(MMOs);
 
@@ -28403,7 +28417,8 @@ X86TargetLowering::emitEHSjLjLongJmp(MachineInstr &MI,
     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(MMOs);
 

diff --git a/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir b/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir
@@ -0,0 +1,35 @@
+# 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
+...