From: Jordan Rose Date: Sat, 2 Feb 2013 05:15:53 +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=2a3fe34b4a2a1b6ceab8838b896435378ae0e692;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174265 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 1d006b09cc..54afe65d3e 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}} }