From 2b97f3cfc8043a7f06799f314a3824738ba24984 Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Tue, 25 Sep 2018 22:10:12 +0000 Subject: [PATCH] [analyzer] NFC: Legalize state manager factory injection. When a checker maintains a program state trait that isn't a simple list/set/map, but is a combination of multiple lists/sets/maps (eg., a multimap - which may be implemented as a map from something to set of something), ProgramStateManager only contains the factory for the trait itself. All auxiliary lists/sets/maps need a factory to be provided by the checker, which is annoying. So far two checkers wanted a multimap, and both decided to trick the ProgramStateManager into keeping the auxiliary factory within itself by pretending that it's some sort of trait they're interested in, but then never using this trait but only using the factory. Make this trick legal. Define a convenient macro. One thing that becomes apparent once all pieces are put together is that these two checkers are in fact using the same factory, because the type that identifies it, ImmutableMap>, is the same. This situation is different from two checkers registering similar primitive traits. Differential Revision: https://reviews.llvm.org/D51388 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@343035 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/CheckerContext.h | 46 -------- .../Core/PathSensitive/ProgramStateTrait.h | 108 ++++++++++++++++-- .../Checkers/CheckObjCDealloc.cpp | 10 +- .../Checkers/InnerPointerChecker.cpp | 15 +-- .../Core/ExprEngineCallAndReturn.cpp | 5 +- 5 files changed, 101 insertions(+), 83 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index e342dd6bb7..6ee75b7384 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -21,52 +21,6 @@ namespace clang { namespace ento { - /// Declares an immutable map of type \p NameTy, suitable for placement into - /// the ProgramState. This is implementing using llvm::ImmutableMap. - /// - /// \code - /// State = State->set(K, V); - /// const Value *V = State->get(K); // Returns NULL if not in the map. - /// State = State->remove(K); - /// NameTy Map = State->get(); - /// \endcode - /// - /// The macro should not be used inside namespaces, or for traits that must - /// be accessible from more than one translation unit. - #define REGISTER_MAP_WITH_PROGRAMSTATE(Name, Key, Value) \ - REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, \ - CLANG_ENTO_PROGRAMSTATE_MAP(Key, Value)) - - /// Declares an immutable set of type \p NameTy, suitable for placement into - /// the ProgramState. This is implementing using llvm::ImmutableSet. - /// - /// \code - /// State = State->add(E); - /// State = State->remove(E); - /// bool Present = State->contains(E); - /// NameTy Set = State->get(); - /// \endcode - /// - /// The macro should not be used inside namespaces, or for traits that must - /// be accessible from more than one translation unit. - #define REGISTER_SET_WITH_PROGRAMSTATE(Name, Elem) \ - REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, llvm::ImmutableSet) - - /// Declares an immutable list of type \p NameTy, suitable for placement into - /// the ProgramState. This is implementing using llvm::ImmutableList. - /// - /// \code - /// State = State->add(E); // Adds to the /end/ of the list. - /// bool Present = State->contains(E); - /// NameTy List = State->get(); - /// \endcode - /// - /// The macro should not be used inside namespaces, or for traits that must - /// be accessible from more than one translation unit. - #define REGISTER_LIST_WITH_PROGRAMSTATE(Name, Elem) \ - REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, llvm::ImmutableList) - - class CheckerContext { ExprEngine &Eng; /// The current exploded(symbolic execution) graph node. diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h index 5555b29253..64de736c7e 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h @@ -30,8 +30,7 @@ namespace ento { /// Declares a program state trait for type \p Type called \p Name, and /// introduce a type named \c NameTy. - /// The macro should not be used inside namespaces, or for traits that must - /// be accessible from more than one translation unit. + /// The macro should not be used inside namespaces. #define REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, Type) \ namespace { \ class Name {}; \ @@ -47,6 +46,102 @@ namespace ento { } \ } + /// Declares a factory for objects of type \p Type in the program state + /// manager. The type must provide a ::Factory sub-class. Commonly used for + /// ImmutableMap, ImmutableSet, ImmutableList. The macro should not be used + /// inside namespaces. + #define REGISTER_FACTORY_WITH_PROGRAMSTATE(Type) \ + namespace clang { \ + namespace ento { \ + template <> \ + struct ProgramStateTrait \ + : public ProgramStatePartialTrait { \ + static void *GDMIndex() { static int Index; return &Index; } \ + }; \ + } \ + } + + /// Helper for registering a map trait. + /// + /// If the map type were written directly in the invocation of + /// REGISTER_TRAIT_WITH_PROGRAMSTATE, the comma in the template arguments + /// would be treated as a macro argument separator, which is wrong. + /// This allows the user to specify a map type in a way that the preprocessor + /// can deal with. + #define CLANG_ENTO_PROGRAMSTATE_MAP(Key, Value) llvm::ImmutableMap + + /// Declares an immutable map of type \p NameTy, suitable for placement into + /// the ProgramState. This is implementing using llvm::ImmutableMap. + /// + /// \code + /// State = State->set(K, V); + /// const Value *V = State->get(K); // Returns NULL if not in the map. + /// State = State->remove(K); + /// NameTy Map = State->get(); + /// \endcode + /// + /// The macro should not be used inside namespaces, or for traits that must + /// be accessible from more than one translation unit. + #define REGISTER_MAP_WITH_PROGRAMSTATE(Name, Key, Value) \ + REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, \ + CLANG_ENTO_PROGRAMSTATE_MAP(Key, Value)) + + /// Declares an immutable map type \p Name and registers the factory + /// for such maps in the program state, but does not add the map itself + /// to the program state. Useful for managing lifetime of maps that are used + /// as elements of other program state data structures. + #define REGISTER_MAP_FACTORY_WITH_PROGRAMSTATE(Name, Key, Value) \ + using Name = llvm::ImmutableMap; \ + REGISTER_FACTORY_WITH_PROGRAMSTATE(Name) + + + /// Declares an immutable set of type \p NameTy, suitable for placement into + /// the ProgramState. This is implementing using llvm::ImmutableSet. + /// + /// \code + /// State = State->add(E); + /// State = State->remove(E); + /// bool Present = State->contains(E); + /// NameTy Set = State->get(); + /// \endcode + /// + /// The macro should not be used inside namespaces, or for traits that must + /// be accessible from more than one translation unit. + #define REGISTER_SET_WITH_PROGRAMSTATE(Name, Elem) \ + REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, llvm::ImmutableSet) + + /// Declares an immutable set type \p Name and registers the factory + /// for such sets in the program state, but does not add the set itself + /// to the program state. Useful for managing lifetime of sets that are used + /// as elements of other program state data structures. + #define REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(Name, Elem) \ + using Name = llvm::ImmutableSet; \ + REGISTER_FACTORY_WITH_PROGRAMSTATE(Name) + + + /// Declares an immutable list type \p NameTy, suitable for placement into + /// the ProgramState. This is implementing using llvm::ImmutableList. + /// + /// \code + /// State = State->add(E); // Adds to the /end/ of the list. + /// bool Present = State->contains(E); + /// NameTy List = State->get(); + /// \endcode + /// + /// The macro should not be used inside namespaces, or for traits that must + /// be accessible from more than one translation unit. + #define REGISTER_LIST_WITH_PROGRAMSTATE(Name, Elem) \ + REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, llvm::ImmutableList) + + /// Declares an immutable list of type \p Name and registers the factory + /// for such lists in the program state, but does not add the list itself + /// to the program state. Useful for managing lifetime of lists that are used + /// as elements of other program state data structures. + #define REGISTER_LIST_FACTORY_WITH_PROGRAMSTATE(Name, Elem) \ + using Name = llvm::ImmutableList; \ + REGISTER_FACTORY_WITH_PROGRAMSTATE(Name) + + // Partial-specialization for ImmutableMap. template struct ProgramStatePartialTrait> { @@ -95,15 +190,6 @@ namespace ento { } }; - /// Helper for registering a map trait. - /// - /// If the map type were written directly in the invocation of - /// REGISTER_TRAIT_WITH_PROGRAMSTATE, the comma in the template arguments - /// would be treated as a macro argument separator, which is wrong. - /// This allows the user to specify a map type in a way that the preprocessor - /// can deal with. - #define CLANG_ENTO_PROGRAMSTATE_MAP(Key, Value) llvm::ImmutableMap - // Partial-specialization for ImmutableSet. template struct ProgramStatePartialTrait> { diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index f4d2e32cef..c330d7504b 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -178,20 +178,12 @@ private: }; } // End anonymous namespace. -typedef llvm::ImmutableSet SymbolSet; /// Maps from the symbol for a class instance to the set of /// symbols remaining that must be released in -dealloc. +REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef) REGISTER_MAP_WITH_PROGRAMSTATE(UnreleasedIvarMap, SymbolRef, SymbolSet) -namespace clang { -namespace ento { -template<> struct ProgramStateTrait -: public ProgramStatePartialTrait { - static void *GDMIndex() { static int index = 0; return &index; } -}; -} -} /// An AST check that diagnose when the class requires a -dealloc method and /// is missing one. diff --git a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index b3638d0b9c..716a89972a 100644 --- a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -25,23 +25,10 @@ using namespace clang; using namespace ento; -using PtrSet = llvm::ImmutableSet; - // Associate container objects with a set of raw pointer symbols. +REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(PtrSet, SymbolRef); REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, PtrSet) -// This is a trick to gain access to PtrSet's Factory. -namespace clang { -namespace ento { -template <> -struct ProgramStateTrait : public ProgramStatePartialTrait { - static void *GDMIndex() { - static int Index = 0; - return &Index; - } -}; -} // end namespace ento -} // end namespace clang namespace { diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 202deb0d4f..66cbebad1c 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -406,9 +406,8 @@ namespace { }; } // end anonymous namespace -REGISTER_TRAIT_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap, - CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, - unsigned)) +REGISTER_MAP_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap, + const MemRegion *, unsigned) bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, -- 2.40.0