From: Jordan Rose Date: Thu, 31 Jan 2013 18:04:03 +0000 (+0000) Subject: Revert "[analyzer] Model trivial copy/move ctors with an aggregate bind." X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=33e83b6cf776875be5716d214710717a898325c0;p=clang Revert "[analyzer] Model trivial copy/move ctors with an aggregate bind." It's causing hangs on our internal analyzer buildbot. Will restore after investigating. This reverts r173951 / baa7ca1142990e1ad6d4e9d2c73adb749ff50789. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174069 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 598a915cd8..ba63620afb 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -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. diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index fdd50a6b54..1f0c523ac6 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -49,35 +49,6 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME, 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(&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(const CXXConstructExpr *CE, 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(const CXXConstructExpr *CE, 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(const CXXConstructExpr *CE, 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); } diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 2c1f6c1d8f..824f76d952 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -187,23 +187,6 @@ static bool wasDifferentDeclUsedForInlining(CallEventRef<> Call, 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(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(ExplodedNode *CEBNode) { 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(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(MR)) + ThisV = state->getSVal(cast(ThisV)); state = state->BindExpr(CCE, callerCtx, ThisV); } @@ -705,13 +692,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call, } } } else if (const CXXConstructorCall *C = dyn_cast(&Call)){ - SVal ThisV = C->getCXXThisVal(); - - // If the constructed object is a temporary prvalue, get its bindings. - if (isTemporaryPRValue(cast(E), ThisV)) - ThisV = State->getSVal(cast(ThisV)); - - return State->BindExpr(E, LCtx, ThisV); + return State->BindExpr(E, LCtx, C->getCXXThisVal()); } // Conjure a symbol if the return value is unknown. diff --git a/test/Analysis/ctor-inlining.mm b/test/Analysis/ctor-inlining.mm index 1603ff3893..be5d81a58e 100644 --- a/test/Analysis/ctor-inlining.mm +++ b/test/Analysis/ctor-inlining.mm @@ -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 -T &&move(T &obj) { - return static_cast(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); - } -} diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp index e885878783..547be02791 100644 --- a/test/Analysis/temporaries.cpp +++ b/test/Analysis/temporaries.cpp @@ -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}} }