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

[Anna Zaks]

On Oct 30, 2012, at 9:18 AM, Jordan Rose wrote:

> 
> 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?

I could not get your suggestion to work as is, so something is missing.

Complicating the macro is not a problem if we can get rid of Name ## Ty.

Anna.

> 
>> 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
>