r173951 - [analyzer] Model trivial copy/move ctors with an aggregate bind.
Jordan Rose
jordan_rose at apple.com
Wed Jan 30 10:16:06 PST 2013
Author: jrose
Date: Wed Jan 30 12:16:06 2013
New Revision: 173951
URL: http://llvm.org/viewvc/llvm-project?rev=173951&view=rev
Log:
[analyzer] Model trivial copy/move ctors with an aggregate bind.
This is faster for the analyzer to process than inlining the constructor
and performing a member-wise copy, and it also solves the problem of
warning when a partially-initialized POD struct is copied.
Before:
CGPoint p;
p.x = 0;
CGPoint p2 = p; <-- assigned value is garbage or undefined
After:
CGPoint p;
p.x = 0;
CGPoint p2 = p; // no-warning
This matches our behavior in C, where we don't see a field-by-field copy.
<rdar://problem/12305288>
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=173951&r1=173950&r2=173951&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Jan 30 12:16:06 2013
@@ -44,6 +44,7 @@ namespace ento {
class AnalysisManager;
class CallEvent;
class SimpleCall;
+class CXXConstructorCall;
class ExprEngine : public SubEngine {
public:
@@ -546,6 +547,10 @@ 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=173951&r1=173950&r2=173951&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jan 30 12:16:06 2013
@@ -49,6 +49,35 @@ 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) {
@@ -56,6 +85,7 @@ void ExprEngine::VisitCXXConstructExpr(c
ProgramStateRef State = Pred->getState();
const MemRegion *Target = 0;
+ bool IsArray = false;
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
@@ -79,6 +109,7 @@ void ExprEngine::VisitCXXConstructExpr(c
Target = State->getLValue(AT->getElementType(),
getSValBuilder().makeZeroArrayIndex(),
Base).getAsRegion();
+ IsArray = true;
} else {
Target = State->getLValue(Var, LCtx).getAsRegion();
}
@@ -148,14 +179,25 @@ void ExprEngine::VisitCXXConstructExpr(c
getCheckerManager().runCheckersForPreCall(DstPreCall, DstPreVisit,
*Call, *this);
- ExplodedNodeSet DstInvalidated;
- StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
- for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
- I != E; ++I)
- defaultEvalCall(Bldr, *I, *Call);
+ 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 DstPostCall;
- getCheckerManager().runCheckersForPostCall(DstPostCall, DstInvalidated,
+ getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
*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=173951&r1=173950&r2=173951&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Jan 30 12:16:06 2013
@@ -187,6 +187,23 @@ 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:
@@ -247,13 +264,9 @@ void ExprEngine::processCallExit(Explode
svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
SVal ThisV = state->getSVal(This);
- // 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));
+ // If the constructed object is a temporary prvalue, get its bindings.
+ if (isTemporaryPRValue(CCE, ThisV))
+ ThisV = state->getSVal(cast<Loc>(ThisV));
state = state->BindExpr(CCE, callerCtx, ThisV);
}
@@ -692,7 +705,13 @@ ProgramStateRef ExprEngine::bindReturnVa
}
}
} else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
- return State->BindExpr(E, LCtx, C->getCXXThisVal());
+ 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);
}
// 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=173951&r1=173950&r2=173951&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor-inlining.mm (original)
+++ cfe/trunk/test/Analysis/ctor-inlining.mm Wed Jan 30 12:16:06 2013
@@ -1,8 +1,15 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -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;
};
@@ -117,3 +124,96 @@ 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=173951&r1=173950&r2=173951&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Wed Jan 30 12:16:06 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 'struct Trivial' returned to caller}}
+ return Trivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const 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 'struct NonTrivial' returned to caller}}
+ return NonTrivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const struct NonTrivial' returned to caller}}
}
More information about the cfe-commits
mailing list