Merge pull request #2704 from TNorthover/tailcc-ssp

StackProtector: ensure protection does not interfere with tail call frame

Author: varungandhi-apple

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

@@ -1691,16 +1691,35 @@ static bool MIIsInTerminatorSequence(const MachineInstr &MI) {
 /// terminator, but additionally the copies that move the vregs into the
 /// physical registers.
 static MachineBasicBlock::iterator
-FindSplitPointForStackProtector(MachineBasicBlock *BB) {
+FindSplitPointForStackProtector(MachineBasicBlock *BB,
+                                const TargetInstrInfo &TII) {
   MachineBasicBlock::iterator SplitPoint = BB->getFirstTerminator();
-  //
   if (SplitPoint == BB->begin())
     return SplitPoint;
 
   MachineBasicBlock::iterator Start = BB->begin();
   MachineBasicBlock::iterator Previous = SplitPoint;
   --Previous;
 
+  if (TII.isTailCall(*SplitPoint) &&
+      Previous->getOpcode() == TII.getCallFrameDestroyOpcode()) {
+    // Call frames cannot be nested, so if this frame is describing the tail
+    // call itself then we must insert before the sequence even starts. On the
+    // other hand, it could be an unrelated call in which case this tail call
+    // has to register moves of its own and should be the split point.
+    bool DifferentCall = false;
+    MachineBasicBlock::iterator I = Previous;
+    do {
+      --I;
+      if (I->isCall()) {
+        DifferentCall = true;
+        break;
+      }
+    } while(I->getOpcode() != TII.getCallFrameSetupOpcode());
+
+    return DifferentCall ? SplitPoint : I;
+  }
+
   while (MIIsInTerminatorSequence(*Previous)) {
     SplitPoint = Previous;
     if (Previous == Start)
@@ -1740,7 +1759,7 @@ SelectionDAGISel::FinishBasicBlock() {
     // Add load and check to the basicblock.
     FuncInfo->MBB = ParentMBB;
     FuncInfo->InsertPt =
-        FindSplitPointForStackProtector(ParentMBB);
+        FindSplitPointForStackProtector(ParentMBB, *TII);
     SDB->visitSPDescriptorParent(SDB->SPDescriptor, ParentMBB);
     CurDAG->setRoot(SDB->getRoot());
     SDB->clear();
@@ -1759,7 +1778,7 @@ SelectionDAGISel::FinishBasicBlock() {
     // register allocation issues caused by us splitting the parent mbb. The
     // register allocator will clean up said virtual copies later on.
     MachineBasicBlock::iterator SplitPoint =
-        FindSplitPointForStackProtector(ParentMBB);
+        FindSplitPointForStackProtector(ParentMBB, *TII);
 
     // Splice the terminator of ParentMBB into SuccessMBB.
     SuccessMBB->splice(SuccessMBB->end(), ParentMBB,

llvm/test/CodeGen/X86/tailcc-ssp.ll

@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=x86_64-windows-msvc %s -o - -verify-machineinstrs | FileCheck %s
+
+declare void @h(i8*, i64, i8*)
+
+define tailcc void @tailcall_frame(i8* %0, i64 %1) sspreq {
+; CHECK-LABEL: tailcall_frame:
+; CHECK: callq __security_check_cookie
+; CHECK: xorl %ecx, %ecx
+; CHECK: jmp h
+
+   tail call tailcc void @h(i8* null, i64 0, i8* null)
+   ret void
+}
+
+declare void @bar()
+define void @tailcall_unrelated_frame() sspreq {
+; CHECK-LABEL: tailcall_unrelated_frame:
+; CHECK: subq [[STACK:\$.*]], %rsp
+; CHECK: callq bar
+; CHECK: callq __security_check_cookie
+; CHECK: addq [[STACK]], %rsp
+; CHECK: jmp bar
+  call void @bar()
+  tail call void @bar()
+  ret void
+}