baloghadamsoftware updated this revision to Diff 64149.
baloghadamsoftware added a comment.
Revised version based on comments.
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
@@ -19,6 +19,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
using namespace clang;
using namespace ento;
@@ -34,19 +36,136 @@
Bldr.generateNode(ME, Pred, state);
}
+static 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;
+}
+
+static bool isAlmostTrivialConstructor(const CXXMethodDecl *Met) {
+ if (Met->isTrivial())
+ return true;
+
+ if (!Met->hasTrivialBody()) // Returns false if body is not available
+ 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;
+
+ llvm::SmallPtrSet<const Type *, 32> InitBaseSet;
+ llvm::SmallPtrSet<const FieldDecl *, 32> InitFieldSet;
+
+ for (const auto *Initzer : Constr->inits()) {
+ if (Initzer->isBaseInitializer()) {
+ const auto *Base = Initzer->getBaseClass();
+ if (const auto *CtrCall = dyn_cast<CXXConstructExpr>(
+ Initzer->getInit()->IgnoreParenImpCasts())) {
+ if (!isCopyOrMoveLike(CtrCall->getConstructor()) ||
+ !isAlmostTrivialConstructor(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;
+ }
+ InitBaseSet.insert(Base);
+ } else if (Initzer->isMemberInitializer()) {
+ const auto *Field = Initzer->getMember();
+ 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()) ||
+ !isAlmostTrivialConstructor(CtrCall->getConstructor()))
+ return false;
+ InitMem =
+ dyn_cast<MemberExpr>(CtrCall->getArg(0)->IgnoreParenImpCasts());
+ } else {
+ return false;
+ }
+ }
+ if (!InitMem)
+ return false;
+ if (InitMem->getMemberDecl() != Field)
+ return false;
+ if (const auto *Base = dyn_cast<DeclRefExpr>(InitMem->getBase())) {
+ if (Base->getDecl() != Param)
+ return false;
+ } else {
+ return false;
+ }
+ InitFieldSet.insert(Field);
+ }
+ }
+
+ for (const auto Base : ThisRecDecl->bases()) {
+ if (Base.getType()->getAsCXXRecordDecl()->field_empty())
+ continue;
+ if (!InitBaseSet.count(&*Base.getType()))
+ return false;
+ }
+
+ for (const auto *Field : ThisRecDecl->fields()) {
+ if (!Field->getType()->isScalarType() && !Field->getType()->isRecordType())
+ return false;
+ if (!InitFieldSet.count(Field))
+ 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(isAlmostTrivialConstructor(Ctor->getDecl()));
+ assert(isCopyOrMoveLike(Ctor->getDecl()));
ThisVal = Ctor->getCXXThisVal();
AlwaysReturnsLValue = false;
} else {
- assert(cast<CXXMethodDecl>(Call.getDecl())->isTrivial());
+ assert(isAlmostTrivialConstructor(cast<CXXMethodDecl>(Call.getDecl())));
assert(cast<CXXMethodDecl>(Call.getDecl())->getOverloadedOperator() ==
OO_Equal);
ThisVal = cast<CXXInstanceCall>(Call).getCXXThisVal();
@@ -332,9 +451,8 @@
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
bool IsArray = isa<ElementRegion>(Target);
- if (CE->getConstructor()->isTrivial() &&
- CE->getConstructor()->isCopyOrMoveConstructor() &&
- !IsArray) {
+ if (isAlmostTrivialConstructor(CE->getConstructor()) &&
+ isCopyOrMoveLike(CE->getConstructor()) && !IsArray) {
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
