Final version of the patch. I consider this ready for submission now.
Hi jordan_rose,
http://llvm-reviews.chandlerc.com/D1259
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1259?vs=3148&id=3317#toc
Files:
lib/Analysis/CFG.cpp
lib/Analysis/LiveVariables.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/analyzer-config.c
test/Analysis/analyzer-config.cpp
test/Analysis/dtor.cpp
test/Analysis/temp-obj-dtors-cfg-output.cpp
test/Analysis/temporaries.cpp
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -3749,8 +3749,9 @@
} else if (Optional<CFGTemporaryDtor> TE = E.getAs<CFGTemporaryDtor>()) {
const CXXBindTemporaryExpr *BT = TE->getBindTemporaryExpr();
- OS << "~" << BT->getType()->getAsCXXRecordDecl()->getName() << "()";
- OS << " (Temporary object destructor)\n";
+ OS << "~";
+ BT->getType().print(OS, PrintingPolicy(Helper->getLangOpts()));
+ OS << "() (Temporary object destructor)\n";
}
}
Index: lib/Analysis/LiveVariables.cpp
===================================================================
--- lib/Analysis/LiveVariables.cpp
+++ lib/Analysis/LiveVariables.cpp
@@ -212,6 +212,8 @@
LiveVariables::LivenessValues &val;
LiveVariables::Observer *observer;
const CFGBlock *currentBlock;
+
+ void MarkLogicalOpsLive(const Expr *E);
public:
TransferFunctions(LiveVariablesImpl &im,
LiveVariables::LivenessValues &Val,
@@ -368,9 +370,28 @@
if (observer)
observer->observerKill(DR);
}
+ } else {
+ // All logical sub-operations are live until we reach the outermost
+ // operator. Static analyzer relies on this behaviour.
+ MarkLogicalOpsLive(B);
}
}
+void TransferFunctions::MarkLogicalOpsLive(const Expr *E) {
+ const BinaryOperator *BO = dyn_cast<BinaryOperator>(E);
+ if (!BO || !BO->isLogicalOp())
+ return;
+
+ const Expr *LHS = BO->getLHS()->IgnoreParens();
+ const Expr *RHS = BO->getRHS()->IgnoreParens();
+
+ val.liveStmts = LV.SSetFact.add(val.liveStmts, LHS);
+ val.liveStmts = LV.SSetFact.add(val.liveStmts, RHS);
+
+ MarkLogicalOpsLive(LHS);
+ MarkLogicalOpsLive(RHS);
+}
+
void TransferFunctions::VisitBlockExpr(BlockExpr *BE) {
AnalysisDeclContext::referenced_decls_iterator I, E;
llvm::tie(I, E) =
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -119,7 +119,7 @@
bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
return getBooleanOption(IncludeTemporaryDtorsInCFG,
"cfg-temporary-dtors",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::mayInlineCXXStandardLibrary() {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -601,7 +601,15 @@
void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {}
+ ExplodedNodeSet &Dst) {
+
+ QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
+
+ // FIXME: Inlining of temporary destructors is not supported yet anyway, so we
+ // just put a NULL region for now. This will need to be changed later.
+ VisitCXXDestructor(varType, NULL, D.getBindTemporaryExpr(),
+ /*IsBase=*/ false, Pred, Dst);
+}
void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet &DstTop) {
@@ -1343,12 +1351,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 '&&'.
+ Condition = ResolveCondition(Condition, BldCtx.getBlock());
+ }
+
PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
Condition->getLocStart(),
"Error evaluating branch");
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -501,72 +501,64 @@
}
}
+static ProgramStateRef EvaluateLogicalExpression(const Expr *E,
+ const LocationContext *LC,
+ ProgramStateRef State) {
+ SVal X = State->getSVal(E, LC);
+ if (! X.isUnknown())
+ return State;
+
+ const BinaryOperator *B = cast_or_null<BinaryOperator>(E->IgnoreParens());
+ if (!B || (B->getOpcode() != BO_LAnd && B->getOpcode() != BO_LOr))
+ return State;
+
+ State = EvaluateLogicalExpression(B->getLHS(), LC, State);
+ X = State->getSVal(B->getLHS(), LC);
+ QualType XType = B->getLHS()->getType();
+ assert(!X.isUnknownOrUndef() && "Value should have already been computed.");
+
+ ProgramStateRef StTrue, StFalse;
+ llvm::tie(StTrue, StFalse) = State->assume(X.castAs<DefinedOrUnknownSVal>());
+
+ assert(!StTrue != !StFalse && "Value should be evaluate to true or false.");
+ if(!StFalse == (B->getOpcode() == BO_LAnd)) {
+ // LHS not sufficient, we need to check RHS as well
+ State = EvaluateLogicalExpression(B->getRHS(), LC, State);
+ X = State->getSVal(B->getRHS(), LC);
+ XType = B->getRHS()->getType();
+ }
+
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+ return State->BindExpr(E, LC, SVB.evalCast(X, B->getType(), XType));
+}
+
void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
assert(B->getOpcode() == BO_LAnd ||
B->getOpcode() == BO_LOr);
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
ProgramStateRef state = Pred->getState();
- ExplodedNode *N = Pred;
- while (!N->getLocation().getAs<BlockEntrance>()) {
- ProgramPoint P = N->getLocation();
- assert(P.getAs<PreStmt>()|| P.getAs<PreStmtPurgeDeadSymbols>());
- (void) P;
- assert(N->pred_size() == 1);
- N = *N->pred_begin();
- }
- assert(N->pred_size() == 1);
- N = *N->pred_begin();
- BlockEdge BE = N->getLocation().castAs<BlockEdge>();
- SVal X;
-
- // Determine the value of the expression by introspecting how we
- // got this location in the CFG. This requires looking at the previous
- // block we were in and what kind of control-flow transfer was involved.
- const CFGBlock *SrcBlock = BE.getSrc();
- // The only terminator (if there is one) that makes sense is a logical op.
- CFGTerminator T = SrcBlock->getTerminator();
- if (const BinaryOperator *Term = cast_or_null<BinaryOperator>(T.getStmt())) {
- (void) Term;
- assert(Term->isLogicalOp());
- assert(SrcBlock->succ_size() == 2);
- // Did we take the true or false branch?
- unsigned constant = (*SrcBlock->succ_begin() == BE.getDst()) ? 1 : 0;
- X = svalBuilder.makeIntVal(constant, B->getType());
- }
- else {
- // If there is no terminator, by construction the last statement
- // in SrcBlock is the value of the enclosing expression.
- // However, we still need to constrain that value to be 0 or 1.
- assert(!SrcBlock->empty());
- CFGStmt Elem = SrcBlock->rbegin()->castAs<CFGStmt>();
- const Expr *RHS = cast<Expr>(Elem.getStmt());
- SVal RHSVal = N->getState()->getSVal(RHS, Pred->getLocationContext());
-
- if (RHSVal.isUndef()) {
- X = RHSVal;
- } else {
- DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs<DefinedOrUnknownSVal>();
- ProgramStateRef StTrue, StFalse;
- llvm::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS);
- if (StTrue) {
- if (StFalse) {
- // We can't constrain the value to 0 or 1.
- // The best we can do is a cast.
- X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
- } else {
- // The value is known to be true.
- X = getSValBuilder().makeIntVal(1, B->getType());
- }
- } else {
- // The value is known to be false.
- assert(StFalse && "Infeasible path!");
- X = getSValBuilder().makeIntVal(0, B->getType());
+ state = EvaluateLogicalExpression(B, Pred->getLocationContext(), state);
+ SVal X = state->getSVal(B, Pred->getLocationContext());
+
+ if (!X.isUndef()) {
+ DefinedOrUnknownSVal DefinedRHS = X.castAs<DefinedOrUnknownSVal>();
+ ProgramStateRef StTrue, StFalse;
+ llvm::tie(StTrue, StFalse) = state->assume(DefinedRHS);
+ if (StTrue) {
+ if (!StFalse) {
+ // The value is known to be true.
+ X = getSValBuilder().makeIntVal(1, B->getType());
}
+ } else {
+ // The value is known to be false.
+ assert(StFalse && "Infeasible path!");
+ X = getSValBuilder().makeIntVal(0, B->getType());
}
}
+
Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), X));
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -96,7 +96,11 @@
while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) {
Ty = AT->getElementType();
- LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+ // FIXME: The if test is just a temporary workaround, becase
+ // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
+ // we can properly process temporary destructors.
+ if (LValue.getAsRegion())
+ LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
}
return LValue;
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -807,6 +807,12 @@
AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();
AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
+ // Temporary object destructor processing is currently broken, so we never
+ // inline them.
+ // FIXME: Remove this once temp destructors are working.
+ if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
+ return false;
+
// The auto-synthesized bodies are essential to inline as they are
// usually small and commonly used. Note: we should do this check early on to
// ensure we always inline these calls.
Index: test/Analysis/analyzer-config.c
===================================================================
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -6,7 +6,7 @@
// CHECK: [config]
// CHECK-NEXT: cfg-conditional-static-initializers = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
// CHECK-NEXT: ipa = dynamic-bifurcate
Index: test/Analysis/analyzer-config.cpp
===================================================================
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -17,7 +17,7 @@
// CHECK-NEXT: c++-stdlib-inlining = true
// CHECK-NEXT: c++-template-inlining = true
// CHECK-NEXT: cfg-conditional-static-initializers = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
// CHECK-NEXT: faux-bodies = true
// CHECK-NEXT: graph-trim-interval = 1000
// CHECK-NEXT: ipa = dynamic-bifurcate
Index: test/Analysis/dtor.cpp
===================================================================
--- test/Analysis/dtor.cpp
+++ test/Analysis/dtor.cpp
@@ -416,6 +416,21 @@
f(&x);
*x = 47; // no warning
}
+
+ void g2(int *x) {
+ if (! x) NR();
+ *x = 47; // no warning
+ }
+
+ void f3(int **x) {
+ NR();
+ }
+
+ void g3() {
+ int *x;
+ f3(&x);
+ *x = 47; // no warning
+ }
}
namespace PseudoDtor {
Index: test/Analysis/temp-obj-dtors-cfg-output.cpp
===================================================================
--- test/Analysis/temp-obj-dtors-cfg-output.cpp
+++ test/Analysis/temp-obj-dtors-cfg-output.cpp
@@ -108,6 +108,24 @@
: a(int(A()) + int(B()))
, b() {}
+class NoReturn {
+public:
+ ~NoReturn() __attribute__((noreturn));
+ void f();
+};
+
+void test_noreturn1() {
+ int a;
+ NoReturn().f();
+ int b;
+}
+
+void test_noreturn2() {
+ int a;
+ NoReturn(), 47;
+ int b;
+}
+
// CHECK: [B1 (ENTRY)]
// CHECK: Succs (1): B0
// CHECK: [B0 (EXIT)]
@@ -846,3 +864,36 @@
// CHECK: [B0 (EXIT)]
// CHECK: Preds (1): B1
+// CHECK: [B3 (ENTRY)]
+// CHECK: Succs (1): B2
+// CHECK: [B1]
+// CHECK: 1: int b;
+// CHECK: Succs (1): B0
+// CHECK: [B2]
+// CHECK: 1: int a;
+// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)
+// CHECK: 3: [B2.2] (BindTemporary)
+// CHECK: 4: [B2.3].f
+// CHECK: 5: [B2.4]()
+// CHECK: 6: ~NoReturn() (Temporary object destructor)
+// CHECK: Preds (1): B3
+// CHECK: Succs (1): B0
+// CHECK: [B0 (EXIT)]
+// CHECK: Preds (2): B1 B2
+
+// CHECK: [B3 (ENTRY)]
+// CHECK: Succs (1): B2
+// CHECK: [B1]
+// CHECK: 1: int b;
+// CHECK: Succs (1): B0
+// CHECK: [B2]
+// CHECK: 1: int a;
+// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)
+// CHECK: 3: [B2.2] (BindTemporary)
+// CHECK: 4: 47
+// CHECK: 5: ... , [B2.4]
+// CHECK: 6: ~NoReturn() (Temporary object destructor)
+// CHECK: Preds (1): B3
+// CHECK: Succs (1): B0
+// CHECK: [B0 (EXIT)]
+// CHECK: Preds (2): B1 B2
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
}
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
