baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: dcoughlin.
baloghadamsoftware added subscribers: cfe-commits, o.gyorgy, xazax.hun.
Many classes (e.g. in common stl implementations) contain user-written copy and
move constructors that are identical to the implicit ones. This far ExprEngine
could only handle the official "trivial" case. This patch extends ExprEngine
for copy and move constructors that are user provided but in all other aspects
they are the same as trivial ones.
A typical example is the GNU implementation of the iterator of std::deque,
where the user provided copy constructor of the non-const version also accepts
an instance of the const version as parameter. This patch allows correct
simulation of this iterator.
https://reviews.llvm.org/D22374
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/ctor.mm
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -144,11 +144,11 @@
NonPOD() {}
NonPOD(const NonPOD &Other)
- : x(Other.x), y(Other.y) // expected-warning {{undefined}}
+ : x(Other.x), y(Other.y) // no-warning
{
}
NonPOD(NonPOD &&Other)
- : x(Other.x), y(Other.y) // expected-warning {{undefined}}
+ : x(Other.x), y(Other.y) // no-warning
{
}
@@ -174,11 +174,11 @@
Inner() {}
Inner(const Inner &Other)
- : x(Other.x), y(Other.y) // expected-warning {{undefined}}
+ : x(Other.x), y(Other.y) // no-warning
{
}
Inner(Inner &&Other)
- : x(Other.x), y(Other.y) // expected-warning {{undefined}}
+ : x(Other.x), y(Other.y) // no-warning
{
}
@@ -224,25 +224,29 @@
void testNonPOD() {
NonPOD p;
p.x = 1;
- NonPOD p2 = p;
+ NonPOD p2 = p; // no-warning
+ clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
}
void testNonPODMove() {
NonPOD p;
p.x = 1;
- NonPOD p2 = move(p);
+ NonPOD p2 = move(p); // no-warning
+ clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
}
void testNonPODWrapper() {
NonPODWrapper w;
w.p.y = 1;
- NonPODWrapper w2 = w;
+ NonPODWrapper w2 = w; // no-warning
+ clang_analyzer_eval(w2.p.y == 1); // expected-warning{{TRUE}}
}
void testNonPODWrapperMove() {
NonPODWrapper w;
w.p.y = 1;
- NonPODWrapper w2 = move(w);
+ NonPODWrapper w2 = move(w); // no-warning
+ clang_analyzer_eval(w2.p.y == 1); // expected-warning{{TRUE}}
}
// Not strictly about constructors, but trivial assignment operators should
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -34,19 +34,132 @@
Bldr.generateNode(ME, Pred, state);
}
+bool isCopyOrMoveLike(const CXXConstructorDecl *Constr) {
+ if (Constr->isCopyOrMoveConstructor())
+ return true;
+
+ if(Constr->getNumParams() != 1)
+ return false;
+
+ const auto ParamType =
+ Constr->getParamDecl(0)->getType()->getUnqualifiedDesugaredType();
+ if(!ParamType->isReferenceType())
+ return false;
+
+ const auto ParamPointeeType =
+ ParamType->getAs<ReferenceType>()->getPointeeType();
+ if(!ParamPointeeType->isRecordType())
+ return false;
+
+ const auto *ParamRecDecl = ParamPointeeType->getAs<RecordType>()->getDecl();
+ const auto *ThisRecDecl = Constr->getParent();
+
+ if(ParamRecDecl!=ThisRecDecl)
+ return false;
+
+ return true;
+}
+
+bool isAlmostTrivial(const CXXMethodDecl *Met) {
+ if (Met->isTrivial())
+ return true;
+
+ if(!Met->hasTrivialBody())
+ return false;
+
+ if(Met->getNumParams() != 1)
+ return false;
+
+ const auto *Param = Met->getParamDecl(0);
+ const auto *ThisRecDecl = Met->getParent();
+
+ const auto *Constr = dyn_cast<CXXConstructorDecl>(Met);
+ if(!Constr)
+ return false;
+
+ if(ThisRecDecl->getNumVBases()>0)
+ return false;
+
+ for(const auto Base: ThisRecDecl->bases()) {
+ if(Base.getType()->getAsCXXRecordDecl()->field_empty())
+ continue;
+ for(const auto *Initzer: Constr->inits()) {
+ if(Initzer->isBaseInitializer() &&
+ Initzer->getBaseClass() == &*Base.getType()) {
+ if(const auto *CtrCall = dyn_cast<CXXConstructExpr>(Initzer->getInit()->IgnoreParenImpCasts())) {
+ if(!isCopyOrMoveLike(CtrCall->getConstructor()) ||
+ !isAlmostTrivial(CtrCall->getConstructor()))
+ return false;
+ if(const auto *Init = dyn_cast<DeclRefExpr>(CtrCall->getArg(0)->IgnoreParenImpCasts())) {
+ if(Init->getDecl() != Param)
+ return false;
+ } else {
+ return false;
+ }
+ } else {
+ return false;
+ }
+ break;
+ }
+ }
+ }
+
+ for(const auto *Field: ThisRecDecl->fields()) {
+ if(!Field->getType()->isScalarType() &&
+ !Field->getType()->isRecordType())
+ return false;
+ bool found = false;
+ for(const auto *Initzer: Constr->inits()) {
+ if(Initzer->isMemberInitializer() && Initzer->getMember() == Field) {
+ found = true;
+ const MemberExpr *InitMem;
+ if(Field->getType()->isScalarType()) {
+ InitMem = dyn_cast<MemberExpr>(Initzer->getInit()->IgnoreParenImpCasts());
+ } else {
+ if(const auto *CtrCall = dyn_cast<CXXConstructExpr>(Initzer->getInit()->IgnoreParenImpCasts())) {
+ if(!isCopyOrMoveLike(CtrCall->getConstructor()) ||
+ !isAlmostTrivial(CtrCall->getConstructor()))
+ return false;
+ InitMem = dyn_cast<MemberExpr>(CtrCall->getArg(0)->IgnoreParenImpCasts());
+ } else {
+ return false;
+ }
+ }
+ if(InitMem) {
+ if(InitMem->getMemberDecl() != Field)
+ return false;
+ if(const auto *Base = dyn_cast<DeclRefExpr>(InitMem->getBase())) {
+ if(Base->getDecl() != Param)
+ return false;
+ } else {
+ return false;
+ }
+ } else {
+ return false;
+ }
+ break;
+ }
+ }
+ if(!found)
+ return false;
+ }
+
+ return true;
+}
+
// FIXME: This is the sort of code that should eventually live in a Core
// checker rather than as a special case in ExprEngine.
void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
const CallEvent &Call) {
SVal ThisVal;
bool AlwaysReturnsLValue;
if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
- assert(Ctor->getDecl()->isTrivial());
- assert(Ctor->getDecl()->isCopyOrMoveConstructor());
+ assert(isAlmostTrivial(Ctor->getDecl()));
+ assert(isCopyOrMoveLike(Ctor->getDecl()));
ThisVal = Ctor->getCXXThisVal();
AlwaysReturnsLValue = false;
} else {
- assert(cast<CXXMethodDecl>(Call.getDecl())->isTrivial());
+ assert(isAlmostTrivial(cast<CXXMethodDecl>(Call.getDecl())));
assert(cast<CXXMethodDecl>(Call.getDecl())->getOverloadedOperator() ==
OO_Equal);
ThisVal = cast<CXXInstanceCall>(Call).getCXXThisVal();
@@ -332,8 +445,8 @@
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
bool IsArray = isa<ElementRegion>(Target);
- if (CE->getConstructor()->isTrivial() &&
- CE->getConstructor()->isCopyOrMoveConstructor() &&
+ if (isAlmostTrivial(CE->getConstructor()) &&
+ isCopyOrMoveLike(CE->getConstructor()) &&
!IsArray) {
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
