baloghadamsoftware updated this revision to Diff 120954.
baloghadamsoftware added a comment.
Herald added subscribers: szepet, whisperity.
Tests updated.
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
@@ -146,10 +146,12 @@
NonPOD(const NonPOD &Other)
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
+ int z = 1;
}
NonPOD(NonPOD &&Other)
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
+ int z = 1;
}
NonPOD &operator=(const NonPOD &Other)
@@ -176,10 +178,12 @@
Inner(const Inner &Other)
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
+ int z = 1;
}
Inner(Inner &&Other)
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
+ int z = 1;
}
Inner &operator=(const Inner &Other)
@@ -199,6 +203,19 @@
Inner p;
};
+ class AlmostPOD {
+ public:
+ int x, y;
+
+ AlmostPOD() {}
+ AlmostPOD(const AlmostPOD &Other)
+ : x(Other.x), y(Other.y) // no-warning
+ {}
+ AlmostPOD(AlmostPOD &&Other)
+ : x(Other.x), y(Other.y) // no-warning
+ {}
+ };
+
void testPOD(const POD &pp) {
POD p;
p.x = 1;
@@ -254,6 +271,31 @@
NonPODWrapper w2 = move(w);
}
+ void testAlmostPOD() {
+ AlmostPOD p;
+ p.x = 1;
+ AlmostPOD p2 = p; // no-warning
+ clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
+ }
+
+ void testAlmostPODMove() {
+ AlmostPOD p;
+ p.x = 1;
+ AlmostPOD p2 = move(p); // no-warning
+ clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
+ }
+
+ // FIXME: These copies are now handled with a single per-structure bind,
+ // and the undefined assignment checker fails to realize that
+ // all contents of the structure that's being copied are undefined.
+ // Perhaps we could teach the checker to warn here.
+ void testCompletelyUndefined() {
+ POD p;
+ POD p2 = p; // no-warning
+ AlmostPOD ap;
+ AlmostPOD ap2 = ap; // no-warning
+ }
+
// Not strictly about constructors, but trivial assignment operators should
// essentially work the same way.
namespace AssignmentOperator {
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
