[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC334682: [analyzer] Track class member initializer
constructors path-sensitively. (authored by dergachev, committed by ).
Herald added a subscriber: mikhail.ramalho.
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: 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 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 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 getObjectUnderConstruction(ProgramStateRef State,
- const Stmt *S,
- const LocationContext *LC);
+ static Optional getObjectUnderConstruction(
+ ProgramStateRef State,
+ llvm::PointerUnion P,
+ const LocationContext *LC);
/// Check if all objects under construction have been fully constructed
/// for the given context range (including FromLC, not including ToLC).
Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -21,6 +21,37 @@
} // 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;
+// FIXME: Should be TRUE in C++11 as well.
+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 struct AddressVector {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -109,7 +109,75 @@
// to the object's location, so that on every such statement the location
// could have been retrieved.
-typedef std::pair 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 {
+ typedef std::pair<
+ llvm::PointerUnion,
+ const LocationContext *> ConstructedObjectKeyImpl;
+
+ ConstructedObjectKeyImpl Impl;
+
+ const void *getAnyASTNodePtr() const {
+if (const Stmt *S = getStmt())
+ return S;
+else
+ return getCXXCtorInitializer();
+ }
+
+public:
+ ConstructedObjectKey(
+ llvm::PointerUnion P,
+ const LocationContext *LC)
+ : Impl(P, 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(getStmt()) ||
+ isa(getStmt()) || isa(getStmt()) ||
+ isa(getStmt()));
+ }
+
+ const Stmt *getStmt() const {
+return Impl.first.dyn_cast();
+ }
+
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+return Impl.first.dyn_cast();
+ }
+
+ const LocationContext *getLocationContext() const {
+return Impl.second;
+ }
+
+ void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &P
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
george.karpenkov added a comment. 👍 https://reviews.llvm.org/D47350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
NoQ updated this revision to Diff 149337.
NoQ added a comment.
Hmm, actually composition looks very pretty if you use the magic word "impl".
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,37 @@
} // 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;
+// FIXME: Should be TRUE in C++11 as well.
+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 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() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
- }
-
- if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(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
@@ -109,7 +109,75 @@
// to the object's location, so that on every such statement the location
// could have been retrieved.
-typedef std::pair 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 {
+ typedef std::pair<
+ llvm::PointerUnion,
+ const LocationContext *> ConstructedObjectKeyImpl;
+
+ ConstructedObjectKeyImpl Impl;
+
+ const void *getAnyASTNodePtr() const {
+if (const Stmt *S = getStmt())
+ return S;
+else
+ return getCXXCtorInitializer();
+ }
+
+public:
+ ConstructedObjectKey(
+ llvm::PointerUnion P,
+ const LocationContext *LC)
+ : Impl(P, 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(getStmt()) ||
+ isa(getStmt()) || isa(getStmt()) ||
+ isa(getStmt()));
+ }
+
+ const Stmt *getStmt() const {
+return Impl.first.dyn_cast();
+ }
+
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+return Impl.first.dyn_cast();
+ }
+
+ const LocationContext *getLocationContext() const {
+return Impl.second;
+ }
+
+ void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) {
+OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") ";
+if
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
NoQ added a comment.
Refactor everything to address review comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+/// AST nodes.
+class ConstructedObjectKey : public ConstructedObjectKeyImpl {
+ const void *getAnyASTNodePtr() const {
george.karpenkov wrote:
> sorry I find that very confusing, and I think the previous version was better
> (prefer composition over inheritance)
Ok, so, i mean, i don't think it would be faster, i really hope the compiler
would know how to inline `std::pair` methods. I think converting to a
`std::pair` or `std::tuple` is a fairly standard trick to avoid `x < rhs.x ||
(x == rhs.x && y < rhs.y)` kind of code. But with inheritance we at least have
boilerplate operators sorted out for free.
May i leave the actual previous version around without fixing it?^^
https://reviews.llvm.org/D47350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.
LGTM, but I would really prefer if you could split that inheritance back into
composition.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+/// AST nodes.
+class ConstructedObjectKey : public ConstructedObjectKeyImpl {
+ const void *getAnyASTNodePtr() const {
sorry I find that very confusing, and I think the previous version was better
(prefer composition over inheritance)
https://reviews.llvm.org/D47350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
NoQ updated this revision to Diff 149205.
NoQ marked 3 inline comments as done.
NoQ added a comment.
Refactor everything to address review comments.
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,37 @@
} // 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;
+// FIXME: Should be TRUE in C++11 as well.
+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 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() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
- }
-
- if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(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
@@ -109,7 +109,65 @@
// to the object's location, so that on every such statement the location
// could have been retrieved.
-typedef std::pair ConstructedObjectKey;
+typedef std::pair,
+ const LocationContext *>
+ConstructedObjectKeyImpl;
+
+/// 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 : public ConstructedObjectKeyImpl {
+ const void *getAnyASTNodePtr() const {
+if (const Stmt *S = getStmt())
+ return S;
+else
+ return getCXXCtorInitializer();
+ }
+
+public:
+ ConstructedObjectKey(
+ llvm::PointerUnion P,
+ const LocationContext *LC)
+ : ConstructedObjectKeyImpl(P, 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(getStmt()) ||
+ isa(getStmt()) || isa(getStmt()) ||
+ isa(getStmt()));
+ }
+
+ const Stmt *getStmt() const {
+return first.dyn_cast();
+ }
+
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+return first.dyn_cast();
+ }
+
+ const LocationContext *getLocationContext() const {
+return second;
+ }
+
+ void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) {
+OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") ";
+if (
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
requesting changes due to minor nits inline
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:152
+}
+ }
+ void Profile(llvm::FoldingSetNodeID &ID) const {
newline after the brace. here and elsewhere within this class.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:161
+ bool operator<(const ConstructedObjectKey &Other) const {
+return std::make_pair(P, LC) < std::make_pair(Other.P, Other.LC);
+ }
would it be faster to store a pair in the first place, instead of constructing
it on each comparison? Is this operator required to be in a GDM?
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:756
SVal FieldLoc;
+ bool ObjectUnderConstructionNeedsCleanup = false;
gotta love state variables used 70 lines later on. Could we add a comment here?
What is the semantics of the cleanup action? Running destructors? Maybe a
better name would make this more readable?
Also would it be worse to change `finishObjectConstruction` function to not do
anything when there's no object being constructed, and then we could simply
call it on all code paths?
https://reviews.llvm.org/D47350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
NoQ updated this revision to Diff 148502.
NoQ added a comment.
Add a forgotten FIXME to the test.
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,37 @@
} // 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;
+// FIXME: Should be TRUE in C++11 as well.
+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 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() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
- }
-
- if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(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 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 P;
+ const LocationContext *LC;
+
+ const void *getAnyASTNodePtr() const {
+if (const Stmt *S = getStmt())
+ return S;
+else
+ return getCXXCtorInitializer();
+ }
+
+public:
+ ConstructedObjectKey(
+ llvm::PointerUnion 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(getStmt()) ||
+ isa(getStmt()) || isa(getStmt()) ||
+ isa(getStmt()));
+ }
+
+ const Stmt *getStmt() const {
+return P.dyn_cast();
+ }
+ const CXXCtorInitializer *getCXXCtorInitializer() const {
+return P.dyn_cast();
+ }
+ 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()) {
+ S->printPretty(OS, Helper, PP);
+} else {
+ const CXXCtorInitializer *I = P.get();
+ OS << I->getAnyMember()->getNameAsString();
+
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
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 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() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
- }
-
- if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(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 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
