diff options
| -rw-r--r-- | lib/StaticAnalyzer/Core/CallEvent.cpp | 10 | ||||
| -rw-r--r-- | lib/StaticAnalyzer/Core/ExprEngine.cpp | 77 | ||||
| -rw-r--r-- | test/Analysis/array-struct-region.cpp | 73 | ||||
| -rw-r--r-- | test/Analysis/fields.c | 12 | ||||
| -rw-r--r-- | test/Analysis/reference.cpp | 4 |
5 files changed, 136 insertions, 40 deletions
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 3b4c13471b..31e4cf05d9 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -401,15 +401,7 @@ SVal CXXInstanceCall::getCXXThisVal() const { return UnknownVal(); SVal ThisVal = getSVal(Base); - - // FIXME: This is only necessary because we can call member functions on - // struct rvalues, which do not have regions we can use for a 'this' pointer. - // Ideally this should eventually be changed to an assert, i.e. all - // non-Unknown, non-null 'this' values should be loc::MemRegionVals. - if (isa<DefinedSVal>(ThisVal)) - if (!ThisVal.getAsRegion() && !ThisVal.isConstant()) - return UnknownVal(); - + assert(ThisVal.isUnknownOrUndef() || isa<Loc>(ThisVal)); return ThisVal; } diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index 007bcf5208..213ab7ad5c 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -163,6 +163,23 @@ ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) { return state; } +/// If the value of the given expression is a NonLoc, copy it into a new +/// temporary region, and replace the value of the expression with that. +static ProgramStateRef createTemporaryRegionIfNeeded(ProgramStateRef State, + const LocationContext *LC, + const Expr *E) { + SVal V = State->getSVal(E, LC); + + if (isa<NonLoc>(V)) { + MemRegionManager &MRMgr = State->getStateManager().getRegionManager(); + const MemRegion *R = MRMgr.getCXXTempObjectRegion(E, LC); + State = State->bindLoc(loc::MemRegionVal(R), V); + State = State->BindExpr(E, LC, loc::MemRegionVal(R)); + } + + return State; +} + //===----------------------------------------------------------------------===// // Top-level transfer function logic (Dispatcher). //===----------------------------------------------------------------------===// @@ -717,8 +734,26 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, break; } + case Stmt::CXXOperatorCallExprClass: { + const CXXOperatorCallExpr *OCE = cast<CXXOperatorCallExpr>(S); + + // For instance method operators, make sure the 'this' argument has a + // valid region. + const Decl *Callee = OCE->getCalleeDecl(); + if (const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(Callee)) { + if (MD->isInstance()) { + ProgramStateRef State = Pred->getState(); + const LocationContext *LCtx = Pred->getLocationContext(); + ProgramStateRef NewState = + createTemporaryRegionIfNeeded(State, LCtx, OCE->getArg(0)); + if (NewState != State) + Pred = Bldr.generateNode(OCE, Pred, NewState, /*Tag=*/0, + ProgramPoint::PreStmtKind); + } + } + // FALLTHROUGH + } case Stmt::CallExprClass: - case Stmt::CXXOperatorCallExprClass: case Stmt::CXXMemberCallExprClass: case Stmt::UserDefinedLiteralClass: { Bldr.takeNodes(Pred); @@ -1478,6 +1513,7 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, ExplodedNodeSet Dst; Decl *member = M->getMemberDecl(); + // Handle static member variables accessed via member syntax. if (VarDecl *VD = dyn_cast<VarDecl>(member)) { assert(M->isGLValue()); Bldr.takeNodes(Pred); @@ -1486,40 +1522,27 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, return; } + ProgramStateRef state = Pred->getState(); + const LocationContext *LCtx = Pred->getLocationContext(); + Expr *BaseExpr = M->getBase(); + // Handle C++ method calls. if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(member)) { - Bldr.takeNodes(Pred); - SVal MDVal = svalBuilder.getFunctionPointer(MD); - ProgramStateRef state = - Pred->getState()->BindExpr(M, Pred->getLocationContext(), MDVal); - Bldr.generateNode(M, Pred, state); - return; - } - + if (MD->isInstance()) + state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr); - FieldDecl *field = dyn_cast<FieldDecl>(member); - if (!field) // FIXME: skipping member expressions for non-fields - return; + SVal MDVal = svalBuilder.getFunctionPointer(MD); + state = state->BindExpr(M, LCtx, MDVal); - Expr *baseExpr = M->getBase()->IgnoreParens(); - ProgramStateRef state = Pred->getState(); - const LocationContext *LCtx = Pred->getLocationContext(); - SVal baseExprVal = state->getSVal(baseExpr, Pred->getLocationContext()); - if (isa<nonloc::LazyCompoundVal>(baseExprVal) || - isa<nonloc::CompoundVal>(baseExprVal) || - // FIXME: This can originate by conjuring a symbol for an unknown - // temporary struct object, see test/Analysis/fields.c: - // (p = getit()).x - isa<nonloc::SymbolVal>(baseExprVal)) { - Bldr.generateNode(M, Pred, state->BindExpr(M, LCtx, UnknownVal())); + Bldr.generateNode(M, Pred, state); return; } - // FIXME: Should we insert some assumption logic in here to determine - // if "Base" is a valid piece of memory? Before we put this assumption - // later when using FieldOffset lvals (which we no longer have). + // Handle regular struct fields / member variables. + state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr); + SVal baseExprVal = state->getSVal(BaseExpr, LCtx); - // For all other cases, compute an lvalue. + FieldDecl *field = cast<FieldDecl>(member); SVal L = state->getLValue(field, baseExprVal); if (M->isGLValue()) { if (field->getType()->isReferenceType()) { diff --git a/test/Analysis/array-struct-region.cpp b/test/Analysis/array-struct-region.cpp index 3581566bdc..cffa64d21a 100644 --- a/test/Analysis/array-struct-region.cpp +++ b/test/Analysis/array-struct-region.cpp @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s void clang_analyzer_eval(int); @@ -8,10 +10,29 @@ struct S { #if __cplusplus const struct S *getThis() const { return this; } + const struct S *operator +() const { return this; } + + bool check() const { return this == this; } + bool operator !() const { return this != this; } + + int operator *() const { return field; } #endif }; +#if __cplusplus +const struct S *operator -(const struct S &s) { return &s; } +bool operator ~(const struct S &s) { return &s != &s; } +#endif + + +#ifdef INLINE +struct S getS() { + struct S s = { 42 }; + return s; +} +#else struct S getS(); +#endif void testAssignment() { @@ -25,6 +46,14 @@ void testAssignment() { #if __cplusplus clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}} + clang_analyzer_eval(+s == &s); // expected-warning{{TRUE}} + clang_analyzer_eval(-s == &s); // expected-warning{{TRUE}} + + clang_analyzer_eval(s.check()); // expected-warning{{TRUE}} + clang_analyzer_eval(!s); // expected-warning{{FALSE}} + clang_analyzer_eval(~s); // expected-warning{{FALSE}} + + clang_analyzer_eval(*s == 0); // expected-warning{{TRUE}} #endif } @@ -37,6 +66,12 @@ void testImmediateUse() { #if __cplusplus clang_analyzer_eval((void *)getS().getThis() == (void *)&x); // expected-warning{{FALSE}} + clang_analyzer_eval((void *)+getS() == (void *)&x); // expected-warning{{FALSE}} + clang_analyzer_eval((void *)-getS() == (void *)&x); // expected-warning{{FALSE}} + + clang_analyzer_eval(getS().check()); // expected-warning{{TRUE}} + clang_analyzer_eval(!getS()); // expected-warning{{FALSE}} + clang_analyzer_eval(~getS()); // expected-warning{{FALSE}} #endif } @@ -68,6 +103,13 @@ void testReferenceAssignment() { clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}} clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}} + clang_analyzer_eval(+s == &s); // expected-warning{{TRUE}} + + clang_analyzer_eval(s.check()); // expected-warning{{TRUE}} + clang_analyzer_eval(!s); // expected-warning{{FALSE}} + clang_analyzer_eval(~s); // expected-warning{{FALSE}} + + clang_analyzer_eval(*s == 42); // expected-warning{{TRUE}} } @@ -80,8 +122,39 @@ bool checkThis(const S &s) { return s.getThis() == &s; } +bool checkThisOp(const S &s) { + return +s == &s; +} + +bool checkThisStaticOp(const S &s) { + return -s == &s; +} + void testReferenceArgument() { clang_analyzer_eval(getConstrainedFieldRef(getS()) == 42); // expected-warning{{TRUE}} clang_analyzer_eval(checkThis(getS())); // expected-warning{{TRUE}} + clang_analyzer_eval(checkThisOp(getS())); // expected-warning{{TRUE}} + clang_analyzer_eval(checkThisStaticOp(getS())); // expected-warning{{TRUE}} +} + + +int getConstrainedFieldOp(S s) { + if (*s != 42) return 42; + return *s; +} + +int getConstrainedFieldRefOp(const S &s) { + if (*s != 42) return 42; + return *s; } + +void testImmediateUseOp() { + int x = *getS(); + if (x != 42) return; + clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} + + clang_analyzer_eval(getConstrainedFieldOp(getS()) == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(getConstrainedFieldRefOp(getS()) == 42); // expected-warning{{TRUE}} +} + #endif diff --git a/test/Analysis/fields.c b/test/Analysis/fields.c index da0847a560..12e8bbf367 100644 --- a/test/Analysis/fields.c +++ b/test/Analysis/fields.c @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core %s -analyzer-store=region -verify +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection %s -analyzer-store=region -verify + +void clang_analyzer_eval(int); unsigned foo(); typedef struct bf { unsigned x:2; } bf; @@ -26,3 +28,11 @@ void test() { Point p; (void)(p = getit()).x; } + + +void testLazyCompoundVal() { + Point p = {42, 0}; + Point q; + clang_analyzer_eval((q = p).x == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(q.x == 42); // expected-warning{{TRUE}} +} diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp index 374f3f7261..ce0ee8ed57 100644 --- a/test/Analysis/reference.cpp +++ b/test/Analysis/reference.cpp @@ -116,10 +116,8 @@ void testReferenceAddress(int &x) { struct S { int &x; }; - // FIXME: Should be TRUE. Fields of return-by-value structs are not yet - // symbolicated. Tracked by <rdar://problem/12137950>. extern S getS(); - clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(&getS().x != 0); // expected-warning{{TRUE}} extern S *getSP(); clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}} |