r174265 - Revert "[analyzer] Model trivial copy/move ctors with an aggregate bind."

Jordan Rose jordan_rose at apple.com
Fri Feb 1 21:15:54 PST 2013


Author: jrose
Date: Fri Feb  1 23:15:53 2013
New Revision: 174265

URL: http://llvm.org/viewvc/llvm-project?rev=174265&view=rev
Log:
Revert "[analyzer] Model trivial copy/move ctors with an aggregate bind."

...again. The problem has not been fixed and our internal buildbot is still
getting hangs.

This reverts r174212, originally applied in r173951, then reverted in r174069.
Will not re-apply until the entire project analyzes successfully on my
local machine.

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
    cfe/trunk/test/Analysis/temporaries.cpp

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=174265&r1=174264&r2=174265&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri Feb  1 23:15:53 2013
@@ -44,7 +44,6 @@ namespace ento {
 class AnalysisManager;
 class CallEvent;
 class SimpleCall;
-class CXXConstructorCall;
 
 class ExprEngine : public SubEngine {
 public:
@@ -547,10 +546,6 @@ private:
                      ExplodedNode *Pred);
 
   bool replayWithoutInlining(ExplodedNode *P, const LocationContext *CalleeLC);
-
-  /// Models a trivial copy or move constructor call with a simple bind.
-  void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
-                          const CXXConstructorCall &Call);
 };
 
 /// Traits for storing the call processing policy inside GDM.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=174265&r1=174264&r2=174265&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Feb  1 23:15:53 2013
@@ -49,35 +49,6 @@ void ExprEngine::CreateCXXTemporaryObjec
   Bldr.generateNode(ME, Pred, state->BindExpr(ME, LCtx, V));
 }
 
-void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
-                                    const CXXConstructorCall &Call) {
-  const CXXConstructExpr *CtorExpr = Call.getOriginExpr();
-  assert(CtorExpr->getConstructor()->isCopyOrMoveConstructor());
-  assert(CtorExpr->getConstructor()->isTrivial());
-
-  SVal ThisVal = Call.getCXXThisVal();
-  const LocationContext *LCtx = Pred->getLocationContext();
-
-  ExplodedNodeSet Dst;
-  Bldr.takeNodes(Pred);
-
-  SVal V = Call.getArgSVal(0);
-
-  // Make sure the value being copied is not unknown.
-  if (const Loc *L = dyn_cast<Loc>(&V))
-    V = Pred->getState()->getSVal(*L);
-
-  evalBind(Dst, CtorExpr, Pred, ThisVal, V, true);
-
-  PostStmt PS(CtorExpr, LCtx);
-  for (ExplodedNodeSet::iterator I = Dst.begin(), E = Dst.end();
-       I != E; ++I) {
-    ProgramStateRef State = (*I)->getState();
-    State = bindReturnValue(Call, LCtx, State);
-    Bldr.generateNode(PS, State, *I);
-  }
-}
-
 void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
                                        ExplodedNode *Pred,
                                        ExplodedNodeSet &destNodes) {
@@ -85,7 +56,6 @@ void ExprEngine::VisitCXXConstructExpr(c
   ProgramStateRef State = Pred->getState();
 
   const MemRegion *Target = 0;
-  bool IsArray = false;
 
   switch (CE->getConstructionKind()) {
   case CXXConstructExpr::CK_Complete: {
@@ -109,7 +79,6 @@ void ExprEngine::VisitCXXConstructExpr(c
                 Target = State->getLValue(AT->getElementType(),
                                           getSValBuilder().makeZeroArrayIndex(),
                                           Base).getAsRegion();
-                IsArray = true;
               } else {
                 Target = State->getLValue(Var, LCtx).getAsRegion();
               }
@@ -179,25 +148,14 @@ void ExprEngine::VisitCXXConstructExpr(c
   getCheckerManager().runCheckersForPreCall(DstPreCall, DstPreVisit,
                                             *Call, *this);
 
-  ExplodedNodeSet DstEvaluated;
-  StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
-
-  if (CE->getConstructor()->isTrivial() &&
-      CE->getConstructor()->isCopyOrMoveConstructor() &&
-      !IsArray) {
-    // FIXME: Handle other kinds of trivial constructors as well.
-    for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-         I != E; ++I)
-      performTrivialCopy(Bldr, *I, *Call);
-
-  } else {
-    for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-         I != E; ++I)
-      defaultEvalCall(Bldr, *I, *Call);
-  }
+  ExplodedNodeSet DstInvalidated;
+  StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
+  for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
+       I != E; ++I)
+    defaultEvalCall(Bldr, *I, *Call);
 
   ExplodedNodeSet DstPostCall;
-  getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
+  getCheckerManager().runCheckersForPostCall(DstPostCall, DstInvalidated,
                                              *Call, *this);
   getCheckerManager().runCheckersForPostStmt(destNodes, DstPostCall, CE, *this);
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=174265&r1=174264&r2=174265&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Fri Feb  1 23:15:53 2013
@@ -187,23 +187,6 @@ static bool wasDifferentDeclUsedForInlin
   return RuntimeCallee->getCanonicalDecl() != StaticDecl->getCanonicalDecl();
 }
 
-/// Returns true if the CXXConstructExpr \p E was intended to construct a
-/// prvalue for the region in \p V.
-///
-/// Note that we can't just test for rvalue vs. glvalue because
-/// CXXConstructExprs embedded in DeclStmts and initializers are considered
-/// rvalues by the AST, and the analyzer would like to treat them as lvalues.
-static bool isTemporaryPRValue(const CXXConstructExpr *E, SVal V) {
-  if (E->isGLValue())
-    return false;
-
-  const MemRegion *MR = V.getAsRegion();
-  if (!MR)
-    return false;
-
-  return isa<CXXTempObjectRegion>(MR);
-}
-
 /// The call exit is simulated with a sequence of nodes, which occur between 
 /// CallExitBegin and CallExitEnd. The following operations occur between the 
 /// two program points:
@@ -264,9 +247,13 @@ void ExprEngine::processCallExit(Explode
         svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
       SVal ThisV = state->getSVal(This);
 
-      // If the constructed object is a temporary prvalue, get its bindings.
-      if (isTemporaryPRValue(CCE, ThisV))
-        ThisV = state->getSVal(cast<Loc>(ThisV));
+      // If the constructed object is a prvalue, get its bindings.
+      // Note that we have to be careful here because constructors embedded
+      // in DeclStmts are not marked as lvalues.
+      if (!CCE->isGLValue())
+        if (const MemRegion *MR = ThisV.getAsRegion())
+          if (isa<CXXTempObjectRegion>(MR))
+            ThisV = state->getSVal(cast<Loc>(ThisV));
 
       state = state->BindExpr(CCE, callerCtx, ThisV);
     }
@@ -705,13 +692,7 @@ ProgramStateRef ExprEngine::bindReturnVa
     }
     }
   } else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
-    SVal ThisV = C->getCXXThisVal();
-
-    // If the constructed object is a temporary prvalue, get its bindings.
-    if (isTemporaryPRValue(cast<CXXConstructExpr>(E), ThisV))
-      ThisV = State->getSVal(cast<Loc>(ThisV));
-
-    return State->BindExpr(E, LCtx, ThisV);
+    return State->BindExpr(E, LCtx, C->getCXXThisVal());
   }
 
   // Conjure a symbol if the return value is unknown.

Modified: cfe/trunk/test/Analysis/ctor-inlining.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctor-inlining.mm?rev=174265&r1=174264&r2=174265&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor-inlining.mm (original)
+++ cfe/trunk/test/Analysis/ctor-inlining.mm Fri Feb  1 23:15:53 2013
@@ -1,15 +1,8 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
 
-// A simplified version of std::move.
-template <typename T>
-T &&move(T &obj) {
-  return static_cast<T &&>(obj);
-}
-
-
 struct Wrapper {
   __strong id obj;
 };
@@ -124,96 +117,3 @@ namespace ConstructorUsedAsRValue {
     clang_analyzer_eval(result); // expected-warning{{TRUE}}
   }
 }
-
-namespace PODUninitialized {
-  class POD {
-  public:
-    int x, y;
-  };
-
-  class PODWrapper {
-  public:
-    POD p;
-  };
-
-  class NonPOD {
-  public:
-    int x, y;
-
-    NonPOD() {}
-    NonPOD(const NonPOD &Other)
-      : x(Other.x), y(Other.y) // expected-warning {{undefined}}
-    {
-    }
-    NonPOD(NonPOD &&Other)
-    : x(Other.x), y(Other.y) // expected-warning {{undefined}}
-    {
-    }
-  };
-
-  class NonPODWrapper {
-  public:
-    class Inner {
-    public:
-      int x, y;
-
-      Inner() {}
-      Inner(const Inner &Other)
-        : x(Other.x), y(Other.y) // expected-warning {{undefined}}
-      {
-      }
-      Inner(Inner &&Other)
-      : x(Other.x), y(Other.y) // expected-warning {{undefined}}
-      {
-      }
-    };
-
-    Inner p;
-  };
-
-  void testPOD() {
-    POD p;
-    p.x = 1;
-    POD p2 = p; // no-warning
-    clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}}
-    POD p3 = move(p); // no-warning
-    clang_analyzer_eval(p3.x == 1); // expected-warning{{TRUE}}
-
-    // Use rvalues as well.
-    clang_analyzer_eval(POD(p3).x == 1); // expected-warning{{TRUE}}
-
-    PODWrapper w;
-    w.p.y = 1;
-    PODWrapper w2 = w; // no-warning
-    clang_analyzer_eval(w2.p.y == 1); // expected-warning{{TRUE}}
-    PODWrapper w3 = move(w); // no-warning
-    clang_analyzer_eval(w3.p.y == 1); // expected-warning{{TRUE}}
-
-    // Use rvalues as well.
-    clang_analyzer_eval(PODWrapper(w3).p.y == 1); // expected-warning{{TRUE}}
-  }
-
-  void testNonPOD() {
-    NonPOD p;
-    p.x = 1;
-    NonPOD p2 = p;
-  }
-
-  void testNonPODMove() {
-    NonPOD p;
-    p.x = 1;
-    NonPOD p2 = move(p);
-  }
-
-  void testNonPODWrapper() {
-    NonPODWrapper w;
-    w.p.y = 1;
-    NonPODWrapper w2 = w;
-  }
-
-  void testNonPODWrapperMove() {
-    NonPODWrapper w;
-    w.p.y = 1;
-    NonPODWrapper w2 = move(w);
-  }
-}

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=174265&r1=174264&r2=174265&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Fri Feb  1 23:15:53 2013
@@ -16,7 +16,7 @@ Trivial getTrivial() {
 }
 
 const Trivial &getTrivialRef() {
-  return Trivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const struct Trivial' returned to caller}}
+  return Trivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'struct Trivial' returned to caller}}
 }
 
 
@@ -25,6 +25,6 @@ NonTrivial getNonTrivial() {
 }
 
 const NonTrivial &getNonTrivialRef() {
-  return NonTrivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const struct NonTrivial' returned to caller}}
+  return NonTrivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'struct NonTrivial' returned to caller}}
 }
 





More information about the cfe-commits mailing list