From 8e785e214b35145f6e83365c98544bd920d0e39e Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 17 Oct 2012 19:35:44 +0000 Subject: [PATCH] [analyzer] When binding to a ParenExpr, bind to its inner expression instead. This actually looks through several kinds of expression, such as OpaqueValueExpr and ExprWithCleanups. The idea is that binding and lookup should be consistent, and so if the environment needs to be modified later, the code doing the modification will not have to manually look through these "transparent" expressions to find the real binding to change. This is necessary for proper updating of struct rvalues as described in the previous commit. llvm-svn: 166121 --- .../Core/PathSensitive/Environment.h | 7 +- clang/lib/StaticAnalyzer/Core/Environment.cpp | 194 +++++++++++---------- clang/test/Analysis/array-struct-region.cpp | 16 ++ 3 files changed, 125 insertions(+), 92 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h index 54fce65..eb9bd85 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h @@ -33,10 +33,11 @@ class SValBuilder; /// other things. class EnvironmentEntry : public std::pair { + friend class EnvironmentManager; + EnvironmentEntry makeLocation() const; + public: - EnvironmentEntry(const Stmt *s, const LocationContext *L) - : std::pair(s, L ? L->getCurrentStackFrame():0) {} + EnvironmentEntry(const Stmt *s, const LocationContext *L); const Stmt *getStmt() const { return first; } const LocationContext *getLocationContext() const { return second; } diff --git a/clang/lib/StaticAnalyzer/Core/Environment.cpp b/clang/lib/StaticAnalyzer/Core/Environment.cpp index 07659cb..bab89c5 100644 --- a/clang/lib/StaticAnalyzer/Core/Environment.cpp +++ b/clang/lib/StaticAnalyzer/Core/Environment.cpp @@ -20,6 +20,44 @@ using namespace clang; using namespace ento; +static const Expr *ignoreTransparentExprs(const Expr *E) { + E = E->IgnoreParens(); + + switch (E->getStmtClass()) { + case Stmt::OpaqueValueExprClass: + E = cast(E)->getSourceExpr(); + break; + case Stmt::ExprWithCleanupsClass: + E = cast(E)->getSubExpr(); + break; + case Stmt::CXXBindTemporaryExprClass: + E = cast(E)->getSubExpr(); + break; + case Stmt::SubstNonTypeTemplateParmExprClass: + E = cast(E)->getReplacement(); + break; + case Stmt::CXXDefaultArgExprClass: + E = cast(E)->getExpr(); + break; + default: + // This is the base case: we can't look through more than we already have. + return E; + } + + return ignoreTransparentExprs(E); +} + +static const Stmt *ignoreTransparentExprs(const Stmt *S) { + if (const Expr *E = dyn_cast(S)) + return ignoreTransparentExprs(E); + return S; +} + +EnvironmentEntry::EnvironmentEntry(const Stmt *S, const LocationContext *L) + : std::pair(ignoreTransparentExprs(S), + L ? L->getCurrentStackFrame() : 0) {} + SVal Environment::lookupExpr(const EnvironmentEntry &E) const { const SVal* X = ExprBindings.lookup(E); if (X) { @@ -31,93 +69,71 @@ SVal Environment::lookupExpr(const EnvironmentEntry &E) const { SVal Environment::getSVal(const EnvironmentEntry &Entry, SValBuilder& svalBuilder) const { - const Stmt *E = Entry.getStmt(); + const Stmt *S = Entry.getStmt(); const LocationContext *LCtx = Entry.getLocationContext(); - - for (;;) { - if (const Expr *Ex = dyn_cast(E)) - E = Ex->IgnoreParens(); - - switch (E->getStmtClass()) { - case Stmt::AddrLabelExprClass: - return svalBuilder.makeLoc(cast(E)); - case Stmt::OpaqueValueExprClass: { - const OpaqueValueExpr *ope = cast(E); - E = ope->getSourceExpr(); - continue; - } - case Stmt::ParenExprClass: - case Stmt::GenericSelectionExprClass: - llvm_unreachable("ParenExprs and GenericSelectionExprs should " - "have been handled by IgnoreParens()"); - case Stmt::CharacterLiteralClass: { - const CharacterLiteral* C = cast(E); - return svalBuilder.makeIntVal(C->getValue(), C->getType()); - } - case Stmt::CXXBoolLiteralExprClass: { - const SVal *X = ExprBindings.lookup(EnvironmentEntry(E, LCtx)); - if (X) - return *X; - else - return svalBuilder.makeBoolVal(cast(E)); - } - case Stmt::CXXScalarValueInitExprClass: - case Stmt::ImplicitValueInitExprClass: { - QualType Ty = cast(E)->getType(); - return svalBuilder.makeZeroVal(Ty); - } - case Stmt::IntegerLiteralClass: { - // In C++, this expression may have been bound to a temporary object. - SVal const *X = ExprBindings.lookup(EnvironmentEntry(E, LCtx)); - if (X) - return *X; - else - return svalBuilder.makeIntVal(cast(E)); - } - case Stmt::ObjCBoolLiteralExprClass: - return svalBuilder.makeBoolVal(cast(E)); - - // For special C0xx nullptr case, make a null pointer SVal. - case Stmt::CXXNullPtrLiteralExprClass: - return svalBuilder.makeNull(); - case Stmt::ExprWithCleanupsClass: - E = cast(E)->getSubExpr(); - continue; - case Stmt::CXXBindTemporaryExprClass: - E = cast(E)->getSubExpr(); - continue; - case Stmt::SubstNonTypeTemplateParmExprClass: - E = cast(E)->getReplacement(); - continue; - case Stmt::CXXDefaultArgExprClass: - E = cast(E)->getExpr(); - continue; - case Stmt::ObjCStringLiteralClass: { - MemRegionManager &MRMgr = svalBuilder.getRegionManager(); - const ObjCStringLiteral *SL = cast(E); - return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL)); - } - case Stmt::StringLiteralClass: { - MemRegionManager &MRMgr = svalBuilder.getRegionManager(); - const StringLiteral *SL = cast(E); - return svalBuilder.makeLoc(MRMgr.getStringRegion(SL)); - } - case Stmt::ReturnStmtClass: { - const ReturnStmt *RS = cast(E); - if (const Expr *RE = RS->getRetValue()) { - E = RE; - continue; - } - return UndefinedVal(); - } - - // Handle all other Stmt* using a lookup. - default: - break; - }; + + switch (S->getStmtClass()) { + case Stmt::CXXBindTemporaryExprClass: + case Stmt::CXXDefaultArgExprClass: + case Stmt::ExprWithCleanupsClass: + case Stmt::GenericSelectionExprClass: + case Stmt::OpaqueValueExprClass: + case Stmt::ParenExprClass: + case Stmt::SubstNonTypeTemplateParmExprClass: + llvm_unreachable("Should have been handled by ignoreTransparentExprs"); + + case Stmt::AddrLabelExprClass: + return svalBuilder.makeLoc(cast(S)); + + case Stmt::CharacterLiteralClass: { + const CharacterLiteral *C = cast(S); + return svalBuilder.makeIntVal(C->getValue(), C->getType()); + } + + case Stmt::CXXBoolLiteralExprClass: + return svalBuilder.makeBoolVal(cast(S)); + + case Stmt::CXXScalarValueInitExprClass: + case Stmt::ImplicitValueInitExprClass: { + QualType Ty = cast(S)->getType(); + return svalBuilder.makeZeroVal(Ty); + } + + case Stmt::IntegerLiteralClass: + return svalBuilder.makeIntVal(cast(S)); + + case Stmt::ObjCBoolLiteralExprClass: + return svalBuilder.makeBoolVal(cast(S)); + + // For special C0xx nullptr case, make a null pointer SVal. + case Stmt::CXXNullPtrLiteralExprClass: + return svalBuilder.makeNull(); + + case Stmt::ObjCStringLiteralClass: { + MemRegionManager &MRMgr = svalBuilder.getRegionManager(); + const ObjCStringLiteral *SL = cast(S); + return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL)); + } + + case Stmt::StringLiteralClass: { + MemRegionManager &MRMgr = svalBuilder.getRegionManager(); + const StringLiteral *SL = cast(S); + return svalBuilder.makeLoc(MRMgr.getStringRegion(SL)); + } + + case Stmt::ReturnStmtClass: { + const ReturnStmt *RS = cast(S); + if (const Expr *RE = RS->getRetValue()) + return getSVal(EnvironmentEntry(RE, LCtx), svalBuilder); + return UndefinedVal(); + } + + // Handle all other Stmt* using a lookup. + default: break; } - return lookupExpr(EnvironmentEntry(E, LCtx)); + + return lookupExpr(EnvironmentEntry(S, LCtx)); } Environment EnvironmentManager::bindExpr(Environment Env, @@ -133,16 +149,16 @@ Environment EnvironmentManager::bindExpr(Environment Env, return Environment(F.add(Env.ExprBindings, E, V)); } -static inline EnvironmentEntry MakeLocation(const EnvironmentEntry &E) { - const Stmt *S = E.getStmt(); - S = (const Stmt*) (((uintptr_t) S) | 0x1); - return EnvironmentEntry(S, E.getLocationContext()); +EnvironmentEntry EnvironmentEntry::makeLocation() const { + EnvironmentEntry Result = *this; + reinterpret_cast(Result.first) |= 0x1; + return Result; } Environment EnvironmentManager::bindExprAndLocation(Environment Env, const EnvironmentEntry &E, SVal location, SVal V) { - return Environment(F.add(F.add(Env.ExprBindings, MakeLocation(E), location), + return Environment(F.add(F.add(Env.ExprBindings, E.makeLocation(), location), E, V)); } diff --git a/clang/test/Analysis/array-struct-region.cpp b/clang/test/Analysis/array-struct-region.cpp index cffa64d..12ae5d3 100644 --- a/clang/test/Analysis/array-struct-region.cpp +++ b/clang/test/Analysis/array-struct-region.cpp @@ -90,6 +90,22 @@ void testArgument() { clang_analyzer_eval(getAssignedField(getS()) == 42); // expected-warning{{TRUE}} } +void testImmediateUseParens() { + int x = ((getS())).field; + + if (x != 42) return; + clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} + + clang_analyzer_eval(getConstrainedField(((getS()))) == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(getAssignedField(((getS()))) == 42); // expected-warning{{TRUE}} + +#if __cplusplus + clang_analyzer_eval(((getS())).check()); // expected-warning{{TRUE}} + clang_analyzer_eval(!((getS()))); // expected-warning{{FALSE}} + clang_analyzer_eval(~((getS()))); // expected-warning{{FALSE}} +#endif +} + //-------------------- // C++-only tests -- 2.7.4