[StackProtector] Fix computation of GSCookieOffset and EHCookieOffset with SEH4

[etienneb]

Summary

Fix the computation of the offsets present in the scopetable when using the
SEH (__except_handler4).

This patch added an intrinsic to track the position of the allocation on the
stack of the EHGuard. This position is needed when producing the ScopeTable.

struct _EH4_SCOPETABLE {
    DWORD GSCookieOffset;
    DWORD GSCookieXOROffset;
    DWORD EHCookieOffset;
    DWORD EHCookieXOROffset;
    _EH4_SCOPETABLE_RECORD ScopeRecord[1];
};

struct _EH4_SCOPETABLE_RECORD {
    DWORD EnclosingLevel;
    long (*FilterFunc)();
        union {
        void (*HandlerAddress)();
        void (*FinallyFunc)();
    };
};

The code to generate the EHCookie is added in X86WinEHState.cpp.
Which is adding these instructions when using SEH4.

Lfunc_begin0:
# BB#0:                                 # %entry
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	pushl	%edi
	pushl	%esi
	subl	$28, %esp
	movl	%ebp, %eax                <<-- Loading FramePtr
	movl	%esp, -36(%ebp)
	movl	$-2, -16(%ebp)
	movl	$L__ehtable$use_except_handler4_ssp, %ecx
	xorl	___security_cookie, %ecx
	movl	%ecx, -20(%ebp)
	xorl	___security_cookie, %eax  <<-- XOR FramePtr and Cookie
	movl	%eax, -40(%ebp)           <<-- Storing EHGuard
	leal	-28(%ebp), %eax
	movl	$__except_handler4, -24(%ebp)
	movl	%fs:0, %ecx
	movl	%ecx, -28(%ebp)
	movl	%eax, %fs:0
	movl	$0, -16(%ebp)
	calll	_may_throw_or_crash
LBB1_1:                                 # %cont
	movl	-28(%ebp), %eax
	movl	%eax, %fs:0
	addl	$28, %esp
	popl	%esi
	popl	%edi
	popl	%ebx
	popl	%ebp
	retl

And the corresponding offset is computed:

Luse_except_handler4_ssp$parent_frame_offset = -36
	.p2align	2
L__ehtable$use_except_handler4_ssp:
	.long	-2                      # GSCookieOffset
	.long	0                       # GSCookieXOROffset
	.long	-40                     # EHCookieOffset    <<----
	.long	0                       # EHCookieXOROffset
	.long	-2                      # ToState
	.long	_catchall_filt          # FilterFunction
	.long	LBB1_2                  # ExceptionHandler

Clang is not yet producing function using SEH4, but it's a work in progress.
This patch is a step toward having a valid implementation of SEH4.
Unfortunately, it is not yet fully working. The EH registration block is not
allocated at the right offset on the stack.

[rnk]

The direction is consistent with what we've done before, but in retrospect, I kinda wish we did this in MI rather than IR. Changing that is out of scope though.

I'm fine with this if David majnemer is fine with this.

[majnemer]

lib/CodeGen/AsmPrinter/WinException.cpp

978

Is it possible for this value to actually make it into the object file?

lib/Target/X86/X86WinEHState.cpp

302–304

Is __CxxLongjmpUnwind still used if we are using C++ exceptions + /GS?

643

You could just do EHGuardNode->getNextNode()

[etienneb]

fix comments

lib/CodeGen/AsmPrinter/WinException.cpp

978

For now, I think it can be there.
But, when the feature will be implemented correctly, this should be turn as an "assert".

Would you prefer a TODO?
It's still on my plate to cleanup this.

lib/Target/X86/X86WinEHState.cpp

302–304

I didn't look to C++ exceptions yet, neither the longjmp/setjmp implementation.
I want to fix the stack-frame layout to make SEH4 working for the _try/_catch case.
Then, I'll look to other cases using SEH4.

[etienneb]

nits

[majnemer]

LGTM

lib/CodeGen/AsmPrinter/WinException.cpp

978

Yeah, a TODO would be good.

lib/Target/X86/X86WinEHState.cpp

302–304

Maybe I was thinking of _seh_longjmp_unwind4, It looks like I already added some code for it.

[rnk]

lgtm

[etienneb]

add todo

include/llvm/CodeGen/WinEHFuncInfo.h

struct WinEHFuncInfo {
 
   int getLastStateNumber() const { return CxxUnwindMap.size() - 1; }
 
   void addIPToStateRange(const InvokeInst *II, MCSymbol *InvokeBegin,
                          MCSymbol *InvokeEnd);
 
   int EHRegNodeFrameIndex = INT_MAX;
   int EHRegNodeEndOffset = INT_MAX;
+  int EHGuardFrameIndex = INT_MAX;
   int SEHSetFrameOffset = INT_MAX;
 
   WinEHFuncInfo();
 };
 
 /// Analyze the IR in ParentFn and it's handlers to build WinEHFuncInfo, which
 /// describes the state numbers and tables used by __CxxFrameHandler3. This
 /// analysis assumes that WinEHPrepare has already been run.

include/llvm/IR/IntrinsicsX86.td

 //===----------------------------------------------------------------------===//
 // SEH intrinsics for Windows
 let TargetPrefix = "x86" in {
   def int_x86_seh_lsda : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], [IntrNoMem]>;
 
   // Marks the EH registration node created in LLVM IR prior to code generation.
   def int_x86_seh_ehregnode : Intrinsic<[], [llvm_ptr_ty], []>;
 
+  // Marks the EH guard slot node created in LLVM IR prior to code generation.
+  def int_x86_seh_ehguard : Intrinsic<[], [llvm_ptr_ty], []>;
+
   // Given a pointer to the end of an EH registration object, returns the true
   // parent frame address that can be used with llvm.localrecover.
   def int_x86_seh_recoverfp : Intrinsic<[llvm_ptr_ty],
                                         [llvm_ptr_ty, llvm_ptr_ty],
                                         [IntrNoMem]>;
 }
 
 //===----------------------------------------------------------------------===//

lib/CodeGen/AsmPrinter/WinException.cpp

if (PerName == "_except_handler4") {
     // struct EH4ScopeTable {
     //   int32_t GSCookieOffset;
     //   int32_t GSCookieXOROffset;
     //   int32_t EHCookieOffset;
     //   int32_t EHCookieXOROffset;
     //   ScopeTableEntry ScopeRecord[];
     // };
     //
-    // Only the EHCookieOffset field appears to vary, and it appears to be the
-    // offset from the final saved SP value to the retaddr.
+    // Offsets are %ebp relative.
+    //
+    // The GS cookie is present only if the function needs stack protection.
+    // GSCookieOffset = -2 means that GS cookie is not used.
+    //
+    // The EH cookie is always present.
+    //
+    // Check is done the following way:
+    //    (ebp+CookieXOROffset) ^ [ebp+CookieOffset] == _security_cookie
+
+    // Retrieve the Guard Stack slot.
+    int GSCookieOffset = -2;
+    const MachineFrameInfo *MFI = MF->getFrameInfo();
+    if (MFI->hasStackProtectorIndex()) {
+      unsigned UnusedReg;
+      const TargetFrameLowering *TFI = MF->getSubtarget().getFrameLowering();
+      int SSPIdx = MFI->getStackProtectorIndex();
+      GSCookieOffset = TFI->getFrameIndexReference(*MF, SSPIdx, UnusedReg);
+    }
+
+    // Retrieve the EH Guard slot.
+    // TODO(etienneb): Get rid of this value and change it for and assertion.
+    int EHCookieOffset = 9999;
+    if (FuncInfo.EHGuardFrameIndex != INT_MAX) {
+      unsigned UnusedReg;
+      const TargetFrameLowering *TFI = MF->getSubtarget().getFrameLowering();
+      int EHGuardIdx = FuncInfo.EHGuardFrameIndex;
+      EHCookieOffset = TFI->getFrameIndexReference(*MF, EHGuardIdx, UnusedReg);
+    }
+
     AddComment("GSCookieOffset");
-    OS.EmitIntValue(-2, 4);
+    OS.EmitIntValue(GSCookieOffset, 4);
     AddComment("GSCookieXOROffset");
     OS.EmitIntValue(0, 4);
-    // FIXME: Calculate.
     AddComment("EHCookieOffset");
-    OS.EmitIntValue(9999, 4);
+    OS.EmitIntValue(EHCookieOffset, 4);
     AddComment("EHCookieXOROffset");
     OS.EmitIntValue(0, 4);
     BaseState = -2;
   }
 
   assert(!FuncInfo.SEHUnwindMap.empty());
   for (const SEHUnwindMapEntry &UME : FuncInfo.SEHUnwindMap) {
     auto *Handler = UME.Handler.get<MachineBasicBlock *>();

lib/Target/X86/X86ISelLowering.cpp

static SDValue MarkEHRegistrationNode(SDValue Op, SelectionDAG &DAG) {
   if (!FINode)
     report_fatal_error("llvm.x86.seh.ehregnode expects a static alloca");
   EHInfo->EHRegNodeFrameIndex = FINode->getIndex();
 
   // Return the chain operand without making any DAG nodes.
   return Chain;
 }
 
+static SDValue MarkEHGuard(SDValue Op, SelectionDAG &DAG) {
+  MachineFunction &MF = DAG.getMachineFunction();
+  SDValue Chain = Op.getOperand(0);
+  SDValue EHGuard = Op.getOperand(2);
+  WinEHFuncInfo *EHInfo = MF.getWinEHFuncInfo();
+  if (!EHInfo)
+    report_fatal_error("EHGuard only live in functions using WinEH");
+
+  // Cast the operand to an alloca, and remember the frame index.
+  auto *FINode = dyn_cast<FrameIndexSDNode>(EHGuard);
+  if (!FINode)
+    report_fatal_error("llvm.x86.seh.ehguard expects a static alloca");
+  EHInfo->EHGuardFrameIndex = FINode->getIndex();
+
+  // Return the chain operand without making any DAG nodes.
+  return Chain;
+}
+
 static SDValue LowerINTRINSIC_W_CHAIN(SDValue Op, const X86Subtarget &Subtarget,
                                       SelectionDAG &DAG) {
   unsigned IntNo = cast<ConstantSDNode>(Op.getOperand(1))->getZExtValue();
 
   const IntrinsicData* IntrData = getIntrinsicWithChain(IntNo);
   if (!IntrData) {
     if (IntNo == llvm::Intrinsic::x86_seh_ehregnode)
       return MarkEHRegistrationNode(Op, DAG);
+    if (IntNo == llvm::Intrinsic::x86_seh_ehguard)
+      return MarkEHGuard(Op, DAG);
     if (IntNo == llvm::Intrinsic::x86_flags_read_u32 ||
         IntNo == llvm::Intrinsic::x86_flags_read_u64 ||
         IntNo == llvm::Intrinsic::x86_flags_write_u32 ||
         IntNo == llvm::Intrinsic::x86_flags_write_u64) {
       // We need a frame pointer because this will get lowered to a PUSH/POP
       // sequence.
       MachineFrameInfo *MFI = DAG.getMachineFunction().getFrameInfo();
       MFI->setHasCopyImplyingStackAdjustment(true);

lib/Target/X86/X86WinEHState.cpp

private:
   int ParentBaseState;
   Constant *SehLongjmpUnwind = nullptr;
   Constant *Cookie = nullptr;
 
   /// The stack allocation containing all EH data, including the link in the
   /// fs:00 chain and the current state.
   AllocaInst *RegNode = nullptr;
 
+  // The allocation containing the EH security guard.
+  AllocaInst *EHGuardNode = nullptr;
+
   /// The index of the state field of RegNode.
   int StateFieldIndex = ~0U;
 
   /// The linked list node subobject inside of RegNode.
   Value *Link = nullptr;
 };
 }
 
bool WinEHStatePass::runOnFunction(Function &F) {
   // numbers into an immutable analysis pass.
   WinEHFuncInfo FuncInfo;
   addStateStores(F, FuncInfo);
 
   // Reset per-function state.
   PersonalityFn = nullptr;
   Personality = EHPersonality::Unknown;
   UseStackGuard = false;
+  RegNode = nullptr;
+  EHGuardNode = nullptr;
+
   return true;
 }
 
 /// Get the common EH registration subobject:
 ///   typedef _EXCEPTION_DISPOSITION (*PEXCEPTION_ROUTINE)(
 ///       _EXCEPTION_RECORD *, void *, _CONTEXT *, void *);
 ///   struct EHRegistrationNode {
 ///     EHRegistrationNode *Next;
void WinEHStatePass::emitExceptionRegistrationRecord(Function *F) {
   assert(Personality == EHPersonality::MSVC_CXX ||
          Personality == EHPersonality::MSVC_X86SEH);
 
   // Struct type of RegNode. Used for GEPing.
   Type *RegNodeTy;
 
   IRBuilder<> Builder(&F->getEntryBlock(), F->getEntryBlock().begin());
   Type *Int8PtrType = Builder.getInt8PtrTy();
+  Type *Int32Ty = Builder.getInt32Ty();
+  Type *VoidTy = Builder.getVoidTy();
+
   if (Personality == EHPersonality::MSVC_CXX) {
     RegNodeTy = getCXXEHRegistrationType();
     RegNode = Builder.CreateAlloca(RegNodeTy);
     // SavedESP = llvm.stacksave()
     Value *SP = Builder.CreateCall(
         Intrinsic::getDeclaration(TheModule, Intrinsic::stacksave), {});
     Builder.CreateStore(SP, Builder.CreateStructGEP(RegNodeTy, RegNode, 0));
     // TryLevel = -1
     StateFieldIndex = 2;
     ParentBaseState = -1;
     insertStateNumberStore(&*Builder.GetInsertPoint(), ParentBaseState);
     // Handler = __ehhandler$F
     Function *Trampoline = generateLSDAInEAXThunk(F);
     Link = Builder.CreateStructGEP(RegNodeTy, RegNode, 1);
     linkExceptionRegistration(Builder, Trampoline);
 
     CxxLongjmpUnwind = TheModule->getOrInsertFunction(
         "__CxxLongjmpUnwind",
-        FunctionType::get(Type::getVoidTy(TheModule->getContext()), Int8PtrType,
-                          /*isVarArg=*/false));
+        FunctionType::get(VoidTy, Int8PtrType, /*isVarArg=*/false));
     cast<Function>(CxxLongjmpUnwind->stripPointerCasts())
         ->setCallingConv(CallingConv::X86_StdCall);
   } else if (Personality == EHPersonality::MSVC_X86SEH) {
     // If _except_handler4 is in use, some additional guard checks and prologue
     // stuff is required.
+    StringRef PersonalityName = PersonalityFn->getName();
+    UseStackGuard = (PersonalityName == "_except_handler4");
+
+    // Allocate local structures.
     RegNodeTy = getSEHRegistrationType();
     RegNode = Builder.CreateAlloca(RegNodeTy);
+    if (UseStackGuard)
+      EHGuardNode = Builder.CreateAlloca(Int32Ty);
+
     // SavedESP = llvm.stacksave()
     Value *SP = Builder.CreateCall(
         Intrinsic::getDeclaration(TheModule, Intrinsic::stacksave), {});
     Builder.CreateStore(SP, Builder.CreateStructGEP(RegNodeTy, RegNode, 0));
     // TryLevel = -2 / -1
     StateFieldIndex = 4;
-    StringRef PersonalityName = PersonalityFn->getName();
-    UseStackGuard = (PersonalityName == "_except_handler4");
     ParentBaseState = UseStackGuard ? -2 : -1;
     insertStateNumberStore(&*Builder.GetInsertPoint(), ParentBaseState);
     // ScopeTable = llvm.x86.seh.lsda(F)
     Value *LSDA = emitEHLSDA(Builder, F);
-    Type *Int32Ty = Type::getInt32Ty(TheModule->getContext());
     LSDA = Builder.CreatePtrToInt(LSDA, Int32Ty);
     // If using _except_handler4, xor the address of the table with
     // __security_cookie.
     if (UseStackGuard) {
       Cookie = TheModule->getOrInsertGlobal("__security_cookie", Int32Ty);
-      Value *Val = Builder.CreateLoad(Int32Ty, Cookie);
+      Value *Val = Builder.CreateLoad(Int32Ty, Cookie, "cookie");
       LSDA = Builder.CreateXor(LSDA, Val);
     }
     Builder.CreateStore(LSDA, Builder.CreateStructGEP(RegNodeTy, RegNode, 3));
+
+    // If using _except_handler4, the EHGuard contains: FramePtr xor Cookie.
+    if (UseStackGuard) {
+      Value *Val = Builder.CreateLoad(Int32Ty, Cookie);
+      Value *FrameAddr = Builder.CreateCall(
+          Intrinsic::getDeclaration(TheModule, Intrinsic::frameaddress),
+          Builder.getInt32(0), "frameaddr");
+      Value *FrameAddrI32 = Builder.CreatePtrToInt(FrameAddr, Int32Ty);
+      FrameAddrI32 = Builder.CreateXor(FrameAddrI32, Val);
+      Builder.CreateStore(FrameAddrI32, EHGuardNode);
+    }
+
+    // Register the exception handler.
     Link = Builder.CreateStructGEP(RegNodeTy, RegNode, 2);
     linkExceptionRegistration(Builder, PersonalityFn);
 
     SehLongjmpUnwind = TheModule->getOrInsertFunction(
         UseStackGuard ? "_seh_longjmp_unwind4" : "_seh_longjmp_unwind",
         FunctionType::get(Type::getVoidTy(TheModule->getContext()), Int8PtrType,
                           /*isVarArg=*/false));
     cast<Function>(SehLongjmpUnwind->stripPointerCasts())
bool WinEHStatePass::isStateStoreNeeded(EHPersonality Personality,
 
   // If the function throws, it needs a state store.
   return !CS.doesNotThrow();
 }
 
 void WinEHStatePass::addStateStores(Function &F, WinEHFuncInfo &FuncInfo) {
   // Mark the registration node. The backend needs to know which alloca it is so
   // that it can recover the original frame pointer.
-  IRBuilder<> Builder(RegNode->getParent(), std::next(RegNode->getIterator()));
+  IRBuilder<> Builder(RegNode->getNextNode());
   Value *RegNodeI8 = Builder.CreateBitCast(RegNode, Builder.getInt8PtrTy());
   Builder.CreateCall(
       Intrinsic::getDeclaration(TheModule, Intrinsic::x86_seh_ehregnode),
       {RegNodeI8});
 
+  if (EHGuardNode) {
+    IRBuilder<> Builder(EHGuardNode->getNextNode());
+    Value *EHGuardNodeI8 =
+        Builder.CreateBitCast(EHGuardNode, Builder.getInt8PtrTy());
+    Builder.CreateCall(
+        Intrinsic::getDeclaration(TheModule, Intrinsic::x86_seh_ehguard),
+        {EHGuardNodeI8});
+  }
+
   // Calculate state numbers.
   if (isAsynchronousEHPersonality(Personality))
     calculateSEHStateNumbers(&F, FuncInfo);
   else
     calculateWinCXXEHStateNumbers(&F, FuncInfo);
 
   // Iterate all the instructions and emit state number stores.
   DenseMap<BasicBlock *, ColorVector> BlockColors = colorEHFunclets(F);

test/CodeGen/X86/win32-eh.ll

 ; CHECK: movl %[[next]], %fs:0
 ; CHECK: retl
 ; CHECK: LBB2_2: # %catch{{$}}
 
 ; CHECK: .section .xdata,"dr"
 ; CHECK-LABEL: L__ehtable$use_except_handler4:
 ; CHECK-NEXT:  .long   -2
 ; CHECK-NEXT:  .long   0
-; CHECK-NEXT:  .long   9999
+; CHECK-NEXT:  .long   -40
 ; CHECK-NEXT:  .long   0
 ; CHECK-NEXT:  .long   -2
 ; CHECK-NEXT:  .long   _catchall_filt
 ; CHECK-NEXT:  .long   LBB2_2
 
+define void @use_except_handler4_ssp() sspstrong personality i32 (...)* @_except_handler4 {
+entry:
+  invoke void @may_throw_or_crash()
+      to label %cont unwind label %lpad
+cont:
+  ret void
+lpad:
+  %cs = catchswitch within none [label %catch] unwind to caller
+catch:
+  %p = catchpad within %cs [i8* bitcast (i32 ()* @catchall_filt to i8*)]
+  catchret from %p to label %cont
+}
+
+; CHECK-LABEL: _use_except_handler4_ssp:
+; CHECK: pushl %ebp
+; CHECK: movl %esp, %ebp
+; CHECK: subl ${{[0-9]+}}, %esp
+; CHECK: movl %ebp, %[[ehguard:[^ ,]*]]
+; CHECK: movl %esp, -36(%ebp)
+; CHECK: movl $-2, -16(%ebp)
+; CHECK: movl $L__ehtable$use_except_handler4_ssp, %[[lsda:[^ ,]*]]
+; CHECK: xorl ___security_cookie, %[[lsda]]
+; CHECK: movl %[[lsda]], -20(%ebp)
+; CHECK: xorl ___security_cookie, %[[ehguard]]
+; CHECK: movl %[[ehguard]], -40(%ebp)
+; CHECK: leal -28(%ebp), %[[node:[^ ,]*]]
+; CHECK: movl $__except_handler4, -24(%ebp)
+; CHECK: movl %fs:0, %[[next:[^ ,]*]]
+; CHECK: movl %[[next]], -28(%ebp)
+; CHECK: movl %[[node]], %fs:0
+; CHECK: calll _may_throw_or_crash
+; CHECK: movl -28(%ebp), %[[next:[^ ,]*]]
+; CHECK: movl %[[next]], %fs:0   
+; CHECK: retl
+; CHECK: [[catch:[^ ,]*]]: # %catch{{$}}
+
+; CHECK: .section .xdata,"dr"
+; CHECK-LABEL: L__ehtable$use_except_handler4_ssp:
+; CHECK-NEXT:  .long   -2
+; CHECK-NEXT:  .long   0
+; CHECK-NEXT:  .long   -40  
+; CHECK-NEXT:  .long   0
+; CHECK-NEXT:  .long   -2
+; CHECK-NEXT:  .long   _catchall_filt
+; CHECK-NEXT:  .long   [[catch]]
+
 define void @use_CxxFrameHandler3() personality i32 (...)* @__CxxFrameHandler3 {
   invoke void @may_throw_or_crash()
       to label %cont unwind label %catchall
 cont:
   ret void
 
 catchall:
   %cs = catchswitch within none [label %catch] unwind to caller