From a8d937e4bdd39cdf503f77454e9dc4c9c730a9f7 Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Sat, 16 Mar 2013 02:14:06 +0000
Subject: [PATCH] [analyzer] Model trivial copy/move assignment operators with
a bind as well.
r175234 allowed the analyzer to model trivial copy/move constructors as
an aggregate bind. This commit extends that to trivial assignment
operators as well. Like the last commit, one of the motivating factors here
is not warning when the right-hand object is partially-initialized, which
can have legitimate uses.
<rdar://problem/13405162>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177220 91177308-0d34-0410-b5e6-96231b3b80d8
---
.../Core/PathSensitive/ExprEngine.h | 5 +-
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 37 ++++++--
.../Core/ExprEngineCallAndReturn.cpp | 20 +++++
test/Analysis/ctor-inlining.mm | 90 +++++++++++++++++++
4 files changed, 141 insertions(+), 11 deletions(-)
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 8fa74fefefd..32b1f800345 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -553,9 +553,10 @@ private:
bool replayWithoutInlining(ExplodedNode *P, const LocationContext *CalleeLC);
- /// Models a trivial copy or move constructor call with a simple bind.
+ /// Models a trivial copy or move constructor or trivial assignment operator
+ /// call with a simple bind.
void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
- const CXXConstructorCall &Call);
+ const CallEvent &Call);
/// If the value of the given expression is a NonLoc, copy it into a new
/// temporary object region, and replace the value of the expression with
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 32b522cbd5b..d1a591c7fe0 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -48,13 +48,25 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
Bldr.generateNode(ME, Pred, state);
}
+// 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 CXXConstructorCall &Call) {
- const CXXConstructExpr *CtorExpr = Call.getOriginExpr();
- assert(CtorExpr->getConstructor()->isCopyOrMoveConstructor());
- assert(CtorExpr->getConstructor()->isTrivial());
+ const CallEvent &Call) {
+ SVal ThisVal;
+ bool AlwaysReturnsLValue;
+ if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
+ assert(Ctor->getDecl()->isTrivial());
+ assert(Ctor->getDecl()->isCopyOrMoveConstructor());
+ ThisVal = Ctor->getCXXThisVal();
+ AlwaysReturnsLValue = false;
+ } else {
+ assert(cast<CXXMethodDecl>(Call.getDecl())->isTrivial());
+ assert(cast<CXXMethodDecl>(Call.getDecl())->getOverloadedOperator() ==
+ OO_Equal);
+ ThisVal = cast<CXXInstanceCall>(Call).getCXXThisVal();
+ AlwaysReturnsLValue = true;
+ }
- SVal ThisVal = Call.getCXXThisVal();
const LocationContext *LCtx = Pred->getLocationContext();
ExplodedNodeSet Dst;
@@ -62,17 +74,24 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
SVal V = Call.getArgSVal(0);
- // Make sure the value being copied is not unknown.
+ // If the value being copied is not unknown, load from its location to get
+ // an aggregate rvalue.
if (Optional<Loc> L = V.getAs<Loc>())
V = Pred->getState()->getSVal(*L);
+ else
+ assert(V.isUnknown());
- evalBind(Dst, CtorExpr, Pred, ThisVal, V, true);
+ const Expr *CallExpr = Call.getOriginExpr();
+ evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
- PostStmt PS(CtorExpr, LCtx);
+ PostStmt PS(CallExpr, LCtx);
for (ExplodedNodeSet::iterator I = Dst.begin(), E = Dst.end();
I != E; ++I) {
ProgramStateRef State = (*I)->getState();
- State = bindReturnValue(Call, LCtx, State);
+ if (AlwaysReturnsLValue)
+ State = State->BindExpr(CallExpr, LCtx, ThisVal);
+ else
+ State = bindReturnValue(Call, LCtx, State);
Bldr.generateNode(PS, State, *I);
}
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 44803dc4772..e88091180f1 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -750,12 +750,32 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
return true;
}
+static bool isTrivialObjectAssignment(const CallEvent &Call) {
+ const CXXInstanceCall *ICall = dyn_cast<CXXInstanceCall>(&Call);
+ if (!ICall)
+ return false;
+
+ const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(ICall->getDecl());
+ if (!MD)
+ return false;
+ if (!(MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()))
+ return false;
+
+ return MD->isTrivial();
+}
+
void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
const CallEvent &CallTemplate) {
// Make sure we have the most recent state attached to the call.
ProgramStateRef State = Pred->getState();
CallEventRef<> Call = CallTemplate.cloneWithState(State);
+ // Special-case trivial assignment operators.
+ if (isTrivialObjectAssignment(*Call)) {
+ performTrivialCopy(Bldr, Pred, *Call);
+ return;
+ }
+
// Try to inline the call.
// The origin expression here is just used as a kind of checksum;
// this should still be safe even for CallEvents that don't come from exprs.
diff --git a/test/Analysis/ctor-inlining.mm b/test/Analysis/ctor-inlining.mm
index 1603ff38934..fd8caf324aa 100644
--- a/test/Analysis/ctor-inlining.mm
+++ b/test/Analysis/ctor-inlining.mm
@@ -149,6 +149,19 @@ namespace PODUninitialized {
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
}
+
+ NonPOD &operator=(const NonPOD &Other)
+ {
+ x = Other.x;
+ y = Other.y; // expected-warning {{undefined}}
+ return *this;
+ }
+ NonPOD &operator=(NonPOD &&Other)
+ {
+ x = Other.x;
+ y = Other.y; // expected-warning {{undefined}}
+ return *this;
+ }
};
class NonPODWrapper {
@@ -166,6 +179,19 @@ namespace PODUninitialized {
: x(Other.x), y(Other.y) // expected-warning {{undefined}}
{
}
+
+ Inner &operator=(const Inner &Other)
+ {
+ x = Other.x; // expected-warning {{undefined}}
+ y = Other.y;
+ return *this;
+ }
+ Inner &operator=(Inner &&Other)
+ {
+ x = Other.x; // expected-warning {{undefined}}
+ y = Other.y;
+ return *this;
+ }
};
Inner p;
@@ -216,4 +242,68 @@ namespace PODUninitialized {
w.p.y = 1;
NonPODWrapper w2 = move(w);
}
+
+ // Not strictly about constructors, but trivial assignment operators should
+ // essentially work the same way.
+ namespace AssignmentOperator {
+ void testPOD() {
+ POD p;
+ p.x = 1;
+ POD p2;
+ p2 = p; // no-warning
+ clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
+ POD p3;
+ p3 = move(p); // no-warning
+ clang_analyzer_eval(p3.x == 1); // expected-warning{{TRUE}}
+
+ PODWrapper w;
+ w.p.y = 1;
+ PODWrapper w2;
+ w2 = w; // no-warning
+ clang_analyzer_eval(w2.p.y == 1); // expected-warning{{TRUE}}
+ PODWrapper w3;
+ w3 = move(w); // no-warning
+ clang_analyzer_eval(w3.p.y == 1); // expected-warning{{TRUE}}
+ }
+
+ void testReturnValue() {
+ POD p;
+ p.x = 1;
+ POD p2;
+ clang_analyzer_eval(&(p2 = p) == &p2); // expected-warning{{TRUE}}
+
+ PODWrapper w;
+ w.p.y = 1;
+ PODWrapper w2;
+ clang_analyzer_eval(&(w2 = w) == &w2); // expected-warning{{TRUE}}
+ }
+
+ void testNonPOD() {
+ NonPOD p;
+ p.x = 1;
+ NonPOD p2;
+ p2 = p;
+ }
+
+ void testNonPODMove() {
+ NonPOD p;
+ p.x = 1;
+ NonPOD p2;
+ p2 = move(p);
+ }
+
+ void testNonPODWrapper() {
+ NonPODWrapper w;
+ w.p.y = 1;
+ NonPODWrapper w2;
+ w2 = w;
+ }
+
+ void testNonPODWrapperMove() {
+ NonPODWrapper w;
+ w.p.y = 1;
+ NonPODWrapper w2;
+ w2 = move(w);
+ }
+ }
}
--
GitLab