[exceptions] Upgrade exception handlers when stack protector is used

[etienneb]

Summary

MSVC provide exception handlers with enhanced information to deal with security buffer feature (/GS).

To be more secure, the security cookies (GS and SEH) are validated when unwinding the stack.

The following code:

void f() {}

void foo() {
  __try {
    f();
  } __except(1) {
    f();
  }
}

[etienneb]

This is a draft on the exception handlers upgrade.
For now, only 32-bits is implemented.

It's producing this code:

	.text
	.def	 @feat.00;
	.scl	3;
	.type	0;
	.endef
	.globl	@feat.00
@feat.00 = 1
	.def	 "?example@@YAHHH@Z";
	.scl	2;
	.type	32;
	.endef
	.globl	"?example@@YAHHH@Z"
	.p2align	4, 0x90
"?example@@YAHHH@Z":                    # @"\01?example@@YAHHH@Z"
Lfunc_begin0:
# BB#0:
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	pushl	%edi
	pushl	%esi
	subl	$48, %esp
	movl	$"L__ehtable$?example@@YAHHH@Z", %edx
	movl	%esp, -36(%ebp)
	movl	$-2, -16(%ebp)
	movl	12(%ebp), %ecx
	movl	8(%ebp), %eax
"L?example@@YAHHH@Z$frame_escape_0" = -44
	xorl	___security_cookie, %edx
	movl	%edx, -20(%ebp)
	movl	$__except_handler4, -24(%ebp)
	leal	-28(%ebp), %edx
	movl	%fs:0, %esi
	movl	%esi, -28(%ebp)
	movl	%edx, %fs:0
	movl	%ecx, -40(%ebp)
	movl	%eax, -48(%ebp)
	movl	$0, -16(%ebp)
	leal	-58(%ebp), %ecx
	movl	-40(%ebp), %eax
	pushl	%eax
	pushl	$204
	pushl	%ecx
	calll	"?memset@@YAPAXPAXHH@Z"
	addl	$12, %esp
LBB0_2:
	movl	-40(%ebp), %eax
	movl	-28(%ebp), %ecx
	movsbl	-58(%ebp,%eax), %eax
	movl	%ecx, %fs:0
	addl	$48, %esp
	popl	%esi
	popl	%edi
	popl	%ebx
	popl	%ebp
	retl
LBB0_1:
	movl	-24(%ebp), %esp
	addl	$12, %ebp
	jmp	LBB0_2

With MSVC compiler, every exception handlers in the same compilation unit are upgraded (with /GS).
With this approach, only functions with EH and GS are upgraded.

[rnk]

Test? You should be able to add an IR-only test that uses opt -winehprepare -stack-protector like test/CodeGen/WinEH/wineh-cloning.ll.

include/llvm/Analysis/EHPersonalities.h

74

Code in lib/Analysis isn't supposed to modify IR, but inserting a declaration into the module should be fine. Maybe a better name would be getStackGuardEHPersonality, though, since it doesn't actually modify the IR.

lib/Analysis/EHPersonalities.cpp

51

Since we will extend this soon, here's a cute thing you can do:

StringRef NewName = llvm::StringSwitch<StringRef>(F->getName())
              .Case("_except_handler3", "_except_handler4")
              .Case("__C_specific_handler", "__GSHandlerCheck_SEH")
              .Default("");
if (NewName.empty())
  return nullptr;
return M->getOrInsertFunction(NewName, ...);

[etienneb]

add test

[majnemer]

include/llvm/Analysis/EHPersonalities.h

72–74

I'd sink this into WinEHPrepare unless you expect to use it elsewhere.

lib/Analysis/EHPersonalities.cpp

44

Please change the return type to Value *.

lib/CodeGen/WinEHPrepare.cpp

681–683

Ditto.

683–684

You can Fold the assignment into the if.

testCodeGenWinEHwineh-promote-eh.ll

1

-mtriple=-mtriple= looks funky

[etienneb]

comments

[majnemer]

LGTM

lib/CodeGen/WinEHPrepare.cpp

107–112

I think it would make sense to sink this call into prepareExplicitEH.

[etienneb]

nits

[rnk]

lgtm

[thakis]

Did this ever land? If not, is it blocked on anything?

[etienneb]

In D21101#471334, @thakis wrote:

Did this ever land? If not, is it blocked on anything?

It's not blocked by anything.
It was the next thing to land on the Exception/GS issue.

We decided to land step by step the patches related to the feature to see when it will explode.

For now, this patch is a noop since we do not produce stack-guard + exception at the same time.

Should we land it now?

[rnk]

Let's go ahead and land this. Even though it's not functional, it documents our future plans in code.

[etienneb]

rebase (minor fixes)

[etienneb]

temporary reverted.

SEH4 needs to be fixed first.

[Diffusion] rL274251: revert http://reviews.llvm.org/D21101

include/llvm/Analysis/EHPersonalities.h

inline bool isFuncletEHPersonality(EHPersonality Pers) {
   case EHPersonality::CoreCLR:
     return true;
   default:
     return false;
   }
   llvm_unreachable("invalid enum");
 }
 
+/// \brief Returns a promoted personality function compatible with
+/// stack-protection.
+Value* getStackGuardEHPersonality(Value *Pers);
+
 /// \brief Return true if this personality may be safely removed if there
 /// are no invoke instructions remaining in the current function.
 inline bool isNoOpWithoutInvoke(EHPersonality Pers) {
   switch (Pers) {
   case EHPersonality::Unknown:
     return false;
   // All known personalities currently have this behavior
   default:

lib/Analysis/EHPersonalities.cpp

 //===- EHPersonalities.cpp - Compute EH-related information ---------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 /// See if the given exception handling personality function is one that we
 /// understand.  If so, return a description of it; otherwise return Unknown.
 EHPersonality llvm::classifyEHPersonality(const Value *Pers) {
   const Function *F =
return StringSwitch<EHPersonality>(F->getName())
     .Case("_except_handler4",      EHPersonality::MSVC_X86SEH)
     .Case("__C_specific_handler",  EHPersonality::MSVC_Win64SEH)
     .Case("__CxxFrameHandler3",    EHPersonality::MSVC_CXX)
     .Case("ProcessCLRException",   EHPersonality::CoreCLR)
     .Case("rust_eh_personality",   EHPersonality::Rust)
     .Default(EHPersonality::Unknown);
 }
 
+Value* llvm::getStackGuardEHPersonality(Value *Pers) {
+  Function *F =
+      Pers ? dyn_cast<Function>(Pers->stripPointerCasts()) : nullptr;
+  if (!F)
+    return nullptr;
+
+  // TODO(etienneb): Upgrade exception handlers when they are working.
+  StringRef NewName = llvm::StringSwitch<StringRef>(F->getName())
+      .Case("_except_handler3", "_except_handler4")
+      .Default("");
+  if (NewName.empty())
+    return nullptr;
+
+  Module* M = F->getParent();
+  return M->getOrInsertFunction("_except_handler4", F->getFunctionType(),
+                                F->getAttributes());
+}
+
 bool llvm::canSimplifyInvokeNoUnwind(const Function *F) {
   EHPersonality Personality = classifyEHPersonality(F->getPersonalityFn());
   // We can't simplify any invokes to nounwind functions if the personality
   // function wants to catch asynch exceptions.  The nounwind attribute only
   // implies that the function does not throw synchronous exceptions.
   return !isAsynchronousEHPersonality(Personality);
 }
 

lib/CodeGen/WinEHPrepare.cpp

public:
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   const char *getPassName() const override {
     return "Windows exception handling preparation";
   }
 
 private:
+  void promoteEHPersonality(Function &F);
   void insertPHIStores(PHINode *OriginalPHI, AllocaInst *SpillSlot);
   void
   insertPHIStore(BasicBlock *PredBlock, Value *PredVal, AllocaInst *SpillSlot,
                  SmallVectorImpl<std::pair<BasicBlock *, Value *>> &Worklist);
   AllocaInst *insertPHILoads(PHINode *PN, Function &F);
   void replaceUseWithLoad(Value *V, Use &U, AllocaInst *&SpillSlot,
                           DenseMap<BasicBlock *, Value *> &Loads, Function &F);
   bool prepareExplicitEH(Function &F);

 
 FunctionPass *llvm::createWinEHPass(const TargetMachine *TM) {
   return new WinEHPrepare(TM);
 }
 
 bool WinEHPrepare::runOnFunction(Function &Fn) {
   if (!Fn.hasPersonalityFn())
     return false;
 
+  // When stack-protector is present, some exception handlers need to be
+  // promoted to a compatible handlers.
+  promoteEHPersonality(Fn);
+
   // Classify the personality to see what kind of preparation we need.
   Personality = classifyEHPersonality(Fn.getPersonalityFn());
 
   // Do nothing if this is not a funclet-based personality.
   if (!isFuncletEHPersonality(Personality))
     return false;
 
   return prepareExplicitEH(Fn);
 }
for (auto Entry = FuncInfo.ClrEHUnwindMap.rbegin(),
 
     Entry->TryParentState = UnwindDestState;
   }
 
   // Step three: transfer information from pads to invokes.
   calculateStateNumbersForInvokes(Fn, FuncInfo);
 }
 
+void WinEHPrepare::promoteEHPersonality(Function &F) {
+  // Promote the exception handler when stack protection is activated.
+  if (!F.hasFnAttribute(Attribute::StackProtect) &&
+      !F.hasFnAttribute(Attribute::StackProtectReq) &&
+      !F.hasFnAttribute(Attribute::StackProtectStrong))
+    return;
+
+  Value* PersonalityFn = F.getPersonalityFn();
+  if (PersonalityFn) {
+    Value* Personality = getStackGuardEHPersonality(PersonalityFn);
+    if (Personality) {
+      Function* PromotedFn = cast<Function>(Personality);
+      F.setPersonalityFn(PromotedFn);
+    }
+  }
+}
+
 void WinEHPrepare::colorFunclets(Function &F) {
   BlockColors = colorEHFunclets(F);
 
   // Invert the map from BB to colors to color to BBs.
   for (BasicBlock &BB : F) {
     ColorVector &Colors = BlockColors[&BB];
     for (BasicBlock *Color : Colors)
       FuncletBlocks[Color].push_back(&BB);

testCodeGenWinEHwineh-promote-eh.ll

+; RUN: opt -mtriple=-mtriple=i686-windows-msvc -S -winehprepare %s | FileCheck %s
+
+declare i32 @_except_handler3(...)
+
+define void @test1a() personality i32 (...)* @_except_handler3 {
+; CHECK: define void @test1a() personality i32 (...)* @_except_handler3
+entry:
+  ret void
+}
+
+define void @test1b() ssp personality i32 (...)* @_except_handler3 {
+; CHECK: define void @test1b() [[attr:.*]] personality i32 (...)* @_except_handler4
+entry:
+  ret void
+}
+