From 5b24adda264feb29c583581f40288eb21c4509d8 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Fri, 3 Sep 2010 01:07:02 +0000 Subject: [PATCH] Add optional record of "location" SVals in the environment. When we analyzing loads/stores, we lose the location SVal, which makes it difficult to recover in some cases (e.g., for post diagnostics). This is prep for pending changes to GRExprEngine. llvm-svn: 112930 --- .../clang/Checker/PathSensitive/Environment.h | 8 +++- .../clang/Checker/PathSensitive/GRState.h | 7 +++ clang/lib/Checker/Environment.cpp | 41 ++++++++++++++++- clang/lib/Checker/GRState.cpp | 46 +++++++++++++++++-- 4 files changed, 96 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Checker/PathSensitive/Environment.h b/clang/include/clang/Checker/PathSensitive/Environment.h index 2981731f863c..611f5079452a 100644 --- a/clang/include/clang/Checker/PathSensitive/Environment.h +++ b/clang/include/clang/Checker/PathSensitive/Environment.h @@ -83,8 +83,14 @@ public: return Environment(F.GetEmptyMap()); } - Environment BindExpr(Environment Env, const Stmt *S, SVal V, + /// Bind the value 'V' to the statement 'S'. + Environment bindExpr(Environment Env, const Stmt *S, SVal V, bool Invalidate); + + /// Bind the location 'location' and value 'V' to the statement 'S'. This + /// is used when simulating loads/stores. + Environment bindExprAndLocation(Environment Env, const Stmt *S, SVal location, + SVal V); Environment RemoveDeadBindings(Environment Env, SymbolReaper &SymReaper, const GRState *ST, diff --git a/clang/include/clang/Checker/PathSensitive/GRState.h b/clang/include/clang/Checker/PathSensitive/GRState.h index 7d998905de41..d72d63ab3c98 100644 --- a/clang/include/clang/Checker/PathSensitive/GRState.h +++ b/clang/include/clang/Checker/PathSensitive/GRState.h @@ -201,8 +201,15 @@ public: const LocationContext *LC, SVal V) const; + /// Create a new state by binding the value 'V' to the statement 'S' in the + /// state's environment. const GRState *BindExpr(const Stmt *S, SVal V, bool Invalidate = true) const; + /// Create a new state by binding the value 'V' and location 'locaton' to the + /// statement 'S' in the state's environment. + const GRState *bindExprAndLocation(const Stmt *S, SVal location, SVal V) + const; + const GRState *bindDecl(const VarRegion *VR, SVal V) const; const GRState *bindDeclWithNoInit(const VarRegion *VR) const; diff --git a/clang/lib/Checker/Environment.cpp b/clang/lib/Checker/Environment.cpp index 48152ceb46f0..02291f43500c 100644 --- a/clang/lib/Checker/Environment.cpp +++ b/clang/lib/Checker/Environment.cpp @@ -80,7 +80,7 @@ SVal Environment::GetSVal(const Stmt *E, ValueManager& ValMgr) const { return LookupExpr(E); } -Environment EnvironmentManager::BindExpr(Environment Env, const Stmt *S, +Environment EnvironmentManager::bindExpr(Environment Env, const Stmt *S, SVal V, bool Invalidate) { assert(S); @@ -94,6 +94,16 @@ Environment EnvironmentManager::BindExpr(Environment Env, const Stmt *S, return Environment(F.Add(Env.ExprBindings, S, V)); } +static inline const Stmt *MakeLocation(const Stmt *S) { + return (const Stmt*) (((uintptr_t) S) | 0x1); +} + +Environment EnvironmentManager::bindExprAndLocation(Environment Env, + const Stmt *S, + SVal location, SVal V) { + return Environment(F.Add(F.Add(Env.ExprBindings, MakeLocation(S), V), S, V)); +} + namespace { class MarkLiveCallback : public SymbolVisitor { SymbolReaper &SymReaper; @@ -115,6 +125,12 @@ static bool isBlockExprInCallers(const Stmt *E, const LocationContext *LC) { return false; } +// In addition to mapping from Stmt * - > SVals in the Environment, we also +// maintain a mapping from Stmt * -> SVals (locations) that were used during +// a load and store. +static inline bool IsLocation(const Stmt *S) { + return (bool) (((uintptr_t) S) & 0x1); +} // RemoveDeadBindings: // - Remove subexpression bindings. @@ -123,7 +139,6 @@ static bool isBlockExprInCallers(const Stmt *E, const LocationContext *LC) { // - Mark their reachable symbols live in SymbolReaper, // see ScanReachableSymbols. // - Mark the region in DRoots if the binding is a loc::MemRegionVal. - Environment EnvironmentManager::RemoveDeadBindings(Environment Env, SymbolReaper &SymReaper, @@ -136,12 +151,25 @@ EnvironmentManager::RemoveDeadBindings(Environment Env, // individually removing all the subexpression bindings (which will greatly // outnumber block-level expression bindings). Environment NewEnv = getInitialEnvironment(); + + llvm::SmallVector, 10> deferredLocations; // Iterate over the block-expr bindings. for (Environment::iterator I = Env.begin(), E = Env.end(); I != E; ++I) { const Stmt *BlkExpr = I.getKey(); + + // For recorded locations (used when evaluating loads and stores), we + // consider them live only when their associated normal expression is + // also live. + // NOTE: This assumes that loads/stores that evaluated to UnknownVal + // still have an entry in the map. + if (IsLocation(BlkExpr)) { + deferredLocations.push_back(std::make_pair(BlkExpr, I.getData())); + continue; + } + const SVal &X = I.getData(); // Block-level expressions in callers are assumed always live. @@ -186,6 +214,15 @@ EnvironmentManager::RemoveDeadBindings(Environment Env, if (X.isUndef() && cast(X).getData()) NewEnv.ExprBindings = F.Add(NewEnv.ExprBindings, BlkExpr, X); } + + // Go through he deferred locations and add them to the new environment if + // the correspond Stmt* is in the map as well. + for (llvm::SmallVectorImpl >::iterator + I = deferredLocations.begin(), E = deferredLocations.end(); I != E; ++I) { + const Stmt *S = (Stmt*) (((uintptr_t) I->first) & (uintptr_t) ~0x1); + if (NewEnv.ExprBindings.lookup(S)) + NewEnv.ExprBindings = F.Add(NewEnv.ExprBindings, I->first, I->second); + } return NewEnv; } diff --git a/clang/lib/Checker/GRState.cpp b/clang/lib/Checker/GRState.cpp index 2a7c6dc66e72..d38ae21fce94 100644 --- a/clang/lib/Checker/GRState.cpp +++ b/clang/lib/Checker/GRState.cpp @@ -206,8 +206,8 @@ SVal GRState::getSimplifiedSVal(Loc location, QualType T) const { return V; } -const GRState *GRState::BindExpr(const Stmt* Ex, SVal V, bool Invalidate) const{ - Environment NewEnv = getStateManager().EnvMgr.BindExpr(Env, Ex, V, +const GRState *GRState::BindExpr(const Stmt* S, SVal V, bool Invalidate) const{ + Environment NewEnv = getStateManager().EnvMgr.bindExpr(Env, S, V, Invalidate); if (NewEnv == Env) return this; @@ -217,6 +217,19 @@ const GRState *GRState::BindExpr(const Stmt* Ex, SVal V, bool Invalidate) const{ return getStateManager().getPersistentState(NewSt); } +const GRState *GRState::bindExprAndLocation(const Stmt *S, SVal location, + SVal V) const { + Environment NewEnv = + getStateManager().EnvMgr.bindExprAndLocation(Env, S, location, V); + + if (NewEnv == Env) + return this; + + GRState NewSt = *this; + NewSt.Env = NewEnv; + return getStateManager().getPersistentState(NewSt); +} + const GRState *GRState::AssumeInBound(DefinedOrUnknownSVal Idx, DefinedOrUnknownSVal UpperBound, bool Assumption) const { @@ -295,6 +308,11 @@ const GRState* GRState::makeWithStore(Store store) const { // State pretty-printing. //===----------------------------------------------------------------------===// +static bool IsEnvLoc(const Stmt *S) { + // FIXME: This is a layering violation. Should be in environment. + return (bool) (((uintptr_t) S) & 0x1); +} + void GRState::print(llvm::raw_ostream& Out, CFG &C, const char* nl, const char* sep) const { // Print the store. @@ -304,8 +322,9 @@ void GRState::print(llvm::raw_ostream& Out, CFG &C, const char* nl, // Print Subexpression bindings. bool isFirst = true; + // FIXME: All environment printing should be moved inside Environment. for (Environment::iterator I = Env.begin(), E = Env.end(); I != E; ++I) { - if (C.isBlkExpr(I.getKey())) + if (C.isBlkExpr(I.getKey()) || IsEnvLoc(I.getKey())) continue; if (isFirst) { @@ -338,6 +357,27 @@ void GRState::print(llvm::raw_ostream& Out, CFG &C, const char* nl, I.getKey()->printPretty(Out, 0, PrintingPolicy(LO)); Out << " : " << I.getData(); } + + // Print locations. + isFirst = true; + + for (Environment::iterator I = Env.begin(), E = Env.end(); I != E; ++I) { + if (!IsEnvLoc(I.getKey())) + continue; + + if (isFirst) { + Out << nl << nl << "Load/store locations:" << nl; + isFirst = false; + } + else { Out << nl; } + + const Stmt *S = (Stmt*) (((uintptr_t) I.getKey()) & ((uintptr_t) ~0x1)); + + Out << " (" << (void*) S << ") "; + LangOptions LO; // FIXME. + S->printPretty(Out, 0, PrintingPolicy(LO)); + Out << " : " << I.getData(); + } Mgr.getConstraintManager().print(this, Out, nl, sep);