gollvm: fix exception handling stmt bugs
Revise Llvm_backend::exception_handler_statement(), which was not
correct as implemented. Specifically, if an exception is thrown by
something in the catch clause, the catch block for it needs to store
the exception and make a note that the exception took place... then
after the "finally" block has executed, the cleanup code should
rethrow the exception (if there was one) as opposed to unconditionally
returning.
Change-Id: Ia63297cad1bc601f9ab5066ad879b1bc5739efd1
Reviewed-on: https://go-review.googlesource.com/56410
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/llvm-gofrontend/go-llvm-bnode.cpp b/llvm-gofrontend/go-llvm-bnode.cpp
index 7e4d0db..a99cc98 100644
--- a/llvm-gofrontend/go-llvm-bnode.cpp
+++ b/llvm-gofrontend/go-llvm-bnode.cpp
@@ -336,7 +336,7 @@
Bvariable *Bnode::var() const
{
- assert(flavor() == N_Var);
+ assert(flavor() == N_Var || flavor() == N_ExcepStmt);
return u.var;
}
@@ -844,13 +844,16 @@
Bstatement *body,
Bstatement *onexception,
Bstatement *finally,
+ Bvariable *finTempVar,
Location loc)
{
assert(body);
assert(onexception);
assert(finally);
+ assert(finTempVar);
std::vector<Bnode *> kids = { body, onexception, finally };
Bstatement *rval = new Bstatement(N_ExcepStmt, func, kids, loc);
+ rval->u.var = finTempVar;
return archive(rval);
}
@@ -893,8 +896,6 @@
}
-
-
Bblock *BnodeBuilder::mkBlock(Bfunction *func,
const std::vector<Bvariable *> &vars,
Location loc)
diff --git a/llvm-gofrontend/go-llvm-bnode.h b/llvm-gofrontend/go-llvm-bnode.h
index 6fe4dcd..f3e9615 100644
--- a/llvm-gofrontend/go-llvm-bnode.h
+++ b/llvm-gofrontend/go-llvm-bnode.h
@@ -199,7 +199,7 @@
private:
std::vector<Bnode *> kids_;
union {
- Bvariable *var;
+ Bvariable *var; // filled in only for Var and ExcepStmt nodes
Bfunction *func; // filled in only for fcn constants, calls, conditionals
SwitchDescriptor *swcases;
int indices; // for composite expressions, index to BnodeBuilder's indexvecs_
@@ -310,6 +310,7 @@
Bstatement *body,
Bstatement *onexception,
Bstatement *finally,
+ Bvariable *finTempVar,
Location loc);
Bstatement *mkSwitchStmt(Bfunction *func,
Bexpression *swvalue,
diff --git a/llvm-gofrontend/go-llvm.cpp b/llvm-gofrontend/go-llvm.cpp
index 0368fee..a9f2e27 100644
--- a/llvm-gofrontend/go-llvm.cpp
+++ b/llvm-gofrontend/go-llvm.cpp
@@ -1688,9 +1688,14 @@
assert(func == except_stmt->function());
assert(!finally_stmt || func == finally_stmt->function());
+ std::string tname(namegen("finvar"));
+ Bvariable *tvar = func->localVariable(tname, boolType(),
+ false, location);
+ tvar->markAsTemporary();
Bstatement *excepst = nbuilder_.mkExcepStmt(func, bstat,
except_stmt,
- finally_stmt, location);
+ finally_stmt, tvar,
+ location);
return excepst;
}
@@ -2372,6 +2377,11 @@
std::pair<llvm::Instruction*, llvm::BasicBlock *>
postProcessInst(llvm::Instruction *inst,
llvm::BasicBlock *curblock);
+ llvm::Value *populateCatchPadBlock(llvm::BasicBlock *catchpadbb,
+ Bvariable *finvar);
+ llvm::BasicBlock *populateFinallyBlock(llvm::BasicBlock *finBB,
+ Bvariable *finvar,
+ llvm::Value *extmp);
DIBuildHelper *dibuildhelper() const { return dibuildhelper_; }
Llvm_linemap *linemap() { return be_->linemap(); }
@@ -2384,7 +2394,7 @@
std::vector<llvm::BasicBlock*> padBlockStack_;
std::set<llvm::AllocaInst *> temporariesDiscovered_;
std::vector<llvm::AllocaInst *> newTemporaries_;
- llvm::BasicBlock* finallyBlock_;
+ llvm::BasicBlock *finallyBlock_;
Bstatement *cachedReturn_;
bool emitOrphanedCode_;
};
@@ -2731,11 +2741,6 @@
Bexpression *defcallex = defst->getDeferStmtDeferCall();
Bexpression *undcallex = defst->getDeferStmtUndeferCall();
- // Finish bb (see the comments for Llvm_backend::function_defer_statement
- // as to why we use this name).
- llvm::BasicBlock *finbb =
- llvm::BasicBlock::Create(context_, be_->namegen("finish"), func);
-
// Landing pad to which control will be transferred if an exception
// is thrown when executing "undcallex".
llvm::BasicBlock *padbb =
@@ -2744,6 +2749,12 @@
// Catch BB will contain checkdefer (defercall) code.
llvm::BasicBlock *catchbb =
llvm::BasicBlock::Create(context_, be_->namegen("catch"), func);
+
+ // Finish bb (see the comments for Llvm_backend::function_defer_statement
+ // as to why we use this name).
+ llvm::BasicBlock *finbb =
+ llvm::BasicBlock::Create(context_, be_->namegen("finish"), func);
+
if (curblock)
llvm::BranchInst::Create(finbb, curblock);
curblock = finbb;
@@ -2778,6 +2789,131 @@
return contbb;
}
+// Populate the landing pad used to catch exceptions thrown from
+// the catch code from an exceptions handling statement (pad "P2"
+// in the diagram in the header comment for GenBlocks::genExcep).
+
+llvm::Value *GenBlocks::populateCatchPadBlock(llvm::BasicBlock *catchpadbb,
+ Bvariable *finvar)
+{
+ LIRBuilder builder(context_, llvm::ConstantFolder());
+ builder.SetInsertPoint(catchpadbb);
+
+ // Create landing pad instruction.
+ llvm::Type *eht = be_->landingPadExceptionType();
+ llvm::LandingPadInst *caughtResult =
+ builder.CreateLandingPad(eht, 0, be_->namegen("ex2"));
+ caughtResult->setCleanup(true);
+
+ // Create temporary into which caught result will be stored
+ std::string tag(be_->namegen("ehtmp"));
+ llvm::AllocaInst *ai = new llvm::AllocaInst(eht, 0, tag);
+ temporariesDiscovered_.insert(ai);
+ newTemporaries_.push_back(ai);
+
+ // Store caught result into temporary.
+ builder.CreateStore(caughtResult, ai);
+
+ // Emit "finally = false" into catch pad BB following pad inst
+ builder.SetInsertPoint(catchpadbb);
+ llvm::Value *fval = be_->boolean_constant_expression(false)->value();
+ builder.CreateStore(fval, finvar->value());
+
+ return ai;
+}
+
+// At end of finally BB, emit:
+//
+// if (finok)
+// return
+// else
+// resume EXC
+//
+// where EXC is the exception that caused control to flow
+// into the catch pad bb.
+//
+// In addition, this routine creates the shared return statement
+// (if there is a cached return) and fixes up the finally stmt
+// resume block.
+
+llvm::BasicBlock *GenBlocks::populateFinallyBlock(llvm::BasicBlock *finBB,
+ Bvariable *finvar,
+ llvm::Value *extmp)
+{
+ LIRBuilder builder(context_, llvm::ConstantFolder());
+ builder.SetInsertPoint(finBB);
+ llvm::Function *func = function()->function();
+ llvm::BasicBlock *finResBB =
+ llvm::BasicBlock::Create(context_, be_->namegen("finres"), func);
+ llvm::BasicBlock *finRetBB =
+ llvm::BasicBlock::Create(context_, be_->namegen("finret"), func);
+ std::string lname(be_->namegen("fload"));
+ llvm::LoadInst *finvarload = builder.CreateLoad(finvar->value(), lname);
+ llvm::Value *tval = be_->boolean_constant_expression(true)->value();
+ llvm::Value *cmp = builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_EQ,
+ finvarload, tval,
+ be_->namegen("icmp"));
+ builder.CreateCondBr(cmp, finRetBB, finResBB);
+
+ // Populate return block
+ if (cachedReturn_ != nullptr) {
+ llvm::BasicBlock *curblock = finRetBB;
+ Bexpression *re = cachedReturn_->getReturnStmtExpr();
+ llvm::BasicBlock *bb = walkExpr(curblock, re);
+ assert(curblock == bb);
+ cachedReturn_ = nullptr;
+ }
+
+ // Populate resume block
+ builder.SetInsertPoint(finResBB);
+ std::string ename(be_->namegen("excv"));
+ llvm::LoadInst *exload = builder.CreateLoad(extmp, ename);
+ builder.CreateResume(exload);
+
+ return finRetBB;
+}
+
+// Create the needed control flow for an exception handler statement.
+// At a high level, we want something that looks like:
+//
+// try { body }
+// catch { exceptioncode }
+// finally { finallycode }
+//
+// This means creating the following landing pads + control flow:
+//
+// { /* begin body */
+// <any calls are converted to invoke instructions,
+// targeting landing pad P1>
+// /* end body */
+// }
+// goto finok;
+// pad P1:
+// goto catch;
+// catch: {
+// /* begin exceptioncode */
+// <any calls are converted to invoke inst,
+// targeting landing pad P2>
+// /* end exceptioncode */
+// }
+// goto finok;
+// pad P2:
+// store exception -> ehtmp
+// finvar = false
+// goto finally;
+// finok:
+// finvar = true
+// finally:
+// { /* begin finallycode */
+// <calls here are left as is>
+// /* end finallycode */
+// }
+// if (finvar)
+// <normal return>
+// else
+// rethrow exception ehtmp
+//
+
llvm::BasicBlock *GenBlocks::genExcep(Bstatement *excepst,
llvm::BasicBlock *curblock)
{
@@ -2794,26 +2930,52 @@
assert(ifexception);
Bstatement *finally = excepst->getExcepStmtFinally(); // may be null
- // Create a landing pad block. This pad will be where control
- // will arrive if an exception is thrown within the "body" code.
- llvm::BasicBlock *padbb =
- llvm::BasicBlock::Create(context_, be_->namegen("pad"), func);
+ // This temp will track what sort of action is needed at the end of
+ // of the "finally" clause. For this variable, a value of 'true'
+ // signals a normal return, and a value of 'false' indicates that
+ // there was an additional exception thrown during execution of the
+ // "ifexception" code (as opposed to an exception thrown during
+ // execution of "body") that needs to be passed to unwind/resume
+ // after the "finally" code.
+ Bvariable *finvar = excepst->var();
+ assert(finvar);
+
+ // Create a "finok" BB; this block will set the "finally" variable
+ // to true and then jump to the finally block.
+ std::string fbbname(be_->namegen("finok"));
+ llvm::BasicBlock *finokBB =
+ llvm::BasicBlock::Create(context_, fbbname, func);
// Create a "finally" BB, corresponding to where control will wind
// up once both the body and (possibly) the exception clause are
// complete. This will also be where we'll place any code in the
// "finally" statement above.
std::string cbbname(be_->namegen("finally"));
- llvm::BasicBlock *contbb =
+ llvm::BasicBlock *contBB =
llvm::BasicBlock::Create(context_, cbbname, func);
- // Not expecting to see nested exception statements, assert if
- // this crops up.
+ // Emit an assignment "finally = true" into the finok block,
+ // then a jump to the finally block.
+ {
+ LIRBuilder builder(context_, llvm::ConstantFolder());
+ builder.SetInsertPoint(finokBB);
+ llvm::Value *tval = be_->boolean_constant_expression(true)->value();
+ builder.CreateStore(tval, finvar->value());
+ llvm::BranchInst::Create(contBB, finokBB);
+ }
+
+ // Create a landing pad block. This pad will be where control
+ // will arrive if an exception is thrown within the "body" code.
+ llvm::BasicBlock *padbb =
+ llvm::BasicBlock::Create(context_, be_->namegen("pad"), func);
+
+ // Not expecting to see nested exception statements, assert if this
+ // crops up.
assert(padBlockStack_.empty());
// If there is a finally block, then record the block for purposes
// of return handling.
- finallyBlock_ = (finally ? contbb : nullptr);
+ finallyBlock_ = (finally ? finokBB : nullptr);
assert(!cachedReturn_);
// Push first pad block onto stack. The presence of a non-empty pad
@@ -2830,16 +2992,7 @@
// If the body block ended without a return, then insert a jump
// to the continue / or finally clause.
if (curblock)
- llvm::BranchInst::Create(contbb, curblock);
-
- // Emit landing pad inst in pad block, followed by branch to catch bb.
- llvm::LandingPadInst *padinst =
- llvm::LandingPadInst::Create(be_->landingPadExceptionType(),
- 0, be_->namegen("ex"), padbb);
- padinst->addClause(llvm::Constant::getNullValue(be_->llvmPtrType()));
- llvm::BasicBlock *catchbb =
- llvm::BasicBlock::Create(context_, be_->namegen("catch"), func);
- llvm::BranchInst::Create(catchbb, padbb);
+ llvm::BranchInst::Create(finokBB, curblock);
// Create second pad block. Here the idea is that if an exception
// is thrown as a result of any call within the catch ("ifexception" code)
@@ -2847,33 +3000,46 @@
llvm::BasicBlock *catchpadbb =
llvm::BasicBlock::Create(context_, be_->namegen("catchpad"), func);
+ // Block containing catch code
+ llvm::BasicBlock *catchbb =
+ llvm::BasicBlock::Create(context_, be_->namegen("catch"), func);
+
+ // Emit landing pad inst in pad block, followed by branch to catch bb.
+ {
+ LIRBuilder builder(context_, llvm::ConstantFolder());
+ builder.SetInsertPoint(padbb);
+ llvm::LandingPadInst *padinst =
+ builder.CreateLandingPad(be_->landingPadExceptionType(),
+ 0, be_->namegen("ex"));
+ padinst->addClause(llvm::Constant::getNullValue(be_->llvmPtrType()));
+ llvm::BranchInst::Create(catchbb, padbb);
+ }
+
// Push second pad, walk exception stmt, then pop the pad.
padBlockStack_.push_back(catchpadbb);
auto bb = walk(ifexception, catchbb);
if (bb)
- llvm::BranchInst::Create(contbb, bb);
+ llvm::BranchInst::Create(finokBB, bb);
padBlockStack_.pop_back();
// Return handling now complete.
finallyBlock_ = nullptr;
- // Fix up second pad block, followed by branch to continue block.
- llvm::LandingPadInst *padinst2 =
- llvm::LandingPadInst::Create(be_->landingPadExceptionType(),
- 0, be_->namegen("ex2"), catchpadbb);
- padinst2->addClause(llvm::Constant::getNullValue(be_->llvmPtrType()));
- llvm::BranchInst::Create(contbb, catchpadbb);
+ // Create landing pad instruction in the second pad block. For this
+ // landing pad, we want to capture the exception object into a
+ // temporary, and set "finally = false" to indicate that an exception
+ // was thrown.
+ llvm::Value *extmp = populateCatchPadBlock(catchpadbb, finvar);
+ llvm::BranchInst::Create(contBB, catchpadbb);
// Handle finally statement where applicable.
- curblock = contbb;
+ curblock = contBB;
if (finally != nullptr) {
curblock = walk(finally, curblock);
- if (cachedReturn_ != nullptr) {
- Bexpression *re = cachedReturn_->getReturnStmtExpr();
- llvm::BasicBlock *bb = walkExpr(curblock, re);
- assert(curblock == bb);
- cachedReturn_ = nullptr;
- }
+
+ // Augment the end of the finally block with an if/jump to
+ // return or resume, and populate the return/resume blocks.
+ curblock = populateFinallyBlock(curblock, finvar, extmp);
}
return curblock;
diff --git a/unittests/BackendCore/BackendStmtTests.cpp b/unittests/BackendCore/BackendStmtTests.cpp
index 01b6204..69b31db 100644
--- a/unittests/BackendCore/BackendStmtTests.cpp
+++ b/unittests/BackendCore/BackendStmtTests.cpp
@@ -519,10 +519,6 @@
store i8 0, i8* %x
br label %finish.0
-finish.0: ; preds = %catch.0, %entry
- invoke void @deferreturn(i8* nest undef, i8* %x)
- to label %cont.0 unwind label %pad.0
-
pad.0: ; preds = %finish.0
%ex.0 = landingpad { i8*, i32 }
catch i8* null
@@ -532,6 +528,10 @@
call void @checkdefer(i8* nest undef, i8* %x)
br label %finish.0
+finish.0: ; preds = %catch.0, %entry
+ invoke void @deferreturn(i8* nest undef, i8* %x)
+ to label %cont.0 unwind label %pad.0
+
cont.0: ; preds = %finish.0
ret void
}
@@ -611,20 +611,28 @@
const char *exp = R"RAW_RESULT(
define void @baz(i8* nest %nest.0) #0 personality i32 (i32, i32, i64, i8*, i8*)* @__gccgo_personality_v0 {
entry:
+ %ehtmp.0 = alloca { i8*, i32 }
%x = alloca i64
+ %finvar.0 = alloca i8
store i64 0, i64* %x
%call.0 = invoke i64 @id(i8* nest undef, i64 99)
to label %cont.0 unwind label %pad.0
+finok.0: ; preds = %cont.2, %cont.1
+ store i8 1, i8* %finvar.0
+ br label %finally.0
+
+finally.0: ; preds = %catchpad.0, %finok.0
+ call void @ohstopit(i8* nest undef)
+ %fload.0 = load i8, i8* %finvar.0
+ %icmp.0 = icmp eq i8 %fload.0, 1
+ br i1 %icmp.0, label %finret.0, label %finres.0
+
pad.0: ; preds = %cont.0, %entry
%ex.0 = landingpad { i8*, i32 }
catch i8* null
br label %catch.0
-finally.0: ; preds = %catchpad.0, %cont.2, %cont.1
- call void @ohstopit(i8* nest undef)
- ret void
-
cont.0: ; preds = %entry
store i64 %call.0, i64* %x
invoke void @plark(i8* nest undef)
@@ -632,19 +640,28 @@
cont.1: ; preds = %cont.0
store i64 123, i64* %x
+ br label %finok.0
+
+catchpad.0: ; preds = %catch.0
+ %ex2.0 = landingpad { i8*, i32 }
+ cleanup
+ store { i8*, i32 } %ex2.0, { i8*, i32 }* %ehtmp.0
+ store i8 0, i8* %finvar.0
br label %finally.0
catch.0: ; preds = %pad.0
invoke void @plix(i8* nest undef)
to label %cont.2 unwind label %catchpad.0
-catchpad.0: ; preds = %catch.0
- %ex2.0 = landingpad { i8*, i32 }
- catch i8* null
- br label %finally.0
-
cont.2: ; preds = %catch.0
- br label %finally.0
+ br label %finok.0
+
+finres.0: ; preds = %finally.0
+ %excv.0 = load { i8*, i32 }, { i8*, i32 }* %ehtmp.0
+ resume { i8*, i32 } %excv.0
+
+finret.0: ; preds = %finally.0
+ ret void
}
)RAW_RESULT";
@@ -763,25 +780,31 @@
const char *exp = R"RAW_RESULT(
define i64 @baz(i8* nest %nest.0, i64 %p0) #0 personality i32 (i32, i32, i64, i8*, i8*)* @__gccgo_personality_v0 {
entry:
+ %ehtmp.0 = alloca { i8*, i32 }
%p0.addr = alloca i64
%ret = alloca i64
+ %finvar.0 = alloca i8
store i64 %p0, i64* %p0.addr
store i64 0, i64* %ret
%call.0 = invoke i64 @splat(i8* nest undef, i64 99)
to label %cont.0 unwind label %pad.0
-pad.0: ; preds = %entry
- %ex.0 = landingpad { i8*, i32 }
- catch i8* null
- br label %catch.0
+finok.0: ; preds = %cont.1, %else.0, %then.0
+ store i8 1, i8* %finvar.0
+ br label %finally.0
-finally.0: ; preds = %catchpad.0, %cont.1, %else.0, %then.0
+finally.0: ; preds = %catchpad.0, %finok.0
%call.2 = call i64 @splat(i8* nest undef, i64 987)
%icmp.1 = icmp eq i64 %call.2, 2
%zext.1 = zext i1 %icmp.1 to i8
%trunc.1 = trunc i8 %zext.1 to i1
br i1 %trunc.1, label %then.1, label %else.1
+pad.0: ; preds = %entry
+ %ex.0 = landingpad { i8*, i32 }
+ catch i8* null
+ br label %catch.0
+
cont.0: ; preds = %entry
%icmp.0 = icmp eq i64 %call.0, 88
%zext.0 = zext i1 %icmp.0 to i8
@@ -790,25 +813,27 @@
then.0: ; preds = %cont.0
store i64 22, i64* %ret
- br label %finally.0
+ br label %finok.0
else.0: ; preds = %cont.0
%p0.ld.0 = load i64, i64* %p0.addr
store i64 %p0.ld.0, i64* %ret
+ br label %finok.0
+
+catchpad.0: ; preds = %catch.0
+ %ex2.0 = landingpad { i8*, i32 }
+ cleanup
+ store { i8*, i32 } %ex2.0, { i8*, i32 }* %ehtmp.0
+ store i8 0, i8* %finvar.0
br label %finally.0
catch.0: ; preds = %pad.0
%call.1 = invoke i64 @splat(i8* nest undef, i64 13)
to label %cont.1 unwind label %catchpad.0
-catchpad.0: ; preds = %catch.0
- %ex2.0 = landingpad { i8*, i32 }
- catch i8* null
- br label %finally.0
-
cont.1: ; preds = %catch.0
store i64 %call.1, i64* %ret
- br label %finally.0
+ br label %finok.0
then.1: ; preds = %finally.0
store i64 9, i64* %ret
@@ -816,11 +841,20 @@
ret i64 %ret.ld.3
fallthrough.1: ; preds = %else.1
- %ret.ld.1 = load i64, i64* %ret
- ret i64 %ret.ld.1
+ %fload.0 = load i8, i8* %finvar.0
+ %icmp.2 = icmp eq i8 %fload.0, 1
+ br i1 %icmp.2, label %finret.0, label %finres.0
else.1: ; preds = %finally.0
br label %fallthrough.1
+
+finres.0: ; preds = %fallthrough.1
+ %excv.0 = load { i8*, i32 }, { i8*, i32 }* %ehtmp.0
+ resume { i8*, i32 } %excv.0
+
+finret.0: ; preds = %fallthrough.1
+ %ret.ld.1 = load i64, i64* %ret
+ ret i64 %ret.ld.1
}
)RAW_RESULT";
