[PATCH] WIP Fix for temporary destructors in conditionals
Pavel Labath
labath at google.com
Fri Aug 2 06:36:09 PDT 2013
Actually, it turns out that the SymbolReaper is working just fine and needs no
modifications. So, all I needed to do was bind the values to the conditional
sub-expressions. I will take one more look at this one Monday, but right now it
seems that all tests pass and it's working as expected. Could you take a look at
this?
Hi jordan_rose,
http://llvm-reviews.chandlerc.com/D1259
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1259?vs=3126&id=3148#toc
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/temporaries.cpp
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1295,14 +1295,14 @@
return state->getSVal(Ex, LCtx);
}
-static const Stmt *ResolveCondition(const Stmt *Condition,
- const CFGBlock *B) {
+static std::pair<const Stmt *, const Stmt *>
+ResolveCondition(const Stmt *Condition, const Stmt *Term, const CFGBlock *B) {
if (const Expr *Ex = dyn_cast<Expr>(Condition))
Condition = Ex->IgnoreParens();
const BinaryOperator *BO = dyn_cast<BinaryOperator>(Condition);
if (!BO || !BO->isLogicalOp())
- return Condition;
+ return std::make_pair(Condition, Term);
// For logical operations, we still have the case where some branches
// use the traditional "merge" approach and others sink the branch
@@ -1320,20 +1320,37 @@
continue;
if (CS->getStmt() != Condition)
break;
- return Condition;
+ return std::make_pair(Condition, Term);
}
assert(I != E);
while (Condition) {
BO = dyn_cast<BinaryOperator>(Condition);
if (!BO || !BO->isLogicalOp())
- return Condition;
+ return std::make_pair(Condition, Term);
+ Term = BO;
Condition = BO->getRHS()->IgnoreParens();
}
llvm_unreachable("could not resolve condition");
}
+static ProgramStateRef SaveConditionValue(ProgramStateRef State,
+ const LocationContext *LC,
+ const Stmt *Condition,
+ const Stmt *Term, bool value) {
+ if (const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(Term)) {
+ if ((BinOp->getOpcode() == BO_LOr && value) ||
+ (BinOp->getOpcode() == BO_LAnd && !value) ||
+ BinOp->getRHS()->IgnoreParens() == Condition) {
+ return State->BindExpr(
+ Term, LC,
+ State->getStateManager().getSValBuilder().makeTruthVal(value));
+ }
+ }
+ return State;
+}
+
void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
NodeBuilderContext& BldCtx,
ExplodedNode *Pred,
@@ -1351,12 +1368,15 @@
return;
}
-
- // Resolve the condition in the precense of nested '||' and '&&'.
if (const Expr *Ex = dyn_cast<Expr>(Condition))
Condition = Ex->IgnoreParens();
- Condition = ResolveCondition(Condition, BldCtx.getBlock());
+ // If the value is already available, we don't need to do anything.
+ if(Pred->getState()->getSVal(Condition, Pred->getLocationContext()).isUnknownOrUndef()) {
+ // Resolve the condition in the precense of nested '||' and '&&'.
+ llvm::tie(Condition, Term) = ResolveCondition(Condition, Term, BldCtx.getBlock());
+ }
+
PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
Condition->getLocStart(),
"Error evaluating branch");
@@ -1413,17 +1433,19 @@
// Process the true branch.
if (builder.isFeasible(true)) {
- if (StTrue)
+ if (StTrue) {
+ StTrue = SaveConditionValue(StTrue, BldCtx.LC, Condition, Term, true);
builder.generateNode(StTrue, true, PredI);
- else
+ } else
builder.markInfeasible(true);
}
// Process the false branch.
if (builder.isFeasible(false)) {
- if (StFalse)
+ if (StFalse) {
+ StFalse = SaveConditionValue(StFalse, BldCtx.LC, Condition, Term, false);
builder.generateNode(StFalse, false, PredI);
- else
+ } else
builder.markInfeasible(false);
}
}
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -135,9 +135,7 @@
if (i != 5)
return;
if (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) {
- // FIXME: Should be no-warning, because the noreturn destructor should
- // fire on all paths.
- clang_analyzer_eval(true); // expected-warning{{TRUE}}
+ clang_analyzer_eval(true); // no warning
}
}
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1259.2.patch
Type: text/x-patch
Size: 4239 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130802/fed5cf73/attachment.bin>
More information about the cfe-commits
mailing list