[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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 8fa74fefefd3534d15b570abdb6a5c6d43818ba9..32b1f8003452e33e90a500ce524eb0ce84f96638 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);
+    }
+  }
 }