r177220 - [analyzer] Model trivial copy/move assignment operators with a bind as well.

Jordan Rose jordan_rose at apple.com
Fri Mar 15 19:14:06 PDT 2013


Author: jrose
Date: Fri Mar 15 21:14:06 2013
New Revision: 177220

URL: http://llvm.org/viewvc/llvm-project?rev=177220&view=rev
Log:
[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>

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/ctor-inlining.mm

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=177220&r1=177219&r2=177220&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri Mar 15 21:14:06 2013
@@ -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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=177220&r1=177219&r2=177220&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Mar 15 21:14:06 2013
@@ -48,13 +48,25 @@ void ExprEngine::CreateCXXTemporaryObjec
   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(Node
 
   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);
   }
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=177220&r1=177219&r2=177220&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Fri Mar 15 21:14:06 2013
@@ -750,12 +750,32 @@ bool ExprEngine::shouldInlineCall(const
   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.

Modified: cfe/trunk/test/Analysis/ctor-inlining.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctor-inlining.mm?rev=177220&r1=177219&r2=177220&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor-inlining.mm (original)
+++ cfe/trunk/test/Analysis/ctor-inlining.mm Fri Mar 15 21:14:06 2013
@@ -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);
+    }
+  }
 }





More information about the cfe-commits mailing list