NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Same as https://reviews.llvm.org/D47305 but for `CXXCtorInitializer`-based
constructors, based on the discussion in
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html
Because these don't suffer from liveness bugs (member variables are born much
earlier than their constructors are called and live much longer than stack
locals), this is mostly a refactoring pass. It has minor observable side
effects, but it will have way more visible effects when we enable C++17
construction contexts because finding the direct constructor would be much
harder.
Currently the observable effect is that we can preserve direct bindings to the
object more often and we need to fall back to binding the lazy compound value
of the initializer expression less often. Direct bindings are modeled better by
the store. In the provided test case the default binding produced by
trivial-copying `s` to `t.s` would overwrite the existing default binding to
`t`. But if we instead preserve direct bindings, only bindings that correspond
to `t.s` will be overwritten and the binding for `t.w` will remain untouched.
This only works for C++17 because in C++11 the member variable is still modeled
as a trivial copy from the temporary object, because there semantically *is* a
temporary object, while in C++17 it is elided.
Repository:
rC Clang
https://reviews.llvm.org/D47350
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/cxx17-mandatory-elision.cpp
Index: test/Analysis/cxx17-mandatory-elision.cpp
===================================================================
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -21,6 +21,36 @@
} // namespace variable_functional_cast_crash
+namespace ctor_initializer {
+
+struct S {
+ int x, y, z;
+};
+
+struct T {
+ S s;
+ int w;
+ T(int w): s(), w(w) {}
+};
+
+class C {
+ T t;
+public:
+ C() : t(T(4)) {
+ S s = {1, 2, 3};
+ t.s = s;
+ clang_analyzer_eval(t.w == 4);
+#if __cplusplus >= 201703L
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+ }
+};
+
+} // namespace ctor_initializer
+
+
namespace address_vector_tests {
template <typename T> struct AddressVector {
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -153,6 +153,7 @@
QualType Ty = Field->getType();
FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
CallOpts.IsArrayCtorOrDtor);
+ State = addObjectUnderConstruction(State, Init, LCtx, FieldVal);
return std::make_pair(State, FieldVal);
}
case ConstructionContext::NewAllocatedObjectKind: {
@@ -272,35 +273,6 @@
State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
}
-const CXXConstructExpr *
-ExprEngine::findDirectConstructorForCurrentCFGElement() {
- // Go backward in the CFG to see if the previous element (ignoring
- // destructors) was a CXXConstructExpr. If so, that constructor
- // was constructed directly into an existing region.
- // This process is essentially the inverse of that performed in
- // findElementDirectlyInitializedByCurrentConstructor().
- if (currStmtIdx == 0)
- return nullptr;
-
- const CFGBlock *B = getBuilderContext().getBlock();
-
- unsigned int PreviousStmtIdx = currStmtIdx - 1;
- CFGElement Previous = (*B)[PreviousStmtIdx];
-
- while (Previous.getAs<CFGImplicitDtor>() && PreviousStmtIdx > 0) {
- --PreviousStmtIdx;
- Previous = (*B)[PreviousStmtIdx];
- }
-
- if (Optional<CFGStmt> PrevStmtElem = Previous.getAs<CFGStmt>()) {
- if (auto *CtorExpr = dyn_cast<CXXConstructExpr>(PrevStmtElem->getStmt())) {
- return CtorExpr;
- }
- }
-
- return nullptr;
-}
-
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -100,7 +100,68 @@
// Keeps track of whether objects corresponding to the statement have been
// fully constructed.
-typedef std::pair<const Stmt *, const LocationContext *> ConstructedObjectKey;
+
+/// ConstructedObjectKey is used for being able to find the path-sensitive
+/// memory region of a freshly constructed object while modeling the AST node
+/// that syntactically represents the object that is being constructed.
+/// Semantics of such nodes may sometimes require access to the region that's
+/// not otherwise present in the program state, or to the very fact that
+/// the construction context was present and contained references to these
+/// AST nodes.
+class ConstructedObjectKey {
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P;
+ const LocationContext *LC;
+
+ const void *getAnyASTNodePtr() const {
+ if (const Stmt *S = getStmt())
+ return S;
+ else
+ return getCXXCtorInitializer();
+ }
+
+public:
+ ConstructedObjectKey(
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC)
+ : P(P), LC(LC) {
+ // This is the full list of statements that require additional actions when
+ // encountered. This list may be expanded when new actions are implemented.
+ assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) ||
+ isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) ||
+ isa<MaterializeTemporaryExpr>(getStmt()));
+ }
+
+ const Stmt *getStmt() const {
+ return P.dyn_cast<const Stmt *>();
+ }
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+ return P.dyn_cast<const CXXCtorInitializer *>();
+ }
+ const LocationContext *getLocationContext() const {
+ return LC;
+ }
+
+ void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) {
+ OS << '(' << LC << ',' << getAnyASTNodePtr() << ") ";
+ if (const Stmt *S = P.dyn_cast<const Stmt *>()) {
+ S->printPretty(OS, Helper, PP);
+ } else {
+ const CXXCtorInitializer *I = P.get<const CXXCtorInitializer *>();
+ OS << I->getAnyMember()->getNameAsString();
+ }
+ }
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(P.getOpaqueValue());
+ ID.AddPointer(LC);
+ }
+ bool operator==(const ConstructedObjectKey &Other) const {
+ return P == Other.P && LC == Other.LC;
+ }
+ bool operator<(const ConstructedObjectKey &Other) const {
+ return std::make_pair(P, LC) < std::make_pair(Other.P, Other.LC);
+ }
+};
+
typedef llvm::ImmutableMap<ConstructedObjectKey, SVal>
ObjectsUnderConstructionMap;
REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
@@ -369,10 +430,11 @@
return State;
}
-ProgramStateRef
-ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const Stmt *S,
- const LocationContext *LC, SVal V) {
- ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+ProgramStateRef ExprEngine::addObjectUnderConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC, SVal V) {
+ ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
if (!State->contains<ObjectsUnderConstruction>(Key)) {
return State->set<ObjectsUnderConstruction>(Key, V);
}
@@ -384,15 +446,19 @@
return State;
}
-Optional<SVal> ExprEngine::getObjectUnderConstruction(ProgramStateRef State,
- const Stmt *S, const LocationContext *LC) {
- ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+Optional<SVal> ExprEngine::getObjectUnderConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC) {
+ ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key));
}
-ProgramStateRef ExprEngine::finishObjectConstruction(ProgramStateRef State,
- const Stmt *S, const LocationContext *LC) {
- ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+ProgramStateRef ExprEngine::finishObjectConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC) {
+ ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
return State->remove<ObjectsUnderConstruction>(Key);
}
@@ -403,7 +469,7 @@
while (LC != ToLC) {
assert(LC && "ToLC must be a parent of FromLC!");
for (auto I : State->get<ObjectsUnderConstruction>())
- if (I.first.second == LC)
+ if (I.first.getLocationContext() == LC)
return false;
LC = LC->getParent();
@@ -445,10 +511,9 @@
for (auto I : State->get<ObjectsUnderConstruction>()) {
ConstructedObjectKey Key = I.first;
SVal Value = I.second;
- if (Key.second != LC)
+ if (Key.getLocationContext() != LC)
continue;
- Out << '(' << Key.second << ',' << Key.first << ") ";
- Key.first->printPretty(Out, nullptr, PP);
+ Key.print(Out, nullptr, PP);
Out << " : " << Value << NL;
}
}
@@ -688,15 +753,16 @@
ExplodedNodeSet Tmp(Pred);
SVal FieldLoc;
+ bool ObjectUnderConstructionNeedsCleanup = false;
// Evaluate the initializer, if necessary
if (BMI->isAnyMemberInitializer()) {
// Constructors build the object directly in the field,
// but non-objects must be copied in from the initializer.
- if (const auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
- assert(BMI->getInit()->IgnoreImplicit() == CtorExpr);
- (void)CtorExpr;
+ if (getObjectUnderConstruction(State, BMI, Pred->getLocationContext())) {
// The field was directly constructed, so there is no need to bind.
+ // But we still need to cleanup objects under construction.
+ ObjectUnderConstructionNeedsCleanup = true;
} else {
const Expr *Init = BMI->getInit()->IgnoreImplicit();
const ValueDecl *Field;
@@ -749,8 +815,12 @@
PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame);
ExplodedNodeSet Dst;
NodeBuilder Bldr(Tmp, Dst, *currBldrCtx);
- for (const auto I : Tmp)
- Bldr.generateNode(PP, I->getState(), I);
+ for (const auto I : Tmp) {
+ ProgramStateRef State = I->getState();
+ if (ObjectUnderConstructionNeedsCleanup)
+ State = finishObjectConstruction(State, BMI, Pred->getLocationContext());
+ Bldr.generateNode(PP, State, I);
+ }
// Enqueue the new nodes onto the work list.
Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
@@ -2113,11 +2183,11 @@
while (LC != ToLC) {
assert(LC && "ToLC must be a parent of FromLC!");
for (auto I : State->get<ObjectsUnderConstruction>())
- if (I.first.second == LC) {
+ if (I.first.getLocationContext() == LC) {
// The comment above only pardons us for not cleaning up a
// CXXBindTemporaryExpr. If any other statements are found here,
// it must be a separate problem.
- assert(isa<CXXBindTemporaryExpr>(I.first.first));
+ assert(isa<CXXBindTemporaryExpr>(I.first.getStmt()));
State = State->remove<ObjectsUnderConstruction>(I.first);
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -762,20 +762,23 @@
/// made within the constructor alive until its declaration actually
/// goes into scope.
static ProgramStateRef addObjectUnderConstruction(
- ProgramStateRef State, const Stmt *S,
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
const LocationContext *LC, SVal V);
/// Mark the object sa fully constructed, cleaning up the state trait
/// that tracks objects under construction.
- static ProgramStateRef finishObjectConstruction(ProgramStateRef State,
- const Stmt *S,
- const LocationContext *LC);
+ static ProgramStateRef finishObjectConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC);
/// If the given statement corresponds to an object under construction,
/// being part of its construciton context, retrieve that object's location.
- static Optional<SVal> getObjectUnderConstruction(ProgramStateRef State,
- const Stmt *S,
- const LocationContext *LC);
+ static Optional<SVal> getObjectUnderConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ const LocationContext *LC);
/// Check if all objects under construction have been fully constructed
/// for the given context range (including FromLC, not including ToLC).
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
