diff --git a/lib/CodeGen/WinEHPrepare.cpp b/lib/CodeGen/WinEHPrepare.cpp index 281fe934292..c4eaef88673 100644 --- a/lib/CodeGen/WinEHPrepare.cpp +++ b/lib/CodeGen/WinEHPrepare.cpp @@ -81,6 +81,7 @@ private: void cloneCommonBlocks(Function &F); void removeImplausibleInstructions(Function &F); void cleanupPreparedFunclets(Function &F); + void fixupNoReturnCleanupPads(Function &F); void verifyPreparedFunclets(Function &F); // All fields are reset by runOnFunction. @@ -656,6 +657,7 @@ void llvm::calculateClrEHStateNumbers(const Function *Fn, void WinEHPrepare::colorFunclets(Function &F) { BlockColors = colorEHFunclets(F); + FuncletBlocks.clear(); // Invert the map from BB to colors to color to BBs. for (BasicBlock &BB : F) { @@ -1001,6 +1003,114 @@ void WinEHPrepare::cleanupPreparedFunclets(Function &F) { removeUnreachableBlocks(F); } +// Cleanuppads which are postdominated by unreachable will not unwind to any +// catchswitches, making them dead. This is problematic if the original source +// had a catch clause which could legitimately catch the exception, causing the +// cleanup to run. +// +// This is only a problem for C++ where down-stream catches cause cleanups to +// run. +void WinEHPrepare::fixupNoReturnCleanupPads(Function &F) { + // We only need to do this for C++ personality routines, + // skip this work for all others. + if (Personality != EHPersonality::MSVC_CXX) + return; + + // Do a quick sanity check on the color map before we throw it away so as to + // avoid hiding latent bugs. + DEBUG(verifyPreparedFunclets(F)); + // Re-color the funclets because cleanupPreparedFunclets might have + // invalidated FuncletBlocks. + colorFunclets(F); + + // We create a unique catchswitch for each parent it will be nested within. + SmallDenseMap NewCatchSwitches; + // Create a new catchswitch+catchpad where the catchpad is post-dominated by + // unreachable. + auto GetOrCreateCatchSwitch = [&](Value *ParentPad) { + auto &CatchSwitch = NewCatchSwitches[ParentPad]; + if (CatchSwitch) + return CatchSwitch; + + auto *ParentBB = isa(ParentPad) + ? &F.getEntryBlock() + : cast(ParentPad)->getParent(); + + StringRef NameSuffix = ParentBB->getName(); + + BasicBlock *CatchSwitchBB = BasicBlock::Create( + F.getContext(), Twine("catchswitchbb.for.", NameSuffix)); + CatchSwitchBB->insertInto(&F, ParentBB->getNextNode()); + CatchSwitch = CatchSwitchInst::Create(ParentPad, /*UnwindDest=*/nullptr, + /*NumHandlers=*/1, + Twine("catchswitch.for.", NameSuffix), + CatchSwitchBB); + + BasicBlock *CatchPadBB = BasicBlock::Create( + F.getContext(), Twine("catchpadbb.for.", NameSuffix)); + CatchPadBB->insertInto(&F, CatchSwitchBB->getNextNode()); + Value *CatchPadArgs[] = { + Constant::getNullValue(Type::getInt8PtrTy(F.getContext())), + ConstantInt::get(Type::getInt32Ty(F.getContext()), 64), + Constant::getNullValue(Type::getInt8PtrTy(F.getContext())), + }; + CatchPadInst::Create(CatchSwitch, CatchPadArgs, + Twine("catchpad.for.", NameSuffix), CatchPadBB); + new UnreachableInst(F.getContext(), CatchPadBB); + + CatchSwitch->addHandler(CatchPadBB); + + return CatchSwitch; + }; + + // Look for all basic blocks which are within cleanups which are postdominated + // by unreachable. + for (auto &Funclets : FuncletBlocks) { + BasicBlock *FuncletPadBB = Funclets.first; + auto *CleanupPad = dyn_cast(FuncletPadBB->getFirstNonPHI()); + // Skip over any non-cleanup funclets. + if (!CleanupPad) + continue; + // Skip over any cleanups have unwind targets, they do not need this. + if (getCleanupRetUnwindDest(CleanupPad) != nullptr) + continue; + // Walk the blocks within the cleanup which end in 'unreachable'. + // We will replace the unreachable instruction with a cleanupret; + // this cleanupret will unwind to a catchswitch with a lone catch-all + // catchpad. + std::vector &BlocksInFunclet = Funclets.second; + for (BasicBlock *BB : BlocksInFunclet) { + auto *UI = dyn_cast(BB->getTerminator()); + if (!UI) + continue; + // Remove the unreachable instruction. + UI->eraseFromParent(); + + // Add our new cleanupret. + auto *ParentPad = CleanupPad->getParentPad(); + CatchSwitchInst *CatchSwitch = GetOrCreateCatchSwitch(ParentPad); + CleanupReturnInst::Create(CleanupPad, CatchSwitch->getParent(), BB); + } + } + + // Update BlockColors and FuncletBlocks to maintain WinEHPrepare's + // invariants. + for (auto CatchSwitchKV : NewCatchSwitches) { + CatchSwitchInst *CatchSwitch = CatchSwitchKV.second; + BasicBlock *CatchSwitchBB = CatchSwitch->getParent(); + + assert(CatchSwitch->getNumSuccessors() == 1); + BasicBlock *CatchPadBB = CatchSwitch->getSuccessor(0); + assert(isa(CatchPadBB->getFirstNonPHI())); + + BlockColors.insert({CatchSwitchBB, ColorVector(CatchSwitchBB)}); + FuncletBlocks[CatchSwitchBB] = {CatchSwitchBB}; + + BlockColors.insert({CatchPadBB, ColorVector(CatchPadBB)}); + FuncletBlocks[CatchPadBB] = {CatchPadBB}; + } +} + void WinEHPrepare::verifyPreparedFunclets(Function &F) { for (BasicBlock &BB : F) { size_t NumColors = BlockColors[&BB].size(); @@ -1036,6 +1146,8 @@ bool WinEHPrepare::prepareExplicitEH(Function &F) { cleanupPreparedFunclets(F); } + fixupNoReturnCleanupPads(F); + DEBUG(verifyPreparedFunclets(F)); // Recolor the CFG to verify that all is well. DEBUG(colorFunclets(F)); diff --git a/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll b/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll new file mode 100644 index 00000000000..09b4d3b8a1b --- /dev/null +++ b/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll @@ -0,0 +1,71 @@ +; RUN: opt -S -winehprepare < %s | FileCheck %s +target triple = "x86_64-pc-windows-msvc" + +; CHECK-LABEL: @test1( +define void @test1(i1 %b) personality i32 (...)* @__CxxFrameHandler3 { +entry: + invoke void @f() + to label %try.cont unwind label %cleanup.bb + +; CHECK: entry: + +; CHECK: [[catchswitch_entry:.*]]: +; CHECK-NEXT: %[[cs0:.*]] = catchswitch within none [label %[[catchpad:.*]]] unwind to caller +; CHECK: [[catchpad]]: +; CHECK-NEXT: %[[cp0:.*]] = catchpad within %[[cs0]] [i8* null, i32 64, i8* null] +; CHECK-NEXT: unreachable + +try.cont: + invoke void @f() + to label %exit unwind label %catchswitch.bb + +cleanup.bb: + %cleanup = cleanuppad within none [] + br i1 %b, label %left, label %right + +left: + call void @exit(i32 0) [ "funclet"(token %cleanup) ] + unreachable + +right: + call void @exit(i32 1) [ "funclet"(token %cleanup) ] + unreachable + +catchswitch.bb: + %cs = catchswitch within none [label %catchpad.bb] unwind to caller + +; CHECK: catchpad.bb: +; CHECK-NEXT: %catch = catchpad within %cs [i8* null, i32 64, i8* null] + +; CHECK: [[catchswitch_catch:.*]]: +; CHECK-NEXT: %[[cs1:.*]] = catchswitch within %catch [label %[[catchpad_catch:.*]]] unwind to caller +; CHECK: [[catchpad_catch]]: +; CHECK-NEXT: %[[cp1:.*]] = catchpad within %[[cs1]] [i8* null, i32 64, i8* null] +; CHECK-NEXT: unreachable + +; CHECK: nested.cleanup.bb: +; CHECK-NEXT: %nested.cleanup = cleanuppad within %catch [] +; CHECK-NEXT: call void @exit(i32 2) [ "funclet"(token %nested.cleanup) ] +; CHECK-NEXT: cleanupret from %nested.cleanup unwind label %[[catchswitch_catch]] + +catchpad.bb: + %catch = catchpad within %cs [i8* null, i32 64, i8* null] + invoke void @f() [ "funclet"(token %catch) ] + to label %unreachable unwind label %nested.cleanup.bb + +nested.cleanup.bb: + %nested.cleanup = cleanuppad within %catch [] + call void @exit(i32 2) [ "funclet"(token %nested.cleanup) ] + unreachable + +unreachable: + unreachable + +exit: + unreachable +} + +declare void @f() +declare void @exit(i32) nounwind noreturn + +declare i32 @__CxxFrameHandler3(...) diff --git a/test/CodeGen/WinEH/wineh-cloning.ll b/test/CodeGen/WinEH/wineh-cloning.ll index 748c07df173..355ffa8c3f6 100644 --- a/test/CodeGen/WinEH/wineh-cloning.ll +++ b/test/CodeGen/WinEH/wineh-cloning.ll @@ -44,7 +44,7 @@ noreturn: ; CHECK: call void @llvm.foo(i32 %x) -define void @test2() personality i32 (...)* @__CxxFrameHandler3 { +define void @test2() personality i32 (...)* @__C_specific_handler { entry: invoke void @f() to label %exit unwind label %cleanup @@ -71,7 +71,7 @@ exit: ; CHECK-NEXT: ret void -define void @test3() personality i32 (...)* @__CxxFrameHandler3 { +define void @test3() personality i32 (...)* @__C_specific_handler { entry: invoke void @f() to label %invoke.cont unwind label %catch.switch diff --git a/test/CodeGen/WinEH/wineh-demotion.ll b/test/CodeGen/WinEH/wineh-demotion.ll index 411952d84bb..048466a26d1 100644 --- a/test/CodeGen/WinEH/wineh-demotion.ll +++ b/test/CodeGen/WinEH/wineh-demotion.ll @@ -1,6 +1,7 @@ ; RUN: opt -mtriple=x86_64-pc-windows-msvc -S -winehprepare < %s | FileCheck %s declare i32 @__CxxFrameHandler3(...) +declare i32 @__C_specific_handler(...) declare void @f() @@ -327,7 +328,7 @@ exit: } ; CHECK-LABEL: @test8( -define void @test8() personality i32 (...)* @__CxxFrameHandler3 { entry: +define void @test8() personality i32 (...)* @__C_specific_handler { entry: invoke void @f() to label %done unwind label %cleanup1 invoke void @f() diff --git a/test/CodeGen/WinEH/wineh-no-demotion.ll b/test/CodeGen/WinEH/wineh-no-demotion.ll index 0901e27c301..277dd5c0e6a 100644 --- a/test/CodeGen/WinEH/wineh-no-demotion.ll +++ b/test/CodeGen/WinEH/wineh-no-demotion.ll @@ -1,4 +1,4 @@ -; RUN: opt -mtriple=x86_x64-pc-windows-msvc -S -winehprepare -disable-demotion -disable-cleanups < %s | FileCheck %s +; RUN: opt -mtriple=x86_64-pc-windows-msvc -S -winehprepare -disable-demotion -disable-cleanups < %s | FileCheck %s declare i32 @__CxxFrameHandler3(...)