From 0492ebdda5b955e12a8e74d86166f631499ccebc Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Sat, 26 Jan 2019 20:06:54 +0000 Subject: [analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@352287 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 12 ++++-- .../Checkers/CallAndMessageChecker.cpp | 43 ++++++++++------------ .../Checkers/CheckSecuritySyntaxOnly.cpp | 8 ++++ lib/StaticAnalyzer/Checkers/InterCheckerAPI.h | 3 -- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp | 8 ++++ .../Checkers/IvarInvalidationChecker.cpp | 8 ++++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 36 +++++------------- lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp | 8 ++++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp | 8 ++++ .../RetainCountChecker/RetainCountChecker.cpp | 8 ++++ .../Checkers/StackAddrEscapeChecker.cpp | 8 ++++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp | 8 ++++ 12 files changed, 102 insertions(+), 56 deletions(-) (limited to 'lib/StaticAnalyzer/Checkers') diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 360891d8ec..d37b2c57f5 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -2475,6 +2475,14 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, C.addTransition(state); } +void ento::registerCStringModeling(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterCStringModeling(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ CStringChecker *checker = mgr.registerChecker(); \ @@ -2490,7 +2498,3 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, REGISTER_CHECKER(CStringOutOfBounds) REGISTER_CHECKER(CStringBufferOverlap) REGISTER_CHECKER(CStringNotNullTerm) - - void ento::registerCStringCheckerBasic(CheckerManager &Mgr) { - Mgr.registerChecker(); - } diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index bb7c91cf57..4992ece132 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -28,14 +28,6 @@ using namespace ento; namespace { -struct ChecksFilter { - DefaultBool Check_CallAndMessageUnInitRefArg; - DefaultBool Check_CallAndMessageChecker; - - CheckName CheckName_CallAndMessageUnInitRefArg; - CheckName CheckName_CallAndMessageChecker; -}; - class CallAndMessageChecker : public Checker< check::PreStmt, check::PreStmt, @@ -56,7 +48,8 @@ class CallAndMessageChecker mutable std::unique_ptr BT_call_few_args; public: - ChecksFilter Filter; + DefaultBool Check_CallAndMessageUnInitRefArg; + CheckName CheckName_CallAndMessageUnInitRefArg; void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const; @@ -151,7 +144,7 @@ bool CallAndMessageChecker::uninitRefOrPointer( CheckerContext &C, const SVal &V, SourceRange ArgRange, const Expr *ArgEx, std::unique_ptr &BT, const ParmVarDecl *ParamDecl, const char *BD, int ArgumentNumber) const { - if (!Filter.Check_CallAndMessageUnInitRefArg) + if (!Check_CallAndMessageUnInitRefArg) return false; // No parameter declaration available, i.e. variadic function argument. @@ -607,17 +600,21 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, C.addTransition(state); } -#define REGISTER_CHECKER(name) \ - void ento::register##name(CheckerManager &mgr) { \ - CallAndMessageChecker *Checker = \ - mgr.registerChecker(); \ - Checker->Filter.Check_##name = true; \ - Checker->Filter.CheckName_##name = mgr.getCurrentCheckName(); \ - } \ - \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } +void ento::registerCallAndMessageChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterCallAndMessageChecker(const LangOptions &LO) { + return true; +} -REGISTER_CHECKER(CallAndMessageUnInitRefArg) -REGISTER_CHECKER(CallAndMessageChecker) +void ento::registerCallAndMessageUnInitRefArg(CheckerManager &mgr) { + CallAndMessageChecker *Checker = + mgr.registerChecker(); + Checker->Check_CallAndMessageUnInitRefArg = true; + Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckName(); +} + +bool ento::shouldRegisterCallAndMessageUnInitRefArg(const LangOptions &LO) { + return true; +} diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index f192dba230..383005aa44 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -905,6 +905,14 @@ public: }; } +void ento::registerSecuritySyntaxChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterSecuritySyntaxChecker(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ SecuritySyntaxChecker *checker = \ diff --git a/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h b/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h index 873352b393..9642588d6a 100644 --- a/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h +++ b/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h @@ -16,9 +16,6 @@ class CheckerManager; namespace ento { -/// Register the checker which evaluates CString API calls. -void registerCStringCheckerBasic(CheckerManager &Mgr); - /// Register the part of MallocChecker connected to InnerPointerChecker. void registerInnerPointerCheckerAux(CheckerManager &Mgr); diff --git a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp index 75450271ed..fbabe33595 100644 --- a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -2393,6 +2393,14 @@ bool compare(ProgramStateRef State, NonLoc NL1, NonLoc NL2, } // namespace +void ento::registerIteratorModeling(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterIteratorModeling(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &Mgr) { \ auto *checker = Mgr.registerChecker(); \ diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index 0dc78eb0c7..77ec825798 100644 --- a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -735,6 +735,14 @@ public: }; } // end anonymous namespace +void ento::registerIvarInvalidationModeling(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterIvarInvalidationModeling(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ IvarInvalidationChecker *checker = \ diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 24e34dc381..016aa636f1 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3086,44 +3086,27 @@ markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) { } // end namespace ento } // end namespace clang -void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) { - registerCStringCheckerBasic(mgr); - MallocChecker *checker = mgr.registerChecker(); - checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( - "Optimistic", false, checker); - checker->ChecksEnabled[MallocChecker::CK_NewDeleteLeaksChecker] = true; - checker->CheckNames[MallocChecker::CK_NewDeleteLeaksChecker] = - mgr.getCurrentCheckName(); - // We currently treat NewDeleteLeaks checker as a subchecker of NewDelete - // checker. - if (!checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker]) { - checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker] = true; - // FIXME: This does not set the correct name, but without this workaround - // no name will be set at all. - checker->CheckNames[MallocChecker::CK_NewDeleteChecker] = - mgr.getCurrentCheckName(); - } -} - -bool ento::shouldRegisterNewDeleteLeaksChecker(const LangOptions &LO) { - return true; -} - // Intended to be used in InnerPointerChecker to register the part of // MallocChecker connected to it. void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) { - registerCStringCheckerBasic(mgr); MallocChecker *checker = mgr.registerChecker(); checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( - "Optimistic", false, checker); + "Optimistic", false, checker); checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true; checker->CheckNames[MallocChecker::CK_InnerPointerChecker] = mgr.getCurrentCheckName(); } +void ento::registerDynamicMemoryModeling(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ - registerCStringCheckerBasic(mgr); \ MallocChecker *checker = mgr.registerChecker(); \ checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( \ "Optimistic", false, checker); \ @@ -3137,4 +3120,5 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) { REGISTER_CHECKER(MallocChecker) REGISTER_CHECKER(NewDeleteChecker) +REGISTER_CHECKER(NewDeleteLeaksChecker) REGISTER_CHECKER(MismatchedDeallocatorChecker) diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index d92c0f09cb..3520b12933 100644 --- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -307,6 +307,14 @@ static bool IsCFError(QualType T, IdentifierInfo *II) { return TT->getDecl()->getIdentifier() == II; } +void ento::registerNSOrCFErrorDerefChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterNSOrCFErrorDerefChecker(const LangOptions &LO) { + return true; +} + void ento::registerNSErrorChecker(CheckerManager &mgr) { mgr.registerChecker(); NSOrCFErrorDerefChecker *checker = diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 50f0edd2eb..7d86f67090 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -1191,6 +1191,14 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State, } } +void ento::registerNullabilityBase(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name, trackingRequired) \ void ento::register##name##Checker(CheckerManager &mgr) { \ NullabilityChecker *checker = mgr.registerChecker(); \ diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index e67cdda4df..5d8e7a83e3 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -1455,6 +1455,14 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, // Checker registration. //===----------------------------------------------------------------------===// +void ento::registerRetainCountBase(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterRetainCountBase(const LangOptions &LO) { + return true; +} + void ento::registerRetainCountChecker(CheckerManager &Mgr) { auto *Chk = Mgr.registerChecker(); Chk->TrackObjCAndCFObjects = true; diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index 4a9dbe4650..f6bb7375d1 100644 --- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -359,6 +359,14 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, } } +void ento::registerStackAddrEscapeBase(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterStackAddrEscapeBase(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &Mgr) { \ StackAddrEscapeChecker *Chk = \ diff --git a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp index b47eadba8e..d3eb506312 100644 --- a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -399,6 +399,14 @@ std::shared_ptr ValistChecker::ValistBugVisitor::VisitNode( return std::make_shared(Pos, Msg, true); } +void ento::registerValistBase(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterValistBase(const LangOptions &LO) { + return true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name##Checker(CheckerManager &mgr) { \ ValistChecker *checker = mgr.registerChecker(); \ -- cgit v1.2.1