NoQ updated this revision to Diff 133956.
NoQ added a comment.
- Test `const C &c = coin ? C(x, y) : C(z, w);`.
- Fix comments surrounding the assertion.
https://reviews.llvm.org/D43104
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/CallEvent.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -779,3 +779,116 @@
}
} // namespace test_temporary_object_expr
+
+namespace test_match_constructors_and_destructors {
+class C {
+public:
+ int &x, &y;
+ C(int &_x, int &_y) : x(_x), y(_y) { ++x; }
+ C(const C &c): x(c.x), y(c.y) { ++x; }
+ ~C() { ++y; }
+};
+
+void test_simple_temporary() {
+ int x = 0, y = 0;
+ {
+ const C &c = C(x, y);
+ }
+ // One constructor and one destructor.
+ clang_analyzer_eval(x == 1);
+ clang_analyzer_eval(y == 1);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+}
+
+void test_simple_temporary_with_copy() {
+ int x = 0, y = 0;
+ {
+ C c = C(x, y);
+ }
+ // Two constructors (temporary object expr and copy) and two destructors.
+ clang_analyzer_eval(x == 2);
+ clang_analyzer_eval(y == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+}
+
+void test_ternary_temporary(int coin) {
+ int x = 0, y = 0, z = 0, w = 0;
+ {
+ const C &c = coin ? C(x, y) : C(z, w);
+ }
+ // This time each branch contains an additional elidable copy constructor.
+ if (coin) {
+ clang_analyzer_eval(x == 2);
+ clang_analyzer_eval(y == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+ clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w == 0); // expected-warning{{TRUE}}
+
+ } else {
+ clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(z == 2);
+ clang_analyzer_eval(w == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+ }
+}
+
+void test_ternary_temporary_with_copy(int coin) {
+ int x = 0, y = 0, z = 0, w = 0;
+ {
+ C c = coin ? C(x, y) : C(z, w);
+ }
+ // Temporary expression, elidable copy within branch,
+ // constructor for variable - 3 total.
+ if (coin) {
+ clang_analyzer_eval(x == 3);
+ clang_analyzer_eval(y == 3);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+ clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w == 0); // expected-warning{{TRUE}}
+
+ } else {
+ clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(z == 3);
+ clang_analyzer_eval(w == 3);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+ }
+}
+} // namespace test_match_constructors_and_destructors
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -332,6 +332,8 @@
// See if we have any stale C++ allocator values.
assert(areCXXNewAllocatorValuesClear(CEEState, calleeCtx, callerCtx));
+ // See if we have any stale C++ temporary object values.
+ assert(areInitializedTemporariesClear(CEEState, calleeCtx, callerCtx));
ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew);
CEENode->addPredecessor(*I, G);
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -102,14 +102,15 @@
const MemRegion *
ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
ExplodedNode *Pred,
+ const ConstructionContext *CC,
EvalCallOptions &CallOpts) {
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
// See if we're constructing an existing region by looking at the
// current construction context.
- if (auto CC = getCurrentCFGElement().getAs<CFGConstructor>()) {
+ if (CC) {
if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(TriggerStmt)) {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
@@ -221,10 +222,20 @@
// the entire array).
EvalCallOptions CallOpts;
+ auto C = getCurrentCFGElement().getAs<CFGConstructor>();
+ const ConstructionContext *CC = C ? C->getConstructionContext() : nullptr;
+
+ const CXXBindTemporaryExpr *BTE = nullptr;
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
- Target = getRegionForConstructedObject(CE, Pred, CallOpts);
+ Target = getRegionForConstructedObject(CE, Pred, CC, CallOpts);
+ if (CC && AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG() &&
+ !CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion &&
+ CallOpts.IsTemporaryCtorOrDtor) {
+ // May as well be a ReturnStmt.
+ BTE = dyn_cast<CXXBindTemporaryExpr>(CC->getTriggerStmt());
+ }
break;
}
case CXXConstructExpr::CK_VirtualBase:
@@ -292,17 +303,18 @@
ExplodedNodeSet DstPreVisit;
getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this);
+ // FIXME: Is it possible and/or useful to do this before PreStmt?
ExplodedNodeSet PreInitialized;
{
StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx);
- if (CE->requiresZeroInitialization()) {
- // Type of the zero doesn't matter.
- SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
-
- for (ExplodedNodeSet::iterator I = DstPreVisit.begin(),
- E = DstPreVisit.end();
- I != E; ++I) {
- ProgramStateRef State = (*I)->getState();
+ for (ExplodedNodeSet::iterator I = DstPreVisit.begin(),
+ E = DstPreVisit.end();
+ I != E; ++I) {
+ ProgramStateRef State = (*I)->getState();
+ if (CE->requiresZeroInitialization()) {
+ // Type of the zero doesn't matter.
+ SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
+
// FIXME: Once we properly handle constructors in new-expressions, we'll
// need to invalidate the region before setting a default value, to make
// sure there aren't any lingering bindings around. This probably needs
@@ -316,9 +328,15 @@
// since it's then possible to be initializing one part of a multi-
// dimensional array.
State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal, LCtx);
- Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
- ProgramPoint::PreStmtKind);
}
+
+ if (BTE) {
+ State = addInitializedTemporary(State, BTE, LCtx,
+ cast<CXXTempObjectRegion>(Target));
+ }
+
+ Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
+ ProgramPoint::PreStmtKind);
}
}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -54,18 +54,21 @@
STATISTIC(NumTimesRetriedWithoutInlining,
"The # of times we re-evaluated a call without inlining");
-typedef std::pair<const CXXBindTemporaryExpr *, const StackFrameContext *>
- CXXBindTemporaryContext;
+typedef llvm::ImmutableMap<std::pair<const CXXBindTemporaryExpr *,
+ const StackFrameContext *>,
+ const CXXTempObjectRegion *>
+ InitializedTemporariesMap;
// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated.
// The StackFrameContext assures that nested calls due to inlined recursive
// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporariesSet,
- llvm::ImmutableSet<CXXBindTemporaryContext>)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
+ InitializedTemporariesMap)
typedef llvm::ImmutableMap<std::pair<const CXXNewExpr *,
- const LocationContext *>, SVal>
- CXXNewAllocatorValuesMap;
+ const LocationContext *>,
+ SVal>
+ CXXNewAllocatorValuesMap;
// Keeps track of return values of various operator new() calls between
// evaluation of the inlined operator new(), through the constructor call,
@@ -323,6 +326,36 @@
return State;
}
+ProgramStateRef ExprEngine::addInitializedTemporary(
+ ProgramStateRef State, const CXXBindTemporaryExpr *BTE,
+ const LocationContext *LC, const CXXTempObjectRegion *R) {
+ const auto &Key = std::make_pair(BTE, LC->getCurrentStackFrame());
+ if (!State->contains<InitializedTemporaries>(Key)) {
+ return State->set<InitializedTemporaries>(Key, R);
+ }
+
+ // FIXME: Currently the state might already contain the marker due to
+ // incorrect handling of temporaries bound to default parameters; for
+ // those, we currently skip the CXXBindTemporaryExpr but rely on adding
+ // temporary destructor nodes. Otherwise, this branch should be unreachable.
+ return State;
+}
+
+bool ExprEngine::areInitializedTemporariesClear(ProgramStateRef State,
+ const LocationContext *FromLC,
+ const LocationContext *ToLC) {
+ const LocationContext *LC = FromLC;
+ do {
+ for (auto I : State->get<InitializedTemporaries>())
+ if (I.first.second == LC)
+ return false;
+
+ LC = LC->getParent();
+ assert(LC && "ToLC must be a parent of FromLC!");
+ } while (LC != ToLC);
+ return true;
+}
+
ProgramStateRef
ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State,
const CXXNewExpr *CNE,
@@ -390,11 +423,16 @@
const LocationContext *LC) {
PrintingPolicy PP =
LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
- for (auto I : State->get<InitializedTemporariesSet>()) {
- if (I.second != LC)
+ for (auto I : State->get<InitializedTemporaries>()) {
+ std::pair<const CXXBindTemporaryExpr *, const LocationContext *> Key =
+ I.first;
+ const MemRegion *Value = I.second;
+ if (Key.second != LC)
continue;
- Out << '(' << I.second << ',' << I.first << ") ";
- I.first->printPretty(Out, nullptr, PP);
+ Out << '(' << Key.second << ',' << Key.first << ") ";
+ Key.first->printPretty(Out, nullptr, PP);
+ if (Value)
+ Out << " : " << Value;
Out << NL;
}
}
@@ -422,7 +460,7 @@
const char *NL, const char *Sep,
const LocationContext *LCtx) {
if (LCtx) {
- if (!State->get<InitializedTemporariesSet>().isEmpty()) {
+ if (!State->get<InitializedTemporaries>().isEmpty()) {
Out << Sep << "Initialized temporaries:" << NL;
LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
@@ -536,6 +574,10 @@
const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr;
SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager());
+ for (auto I : CleanedState->get<InitializedTemporaries>())
+ if (I.second)
+ SymReaper.markLive(I.second);
+
for (auto I : CleanedState->get<CXXNewAllocatorValues>()) {
if (SymbolRef Sym = I.second.getAsSymbol())
SymReaper.markLive(Sym);
@@ -900,12 +942,18 @@
ExplodedNodeSet CleanDtorState;
StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx);
ProgramStateRef State = Pred->getState();
- if (State->contains<InitializedTemporariesSet>(
- std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()))) {
+ const MemRegion *MR = nullptr;
+ if (const CXXTempObjectRegion *const *MRPtr =
+ State->get<InitializedTemporaries>(std::make_pair(
+ D.getBindTemporaryExpr(), Pred->getStackFrame()))) {
// FIXME: Currently we insert temporary destructors for default parameters,
- // but we don't insert the constructors.
- State = State->remove<InitializedTemporariesSet>(
+ // but we don't insert the constructors, so the entry in
+ // InitializedTemporaries may be missing.
+ State = State->remove<InitializedTemporaries>(
std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()));
+ // *MRPtr may still be null when the construction context for the temporary
+ // was not implemented.
+ MR = *MRPtr;
}
StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
@@ -916,12 +964,11 @@
ExplodedNode *CleanPred =
CleanDtorState.empty() ? Pred : *CleanDtorState.begin();
- // 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.
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
- CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
- VisitCXXDestructor(varType, nullptr, D.getBindTemporaryExpr(),
+ if (!MR)
+ CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+ VisitCXXDestructor(varType, MR, D.getBindTemporaryExpr(),
/*IsBase=*/false, CleanPred, Dst, CallOpts);
}
@@ -932,7 +979,7 @@
const CFGBlock *DstT,
const CFGBlock *DstF) {
BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF);
- if (Pred->getState()->contains<InitializedTemporariesSet>(
+ if (Pred->getState()->contains<InitializedTemporaries>(
std::make_pair(BTE, Pred->getStackFrame()))) {
TempDtorBuilder.markInfeasible(false);
TempDtorBuilder.generateNode(Pred->getState(), true, Pred);
@@ -942,32 +989,6 @@
}
}
-void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
- ExplodedNodeSet &PreVisit,
- ExplodedNodeSet &Dst) {
- if (!getAnalysisManager().options.includeTemporaryDtorsInCFG()) {
- // In case we don't have temporary destructors in the CFG, do not mark
- // the initialization - we would otherwise never clean it up.
- Dst = PreVisit;
- return;
- }
- StmtNodeBuilder StmtBldr(PreVisit, Dst, *currBldrCtx);
- for (ExplodedNode *Node : PreVisit) {
- ProgramStateRef State = Node->getState();
-
- if (!State->contains<InitializedTemporariesSet>(
- std::make_pair(BTE, Node->getStackFrame()))) {
- // FIXME: Currently the state might already contain the marker due to
- // incorrect handling of temporaries bound to default parameters; for
- // those, we currently skip the CXXBindTemporaryExpr but rely on adding
- // temporary destructor nodes.
- State = State->add<InitializedTemporariesSet>(
- std::make_pair(BTE, Node->getStackFrame()));
- }
- StmtBldr.generateNode(BTE, Node, State);
- }
-}
-
namespace {
class CollectReachableSymbolsCallback final : public SymbolVisitor {
InvalidatedSymbols Symbols;
@@ -1130,9 +1151,7 @@
Bldr.takeNodes(Pred);
ExplodedNodeSet PreVisit;
getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
- ExplodedNodeSet Next;
- VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), PreVisit, Next);
- getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this);
+ getCheckerManager().runCheckersForPostStmt(Dst, PreVisit, S, *this);
Bldr.addNodes(Dst);
break;
}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1197,9 +1197,8 @@
// destructors, though this could change in the future.
const CFGBlock *B = CalleeCtx->getCallSiteBlock();
CFGElement E = (*B)[CalleeCtx->getIndex()];
- assert(E.getAs<CFGImplicitDtor>() &&
+ assert(E.getAs<CFGImplicitDtor>() || E.getAs<CFGTemporaryDtor>() &&
"All other CFG elements should have exprs");
- assert(!E.getAs<CFGTemporaryDtor>() && "We don't handle temporaries yet");
SValBuilder &SVB = State->getStateManager().getSValBuilder();
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CalleeCtx->getDecl());
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -448,10 +448,6 @@
ExplodedNode *Pred,
ExplodedNodeSet &Dst);
- void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
- ExplodedNodeSet &PreVisit,
- ExplodedNodeSet &Dst);
-
void VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred,
ExplodedNodeSet &Dst);
@@ -696,8 +692,22 @@
/// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
ExplodedNode *Pred,
+ const ConstructionContext *CC,
EvalCallOptions &CallOpts);
+ /// Store the region of a C++ temporary object corresponding to a
+ /// CXXBindTemporaryExpr for later destruction.
+ static ProgramStateRef addInitializedTemporary(
+ ProgramStateRef State, const CXXBindTemporaryExpr *BTE,
+ const LocationContext *LC, const CXXTempObjectRegion *R);
+
+ /// Check if all initialized temporary regions are clear for the given
+ /// context range (including FromLC, not including ToLC).
+ /// This is useful for assertions.
+ static bool areInitializedTemporariesClear(ProgramStateRef State,
+ const LocationContext *FromLC,
+ const LocationContext *ToLC);
+
/// Store the region returned by operator new() so that the constructor
/// that follows it knew what location to initialize. The value should be
/// cleared once the respective CXXNewExpr CFGStmt element is processed.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
