[cfe-commits] r167000 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

[Jordan Rose]

On Oct 29, 2012, at 21:17 , Anna Zaks <ganna at apple.com> wrote:

> Author: zaks
> Date: Mon Oct 29 23:17:40 2012
> New Revision: 167000
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=167000&view=rev
> Log:
> [analyzer] Fix a bug in REGISTER_MAP_WITH_PROGRAMSTATE
> 
> The ImmutableMap should not be the key into the GDM map as there could
> be several entries with the same map type. Thanks, Jordan.
> 
> This complicates the usage of the macro a bit. When we want to retrieve
> the whole map, we need to use another name. Currently, I set it to be
> Name ## Ty as in "type of the map we are storing in the ProgramState".

Not bad. I wonder if we can do better, though (untested):

class Name : public llvm::ImmutableMap<Key, Value> {
  explicit Name(const ImmutableMap::TreeTy *Root) : ImmutableMap(Root) {}
};

It makes the macro a bit uglier, because of the constructor, but it does reunify the lookup type and the map type. Maybe it's not worth it, though. What do you think?


> Modified:
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
>    cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
> 
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=167000&r1=166999&r2=167000&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Mon Oct 29 23:17:40 2012
> @@ -18,20 +18,21 @@
> #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
> #include "llvm/ADT/ImmutableMap.h"
> 
> -/// Declare an immutable map suitable for placement into the ProgramState.
> -#define REGISTER_MAP_WITH_PROGRAMSTATE(Map, Key, Value) \
> -  typedef llvm::ImmutableMap<Key, Value> Map; \
> +/// Declares an immutable map of type NameTy, suitable for placement into
> +/// the ProgramState. The macro should not be used inside namespaces.
> +#define REGISTER_MAP_WITH_PROGRAMSTATE(Name, Key, Value) \
> +  class Name {}; \
> +  typedef llvm::ImmutableMap<Key, Value> Name ## Ty; \
>   namespace clang { \
>   namespace ento { \
>     template <> \
> -    struct ProgramStateTrait<Map> \
> -      : public ProgramStatePartialTrait<Map> { \
> +    struct ProgramStateTrait<Name> \
> +      : public ProgramStatePartialTrait<Name ## Ty> { \
>       static void *GDMIndex() { static int Index; return &Index; } \
>     }; \
>   } \
>   }
> 
> -
> namespace clang {
> namespace ento {
> 
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp?rev=167000&r1=166999&r2=167000&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp Mon Oct 29 23:17:40 2012
> @@ -106,9 +106,7 @@
>                                        CheckerContext &C) const {
>   initIdentifierInfo(C.getASTContext());
> 
> -  if (C.getCalleeIdentifier(Call) != IIfclose)
> -    return;
> -  if (Call->getNumArgs() != 1)
> +  if (C.getCalleeIdentifier(Call) != IIfclose || Call->getNumArgs() != 1)
>     return;
> 
>   // Get the symbolic value corresponding to the file handle.
> @@ -130,9 +128,9 @@
> void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
>                                            CheckerContext &C) const {
>   ProgramStateRef State = C.getState();
> -  StreamMap TrackedStreams = State->get<StreamMap>();
> +  StreamMapTy TrackedStreams = State->get<StreamMap>();
>   SymbolVector LeakedStreams;
> -  for (StreamMap::iterator I = TrackedStreams.begin(),
> +  for (StreamMapTy::iterator I = TrackedStreams.begin(),
>                            E = TrackedStreams.end(); I != E; ++I) {
>     SymbolRef Sym = I->first;
>     if (SymReaper.isDead(Sym)) {
> @@ -154,9 +152,9 @@
> ProgramStateRef SimpleStreamChecker::evalAssume(ProgramStateRef State,
>                                                 SVal Cond,
>                                                 bool Assumption) const {
> -  StreamMap TrackedStreams = State->get<StreamMap>();
> +  StreamMapTy TrackedStreams = State->get<StreamMap>();
>   SymbolVector LeakedStreams;
> -  for (StreamMap::iterator I = TrackedStreams.begin(),
> +  for (StreamMapTy::iterator I = TrackedStreams.begin(),
>                            E = TrackedStreams.end(); I != E; ++I) {
>     SymbolRef Sym = I->first;
>     if (State->getConstraintManager().isNull(State, Sym).isTrue())
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits