From 615f9b7162ac6444c9e45042477634b73ba8850a Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Sun, 8 Dec 2013 00:50:58 +0000 Subject: [PATCH] Fix inlining to not produce duplicate landingpad clauses Before this change, inlining one "invoke" into an outer "invoke" call site can lead to the outer landingpad's catch/filter clauses being copied multiple times into the resulting landingpad. This happens: * when the inlined function contains multiple "resume" instructions, because forwardResume() copies the clauses but is called multiple times; * when the inlined function contains a "resume" and a "call", because HandleCallsInBlockInlinedThroughInvoke() copies the clauses but is redundant with forwardResume(). Fix this by deduplicating the code. This problem doesn't lead to any incorrect execution; it's only untidy. This change will make fixing PR17872 a little easier. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@196710 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/InlineFunction.cpp | 32 ++--- test/Transforms/Inline/inline_invoke.ll | 5 - .../Inline/invoke-combine-clauses.ll | 117 ++++++++++++++++++ 3 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 test/Transforms/Inline/invoke-combine-clauses.ll diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp index 6d23af45a10..405f77e793f 100644 --- a/lib/Transforms/Utils/InlineFunction.cpp +++ b/lib/Transforms/Utils/InlineFunction.cpp @@ -144,7 +144,6 @@ BasicBlock *InvokeInliningInfo::getInnerResumeDest() { void InvokeInliningInfo::forwardResume(ResumeInst *RI, SmallPtrSet &InlinedLPads) { BasicBlock *Dest = getInnerResumeDest(); - LandingPadInst *OuterLPad = getLandingPadInst(); BasicBlock *Src = RI->getParent(); BranchInst::Create(Dest, Src); @@ -155,16 +154,6 @@ void InvokeInliningInfo::forwardResume(ResumeInst *RI, InnerEHValuesPHI->addIncoming(RI->getOperand(0), Src); RI->eraseFromParent(); - - // Append the clauses from the outer landing pad instruction into the inlined - // landing pad instructions. - for (SmallPtrSet::iterator I = InlinedLPads.begin(), - E = InlinedLPads.end(); I != E; ++I) { - LandingPadInst *InlinedLPad = *I; - for (unsigned OuterIdx = 0, OuterNum = OuterLPad->getNumClauses(); - OuterIdx != OuterNum; ++OuterIdx) - InlinedLPad->addClause(OuterLPad->getClause(OuterIdx)); - } } /// HandleCallsInBlockInlinedThroughInvoke - When we inline a basic block into @@ -174,18 +163,9 @@ void InvokeInliningInfo::forwardResume(ResumeInst *RI, /// nodes in that block with the values specified in InvokeDestPHIValues. static void HandleCallsInBlockInlinedThroughInvoke(BasicBlock *BB, InvokeInliningInfo &Invoke) { - LandingPadInst *LPI = Invoke.getLandingPadInst(); - for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E; ) { Instruction *I = BBI++; - if (LandingPadInst *L = dyn_cast(I)) { - unsigned NumClauses = LPI->getNumClauses(); - L->reserveClauses(NumClauses); - for (unsigned i = 0; i != NumClauses; ++i) - L->addClause(LPI->getClause(i)); - } - // We only need to check for function calls: inlined invoke // instructions require no special handling. CallInst *CI = dyn_cast(I); @@ -248,6 +228,18 @@ static void HandleInlinedInvoke(InvokeInst *II, BasicBlock *FirstNewBlock, if (InvokeInst *II = dyn_cast(I->getTerminator())) InlinedLPads.insert(II->getLandingPadInst()); + // Append the clauses from the outer landing pad instruction into the inlined + // landing pad instructions. + LandingPadInst *OuterLPad = Invoke.getLandingPadInst(); + for (SmallPtrSet::iterator I = InlinedLPads.begin(), + E = InlinedLPads.end(); I != E; ++I) { + LandingPadInst *InlinedLPad = *I; + unsigned OuterNum = OuterLPad->getNumClauses(); + InlinedLPad->reserveClauses(OuterNum); + for (unsigned OuterIdx = 0; OuterIdx != OuterNum; ++OuterIdx) + InlinedLPad->addClause(OuterLPad->getClause(OuterIdx)); + } + for (Function::iterator BB = FirstNewBlock, E = Caller->end(); BB != E; ++BB){ if (InlinedCodeInfo.ContainsCalls) HandleCallsInBlockInlinedThroughInvoke(BB, Invoke); diff --git a/test/Transforms/Inline/inline_invoke.ll b/test/Transforms/Inline/inline_invoke.ll index c3941388f93..c53bb5aa17b 100644 --- a/test/Transforms/Inline/inline_invoke.ll +++ b/test/Transforms/Inline/inline_invoke.ll @@ -96,7 +96,6 @@ eh.resume: ; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0 ; CHECK-NEXT: cleanup ; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) -; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) ; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A]]) ; CHECK-NEXT: to label %[[LBL:[^\s]+]] unwind ; CHECK: [[LBL]]: @@ -167,7 +166,6 @@ eh.resume: ; CHECK-NEXT: [[LPADVAL1:%.*]] = landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0 ; CHECK-NEXT: cleanup ; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) -; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) ; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A1]]) ; CHECK-NEXT: to label %[[RESUME1:[^\s]+]] unwind ; CHECK: [[RESUME1]]: @@ -187,7 +185,6 @@ eh.resume: ; CHECK-NEXT: [[LPADVAL2:%.*]] = landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0 ; CHECK-NEXT: cleanup ; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) -; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) ; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A2]]) ; CHECK-NEXT: to label %[[RESUME2:[^\s]+]] unwind ; CHECK: [[RESUME2]]: @@ -275,7 +272,6 @@ lpad.cont: ; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0 ; CHECK-NEXT: cleanup ; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) -; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) ; CHECK-NEXT: invoke void @_ZN1AD1Ev( ; CHECK-NEXT: to label %[[L:[^\s]+]] unwind ; CHECK: [[L]]: @@ -322,7 +318,6 @@ terminate: ; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0 ; CHECK-NEXT: cleanup ; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) -; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*) ; CHECK-NEXT: invoke void @_ZN1AD1Ev( ; CHECK-NEXT: to label %[[L:[^\s]+]] unwind ; CHECK: [[L]]: diff --git a/test/Transforms/Inline/invoke-combine-clauses.ll b/test/Transforms/Inline/invoke-combine-clauses.ll new file mode 100644 index 00000000000..5f06039b9ed --- /dev/null +++ b/test/Transforms/Inline/invoke-combine-clauses.ll @@ -0,0 +1,117 @@ +; RUN: opt %s -inline -S | FileCheck %s + +declare void @external_func() +declare void @abort() + +@exception_inner = external global i8 +@exception_outer = external global i8 +@condition = external global i1 + + +; Check for a bug in which multiple "resume" instructions in the +; inlined function caused "catch i8* @exception_outer" to appear +; multiple times in the resulting landingpad. + +define internal void @inner_multiple_resume() { + invoke void @external_func() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_inner + %cond = load i1* @condition + br i1 %cond, label %resume1, label %resume2 +resume1: + resume i32 1 +resume2: + resume i32 2 +} + +define void @outer_multiple_resume() { + invoke void @inner_multiple_resume() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_outer + resume i32 %lp +} +; CHECK: define void @outer_multiple_resume() +; CHECK: %lp.i = landingpad +; CHECK-NEXT: catch i8* @exception_inner +; CHECK-NEXT: catch i8* @exception_outer +; Check that there isn't another "catch" clause: +; CHECK-NEXT: load + + +; Check for a bug in which having a "resume" and a "call" in the +; inlined function caused "catch i8* @exception_outer" to appear +; multiple times in the resulting landingpad. + +define internal void @inner_resume_and_call() { + call void @external_func() + invoke void @external_func() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_inner + resume i32 %lp +} + +define void @outer_resume_and_call() { + invoke void @inner_resume_and_call() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_outer + resume i32 %lp +} +; CHECK: define void @outer_resume_and_call() +; CHECK: %lp.i = landingpad +; CHECK-NEXT: catch i8* @exception_inner +; CHECK-NEXT: catch i8* @exception_outer +; Check that there isn't another "catch" clause: +; CHECK-NEXT: br + + +; Check what happens if the inlined function contains an "invoke" but +; no "resume". In this case, the inlined landingpad does not need to +; include the "catch i8* @exception_outer" clause from the outer +; function (since the outer function's landingpad will not be +; reachable), but it's OK to include this clause. + +define internal void @inner_no_resume_or_call() { + invoke void @external_func() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_inner + ; A landingpad might have no "resume" if a C++ destructor aborts. + call void @abort() noreturn nounwind + unreachable +} + +define void @outer_no_resume_or_call() { + invoke void @inner_no_resume_or_call() + to label %cont unwind label %lpad +cont: + ret void +lpad: + %lp = landingpad i32 personality i8* null + catch i8* @exception_outer + resume i32 %lp +} +; CHECK: define void @outer_no_resume_or_call() +; CHECK: %lp.i = landingpad +; CHECK-NEXT: catch i8* @exception_inner +; CHECK-NEXT: catch i8* @exception_outer +; Check that there isn't another "catch" clause: +; CHECK-NEXT: call void @abort()