summaryrefslogtreecommitdiffstats
path: root/clang
diff options
context:
space:
mode:
Diffstat (limited to 'clang')
-rw-r--r--clang/lib/CodeGen/CGCleanup.cpp41
-rw-r--r--clang/lib/CodeGen/CGExpr.cpp14
-rw-r--r--clang/lib/CodeGen/CGExprComplex.cpp6
-rw-r--r--clang/lib/CodeGen/CGExprScalar.cpp17
-rw-r--r--clang/lib/CodeGen/CodeGenFunction.h30
-rw-r--r--clang/test/CodeGenCXX/stmtexpr.cpp82
6 files changed, 165 insertions, 25 deletions
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index 3666858e63d..df6741da4a3 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -418,11 +418,15 @@ void CodeGenFunction::ResolveBranchFixups(llvm::BasicBlock *Block) {
}
/// Pops cleanup blocks until the given savepoint is reached.
-void CodeGenFunction::PopCleanupBlocks(EHScopeStack::stable_iterator Old) {
+void CodeGenFunction::PopCleanupBlocks(
+ EHScopeStack::stable_iterator Old,
+ std::initializer_list<llvm::Value **> ValuesToReload) {
assert(Old.isValid());
+ bool HadBranches = false;
while (EHStack.stable_begin() != Old) {
EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.begin());
+ HadBranches |= Scope.hasBranches();
// As long as Old strictly encloses the scope's enclosing normal
// cleanup, we're going to emit another normal cleanup which
@@ -432,14 +436,41 @@ void CodeGenFunction::PopCleanupBlocks(EHScopeStack::stable_iterator Old) {
PopCleanupBlock(FallThroughIsBranchThrough);
}
+
+ // If we didn't have any branches, the insertion point before cleanups must
+ // dominate the current insertion point and we don't need to reload any
+ // values.
+ if (!HadBranches)
+ return;
+
+ // Spill and reload all values that the caller wants to be live at the current
+ // insertion point.
+ for (llvm::Value **ReloadedValue : ValuesToReload) {
+ auto *Inst = dyn_cast_or_null<llvm::Instruction>(*ReloadedValue);
+ if (!Inst)
+ continue;
+ Address Tmp =
+ CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
+
+ // Find an insertion point after Inst and spill it to the temporary.
+ llvm::BasicBlock::iterator InsertBefore;
+ if (auto *Invoke = dyn_cast<llvm::InvokeInst>(Inst))
+ InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+ else
+ InsertBefore = std::next(Inst->getIterator());
+ CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
+
+ // Reload the value at the current insertion point.
+ *ReloadedValue = Builder.CreateLoad(Tmp);
+ }
}
/// Pops cleanup blocks until the given savepoint is reached, then add the
/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
-void
-CodeGenFunction::PopCleanupBlocks(EHScopeStack::stable_iterator Old,
- size_t OldLifetimeExtendedSize) {
- PopCleanupBlocks(Old);
+void CodeGenFunction::PopCleanupBlocks(
+ EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
+ std::initializer_list<llvm::Value **> ValuesToReload) {
+ PopCleanupBlocks(Old, ValuesToReload);
// Move our deferred cleanups onto the EH stack.
for (size_t I = OldLifetimeExtendedSize,
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 633494ab5c2..15cd7ccbaac 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1069,7 +1069,19 @@ LValue CodeGenFunction::EmitLValue(const Expr *E) {
const auto *cleanups = cast<ExprWithCleanups>(E);
enterFullExpression(cleanups);
RunCleanupsScope Scope(*this);
- return EmitLValue(cleanups->getSubExpr());
+ LValue LV = EmitLValue(cleanups->getSubExpr());
+ if (LV.isSimple()) {
+ // Defend against branches out of gnu statement expressions surrounded by
+ // cleanups.
+ llvm::Value *V = LV.getPointer();
+ Scope.ForceCleanup({&V});
+ return LValue::MakeAddr(Address(V, LV.getAlignment()), LV.getType(),
+ getContext(), LV.getAlignmentSource(),
+ LV.getTBAAInfo());
+ }
+ // FIXME: Is it possible to create an ExprWithCleanups that produces a
+ // bitfield lvalue or some other non-simple lvalue?
+ return LV;
}
case Expr::CXXDefaultArgExprClass:
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index 59bc9cdbc05..22e7fa64db9 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -198,7 +198,11 @@ public:
ComplexPairTy VisitExprWithCleanups(ExprWithCleanups *E) {
CGF.enterFullExpression(E);
CodeGenFunction::RunCleanupsScope Scope(CGF);
- return Visit(E->getSubExpr());
+ ComplexPairTy Vals = Visit(E->getSubExpr());
+ // Defend against dominance problems caused by jumps out of expression
+ // evaluation through the shared cleanup block.
+ Scope.ForceCleanup({&Vals.first, &Vals.second});
+ return Vals;
}
ComplexPairTy VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *E) {
assert(E->getType()->isAnyComplexType() && "Expected complex type!");
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 145248c04aa..2896e3556e5 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "CodeGenFunction.h"
+#include "CGCleanup.h"
#include "CGCXXABI.h"
#include "CGDebugInfo.h"
#include "CGObjCRuntime.h"
@@ -477,11 +478,7 @@ public:
return CGF.LoadCXXThis();
}
- Value *VisitExprWithCleanups(ExprWithCleanups *E) {
- CGF.enterFullExpression(E);
- CodeGenFunction::RunCleanupsScope Scope(CGF);
- return Visit(E->getSubExpr());
- }
+ Value *VisitExprWithCleanups(ExprWithCleanups *E);
Value *VisitCXXNewExpr(const CXXNewExpr *E) {
return CGF.EmitCXXNewExpr(E);
}
@@ -1691,6 +1688,16 @@ Value *ScalarExprEmitter::VisitStmtExpr(const StmtExpr *E) {
E->getExprLoc());
}
+Value *ScalarExprEmitter::VisitExprWithCleanups(ExprWithCleanups *E) {
+ CGF.enterFullExpression(E);
+ CodeGenFunction::RunCleanupsScope Scope(CGF);
+ Value *V = Visit(E->getSubExpr());
+ // Defend against dominance problems caused by jumps out of expression
+ // evaluation through the shared cleanup block.
+ Scope.ForceCleanup({&V});
+ return V;
+}
+
//===----------------------------------------------------------------------===//
// Unary Operators
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 659ec0fc5c8..6112f26f6d8 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -580,14 +580,10 @@ public:
CGF.DidCallStackSave = false;
}
- /// \brief Exit this cleanup scope, emitting any accumulated
- /// cleanups.
+ /// \brief Exit this cleanup scope, emitting any accumulated cleanups.
~RunCleanupsScope() {
- if (PerformCleanup) {
- CGF.DidCallStackSave = OldDidCallStackSave;
- CGF.PopCleanupBlocks(CleanupStackDepth,
- LifetimeExtendedCleanupStackSize);
- }
+ if (PerformCleanup)
+ ForceCleanup();
}
/// \brief Determine whether this scope requires any cleanups.
@@ -597,11 +593,15 @@ public:
/// \brief Force the emission of cleanups now, instead of waiting
/// until this object is destroyed.
- void ForceCleanup() {
+ /// \param ValuesToReload - A list of values that need to be available at
+ /// the insertion point after cleanup emission. If cleanup emission created
+ /// a shared cleanup block, these value pointers will be rewritten.
+ /// Otherwise, they not will be modified.
+ void ForceCleanup(std::initializer_list<llvm::Value**> ValuesToReload = {}) {
assert(PerformCleanup && "Already forced cleanup");
CGF.DidCallStackSave = OldDidCallStackSave;
- CGF.PopCleanupBlocks(CleanupStackDepth,
- LifetimeExtendedCleanupStackSize);
+ CGF.PopCleanupBlocks(CleanupStackDepth, LifetimeExtendedCleanupStackSize,
+ ValuesToReload);
PerformCleanup = false;
}
};
@@ -763,13 +763,17 @@ public:
/// \brief Takes the old cleanup stack size and emits the cleanup blocks
/// that have been added.
- void PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize);
+ void
+ PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize,
+ std::initializer_list<llvm::Value **> ValuesToReload = {});
/// \brief Takes the old cleanup stack size and emits the cleanup blocks
/// that have been added, then adds all lifetime-extended cleanups from
/// the given position to the stack.
- void PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize,
- size_t OldLifetimeExtendedStackSize);
+ void
+ PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize,
+ size_t OldLifetimeExtendedStackSize,
+ std::initializer_list<llvm::Value **> ValuesToReload = {});
void ResolveBranchFixups(llvm::BasicBlock *Target);
diff --git a/clang/test/CodeGenCXX/stmtexpr.cpp b/clang/test/CodeGenCXX/stmtexpr.cpp
index 7bf19bbfb46..99b3fbec541 100644
--- a/clang/test/CodeGenCXX/stmtexpr.cpp
+++ b/clang/test/CodeGenCXX/stmtexpr.cpp
@@ -80,3 +80,85 @@ int foo5(bool b) {
y = ({ A a(1); if (b) goto G; a.i; });
G: return y;
}
+
+// When we emit a full expression with cleanups that contains branches out of
+// the full expression, the result of the inner expression (the call to
+// call_with_cleanups in this case) may not dominate the fallthrough destination
+// of the shared cleanup block.
+//
+// In this case the CFG will be a sequence of two diamonds, but the only
+// dynamically possible execution paths are both left hand branches and both
+// right hand branches. The first diamond LHS will call bar, and the second
+// diamond LHS will assign the result to v, but the call to bar does not
+// dominate the assignment.
+int bar(A, int);
+extern "C" int cleanup_exit_scalar(bool b) {
+ int v = bar(A(1), ({ if (b) return 42; 13; }));
+ return v;
+}
+
+// CHECK-LABEL: define i32 @cleanup_exit_scalar({{.*}})
+// CHECK: call {{.*}} @_ZN1AC1Ei
+// Spill after bar.
+// CHECK: %[[v:[^ ]*]] = call i32 @_Z3bar1Ai({{.*}})
+// CHECK-NEXT: store i32 %[[v]], i32* %[[tmp:[^, ]*]]
+// Do cleanup.
+// CHECK: call {{.*}} @_ZN1AD1Ev
+// CHECK: switch
+// Reload before v assignment.
+// CHECK: %[[v:[^ ]*]] = load i32, i32* %[[tmp]]
+// CHECK-NEXT: store i32 %[[v]], i32* %v
+
+// No need to spill when the expression result is a constant, constants don't
+// have dominance problems.
+extern "C" int cleanup_exit_scalar_constant(bool b) {
+ int v = (A(1), (void)({ if (b) return 42; 0; }), 13);
+ return v;
+}
+
+// CHECK-LABEL: define i32 @cleanup_exit_scalar_constant({{.*}})
+// CHECK: store i32 13, i32* %v
+
+// Check for the same bug for lvalue expression evaluation kind.
+// FIXME: What about non-reference lvalues, like bitfield lvalues and vector
+// lvalues?
+int &getref();
+extern "C" int cleanup_exit_lvalue(bool cond) {
+ int &r = (A(1), ({ if (cond) return 0; (void)0; }), getref());
+ return r;
+}
+// CHECK-LABEL: define i32 @cleanup_exit_lvalue({{.*}})
+// CHECK: call {{.*}} @_ZN1AC1Ei
+// Spill after bar.
+// CHECK: %[[v:[^ ]*]] = call dereferenceable(4) i32* @_Z6getrefv({{.*}})
+// CHECK-NEXT: store i32* %[[v]], i32** %[[tmp:[^, ]*]]
+// Do cleanup.
+// CHECK: call {{.*}} @_ZN1AD1Ev
+// CHECK: switch
+// Reload before v assignment.
+// CHECK: %[[v:[^ ]*]] = load i32*, i32** %[[tmp]]
+// CHECK-NEXT: store i32* %[[v]], i32** %r
+
+
+// We handle ExprWithCleanups for complex evaluation type separately, and it had
+// the same bug.
+_Complex float bar_complex(A, int);
+extern "C" int cleanup_exit_complex(bool b) {
+ _Complex float v = bar_complex(A(1), ({ if (b) return 42; 13; }));
+ return v;
+}
+
+// CHECK-LABEL: define i32 @cleanup_exit_complex({{.*}})
+// CHECK: call {{.*}} @_ZN1AC1Ei
+// Spill after bar.
+// CHECK: call {{.*}} @_Z11bar_complex1Ai({{.*}})
+// CHECK: store float %{{.*}}, float* %[[tmp1:[^, ]*]]
+// CHECK: store float %{{.*}}, float* %[[tmp2:[^, ]*]]
+// Do cleanup.
+// CHECK: call {{.*}} @_ZN1AD1Ev
+// CHECK: switch
+// Reload before v assignment.
+// CHECK: %[[v1:[^ ]*]] = load float, float* %[[tmp1]]
+// CHECK: %[[v2:[^ ]*]] = load float, float* %[[tmp2]]
+// CHECK: store float %[[v1]], float* %v.realp
+// CHECK: store float %[[v2]], float* %v.imagp
OpenPOWER on IntegriCloud