diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 8fa74fefefd3534d15b570abdb6a5c6d43818ba9..32b1f8003452e33e90a500ce524eb0ce84f96638 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 32b522cbd5b3369403e64e8f83aa41f227a312a0..d1a591c7fe06285939aa57efa31d6234e7733283 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 44803dc477237aac22ec8d1af6fe6423d60f522e..e88091180f18af7079b2f7333f10aa1cf9e62294 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 1603ff3893449f2fee04f088d31428d4a747f0e6..fd8caf324aa39ff9ef5e718e836572b1178de699 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); + } + } }