Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, jfb,
mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity, mgorny.
This patch solves the problem I explained in an earlier revision[1]. It's huge,
I'm still looking for ways to split it up, hence the WIP tag. Also, it fixes a
major bug, but has no tests.
The solution is implemented as follows:
- Add a new field to the `Checker` class in `CheckerBase.td` called
`Dependencies`, which is a list of `Checker`s. I know some of us have mixed
feelings on the use of tblgen, but I personally found it very helpful, because
few things are more miserable then working with the preprocessor, and this
extra complication is actually another point where things can be verified,
properly organized, and so on. The best way for me to prove that it shouldn't
be dropped is actually use it and document it properly, so I did just that in
this patch.
- Move `unix` checkers before `cplusplus`, as there is no forward declaration
in tblgen :/
- Introduce two new checkers: `CStringBase` and `MallocBase` are essentially
checkers without any of their modules/subcheckers enabled. These are important,
as for example `InnerPointerChecker` would like to see `MallocChecker` enabled,
but does not add `CK_MallocChecker` to `MallocChecker::ChecksEnabled` on it's
own. This was weird, so from now on, "barebone" versions of `CStringChecker`
and `MallocChecker` may be enabled via the new checkers. Note that this doesn't
change anything on the user facing interface.
- Move `InnerPointerChecker` to it's own directory, separate the checker class
into a header file, move its registry function to `MallocChecker`.
- Clear up and registry functions in `MallocChecker`, happily remove old FIXMEs.
- Remove `lib/StaticAnalyzer/Checkers/InterCheckerAPI.h`.
- Add a new `addDependency` function to `CheckerRegistry`.
- Neatly format `RUN` lines in files I looked at while debugging.
[1]**(Copied from https://reviews.llvm.org/D54397's summary!)** While on the
quest of fixing checker options the same way I cleaned up non-checker options,
although I'm making good progress, I ran into a wall: In order to emit warnings
on incorrect checker options, we need to ensure that checkers actually acquire
their options properly -- but, I unearthed a huge bug in checker registration:
dependencies are currently implemented in a way that breaks the already very
fragile registration infrastructure.
Here is where the problem really originates from: this is a snippet from
`CheckerRegistry::initializeManager`.
// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
checkerMgr.setCurrentCheckName(CheckName(i->FullName));
i->Initialize(checkerMgr);
}
Note that `Initialize` is a function pointer to `register##CHECKERNAME`, like
`registerInnerPointerChecker`. Since for each register function call the
current checker name is only set once, when `InnerPointerChecker` attempts to
also register it's dependent `MallocChecker`, both it and `MallocChecker` will
receive the `cplusplus.InnerPointer` name. This results in `MallocChecker`
trying to acquire it's `Optimistic` option as
`cplusplus.InnerPointerChecker:Optimistic`.
Clearly, this problem has to be solved at a higher level -- it makes no sense
to be able to affect other checkers in a registry function. Since I'll already
make changes to how checker registration works, I'll probably tune some things
for the current C++ standard, add much needed documentation, and try to make
the code //a lot less confusing//.
Repository:
rC Clang
https://reviews.llvm.org/D54438
Files:
include/clang/StaticAnalyzer/Checkers/CheckerBase.td
include/clang/StaticAnalyzer/Checkers/Checkers.td
include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.cpp
lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.h
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
test/Analysis/NewDelete+MismatchedDeallocator_intersections.cpp
test/Analysis/NewDelete-checker-test.cpp
test/Analysis/inner-pointer.cpp
utils/TableGen/ClangSACheckersEmitter.cpp
Index: utils/TableGen/ClangSACheckersEmitter.cpp
===================================================================
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -57,14 +57,26 @@
return std::string();
}
+static void printChecker(llvm::raw_ostream &OS, const Record &R) {
+ OS << '\"';
+ OS.write_escaped(getCheckerFullName(&R)) << "\", ";
+ OS << R.getName() << ", \"";
+ OS.write_escaped(getStringValue(R, "HelpText")) << '\"';
+}
+
namespace clang {
void EmitClangSACheckers(RecordKeeper &Records, raw_ostream &OS) {
std::vector<Record*> checkers = Records.getAllDerivedDefinitions("Checker");
std::vector<Record*> packages = Records.getAllDerivedDefinitions("Package");
using SortedRecords = llvm::StringMap<const Record *>;
- OS << "\n#ifdef GET_PACKAGES\n";
+ // Emit packages.
+ //
+ // PACKAGE(PACKAGENAME)
+ // - PACKAGENAME: The name of the package.
+ OS << "\n"
+ "#ifdef GET_PACKAGES\n";
{
SortedRecords sortedPackages;
for (unsigned i = 0, e = packages.size(); i != e; ++i)
@@ -79,19 +91,51 @@
OS << ")\n";
}
}
- OS << "#endif // GET_PACKAGES\n\n";
-
- OS << "\n#ifdef GET_CHECKERS\n";
- for (unsigned i = 0, e = checkers.size(); i != e; ++i) {
- const Record &R = *checkers[i];
+ OS << "#endif // GET_PACKAGES\n"
+ "\n";
- OS << "CHECKER(" << "\"";
- OS.write_escaped(getCheckerFullName(&R)) << "\", ";
- OS << R.getName() << ", ";
- OS << "\"";
- OS.write_escaped(getStringValue(R, "HelpText")) << '\"';
+ // Emit checkers.
+ //
+ // CHECKER(FULLNAME, CLASS, HELPTEXT)
+ // - FULLNAME: The full name of the checker, including packages, e.g.:
+ // alpha.cplusplus.UninitializedObject
+ // - CLASS: The name of the checker, with "Checker" appended, e.g.:
+ // UninitializedObjectChecker
+ // - HELPTEXT: The description of the checker.
+ OS << "\n"
+ "#ifdef GET_CHECKERS\n"
+ "\n";
+ for (const Record *checker : checkers) {
+ OS << "CHECKER(";
+ printChecker(OS, *checker);
OS << ")\n";
}
- OS << "#endif // GET_CHECKERS\n\n";
+ OS << "\n"
+ "#endif // GET_CHECKERS\n"
+ "\n";
+
+ // Emit dependencies.
+ //
+ // CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)
+ // - FULLNAME: The full name of the checker that depends on another checker.
+ // - DEPENDENCY: The full name of the checker FULLNAME depends on.
+ OS << "\n"
+ "#ifdef GET_CHECKER_DEPENDENCIES\n";
+ for (const Record *checker : checkers) {
+ if (checker->isValueUnset("Dependencies"))
+ continue;
+
+ for (const Record *Dependency :
+ checker->getValueAsListOfDefs("Dependencies")) {
+ OS << "CHECKER_DEPENDENCY(";
+ OS << '\"';
+ OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+ OS << '\"';
+ OS.write_escaped(getCheckerFullName(Dependency)) << '\"';
+ OS << ")\n";
+ }
+ }
+ OS << "\n"
+ "#endif // GET_CHECKER_DEPENDENCIES\n";
}
} // end namespace clang
Index: test/Analysis/inner-pointer.cpp
===================================================================
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -1,4 +1,5 @@
-//RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer %s -analyzer-output=text -verify
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.InnerPointer \
+// RUN: -analyzer-output=text -verify %s -analyzer-list-enabled-checkers
namespace std {
Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -1,11 +1,42 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete
+//
+// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
+//
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-config c++-allocator-inlining=true
+//
+// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \
+// RUN: -analyzer-config c++-allocator-inlining=true
+//
+// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \
+// RUN: -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete
+//
+// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \
+// RUN: -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
+//
+// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \
+// RUN: -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-config c++-allocator-inlining=true
+//
+// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \
+// RUN: -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \
+// RUN: -analyzer-config c++-allocator-inlining=true
#include "Inputs/system-header-simulator-cxx.h"
Index: test/Analysis/NewDelete+MismatchedDeallocator_intersections.cpp
===================================================================
--- test/Analysis/NewDelete+MismatchedDeallocator_intersections.cpp
+++ test/Analysis/NewDelete+MismatchedDeallocator_intersections.cpp
@@ -1,5 +1,14 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.MismatchedDeallocator -std=c++11 -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.MismatchedDeallocator -DLEAKS -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-checker=unix.MismatchedDeallocator
+//
+// RUN: %clang_analyze_cc1 -std=c++11 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \
+// RUN: -analyzer-checker=unix.MismatchedDeallocator
+
// expected-no-diagnostics
typedef __typeof(sizeof(int)) size_t;
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -38,11 +38,18 @@
return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
}
+static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
+ const CheckerRegistry::CheckerInfo &b) {
+ return a.FullName < b.FullName;
+}
+
CheckerRegistry::CheckerRegistry(ArrayRef<std::string> plugins,
DiagnosticsEngine &diags) : Diags(diags) {
#define GET_CHECKERS
+
#define CHECKER(FULLNAME, CLASS, HELPTEXT) \
addChecker(register##CLASS, FULLNAME, HELPTEXT);
+
#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
#undef CHECKER
#undef GET_CHECKERS
@@ -76,15 +83,21 @@
if (registerPluginCheckers)
registerPluginCheckers(*this);
}
-}
-static constexpr char PackageSeparator = '.';
+ llvm::sort(Checkers, checkerNameLT);
-static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
- const CheckerRegistry::CheckerInfo &b) {
- return a.FullName < b.FullName;
+#define GET_CHECKER_DEPENDENCIES
+
+#define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY) \
+ addDependency(FULLNAME, DEPENDENCY);
+
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER_DEPENDENCY
+#undef GET_CHECKER_DEPENDENCIES
}
+static constexpr char PackageSeparator = '.';
+
static bool isInPackage(const CheckerRegistry::CheckerInfo &checker,
StringRef packageName) {
// Does the checker's full name have the package as a prefix?
@@ -102,9 +115,17 @@
return false;
}
+static void collectDependencies(
+ CheckerRegistry::CheckerInfoSet &enabledCheckers,
+ const llvm::SmallVector<const CheckerRegistry::CheckerInfo *, 0> &Deps) {
+ for (const CheckerRegistry::CheckerInfo *Dependency : Deps) {
+ enabledCheckers.insert(Dependency);
+ collectDependencies(enabledCheckers, Dependency->Dependencies);
+ }
+}
+
CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers(
const AnalyzerOptions &Opts) const {
-
assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
"In order to efficiently gather checkers, this function expects them "
"to be already sorted!");
@@ -114,7 +135,7 @@
for (const std::pair<std::string, bool> &opt : Opts.CheckersControlList) {
// Use a binary search to find the possible start of the package.
- CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.first, "");
+ CheckerInfo packageInfo(nullptr, opt.first, "");
auto firstRelatedChecker =
std::lower_bound(Checkers.cbegin(), end, packageInfo, checkerNameLT);
@@ -135,11 +156,18 @@
// Step through all the checkers in the package.
for (auto lastRelatedChecker = firstRelatedChecker+size;
- firstRelatedChecker != lastRelatedChecker; ++firstRelatedChecker)
- if (opt.second)
+ firstRelatedChecker != lastRelatedChecker; ++firstRelatedChecker) {
+ if (opt.second) {
+ // Enable the checker.
enabledCheckers.insert(&*firstRelatedChecker);
- else
+
+ // Recursively enable it's dependencies.
+ collectDependencies(enabledCheckers, firstRelatedChecker->Dependencies);
+ } else {
+ // Disable the checker.
enabledCheckers.remove(&*firstRelatedChecker);
+ }
+ }
}
return enabledCheckers;
@@ -160,9 +188,6 @@
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
const AnalyzerOptions &Opts) const {
- // Sort checkers for efficient collection.
- llvm::sort(Checkers, checkerNameLT);
-
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers = getEnabledCheckers(Opts);
@@ -199,8 +224,6 @@
// FIXME: Alphabetical sort puts 'experimental' in the middle.
// Would it be better to name it '~experimental' or something else
// that's ASCIIbetically last?
- llvm::sort(Checkers, checkerNameLT);
-
// FIXME: Print available packages.
out << "CHECKERS:\n";
@@ -234,9 +257,6 @@
void CheckerRegistry::printList(raw_ostream &out,
const AnalyzerOptions &opts) const {
- // Sort checkers for efficient collection.
- llvm::sort(Checkers, checkerNameLT);
-
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers = getEnabledCheckers(opts);
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -13,8 +13,6 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include "InterCheckerAPI.h"
-#include "clang/AST/Attr.h"
#include "clang/AST/ParentMap.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
@@ -31,6 +29,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "AllocationState.h"
+#include "InnerPointer/InnerPointerChecker.h"
#include <climits>
#include <utility>
@@ -190,6 +189,7 @@
/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
enum CheckKind {
+ CK_MallocBase,
CK_MallocChecker,
CK_NewDeleteChecker,
CK_NewDeleteLeaksChecker,
@@ -3081,47 +3081,29 @@
} // end namespace ento
} // end namespace clang
-void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
- registerCStringCheckerBasic(mgr);
- MallocChecker *checker = mgr.registerChecker<MallocChecker>();
- 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();
- }
-}
-
-// 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<MallocChecker>();
- checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
- "Optimistic", false, checker);
- checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
- checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
- mgr.getCurrentCheckName();
-}
-
#define REGISTER_CHECKER(name) \
void ento::register##name(CheckerManager &mgr) { \
- registerCStringCheckerBasic(mgr); \
MallocChecker *checker = mgr.registerChecker<MallocChecker>(); \
checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( \
"Optimistic", false, checker); \
checker->ChecksEnabled[MallocChecker::CK_##name] = true; \
checker->CheckNames[MallocChecker::CK_##name] = mgr.getCurrentCheckName(); \
}
+REGISTER_CHECKER(MallocBase)
REGISTER_CHECKER(MallocChecker)
REGISTER_CHECKER(NewDeleteChecker)
+REGISTER_CHECKER(NewDeleteLeaksChecker)
REGISTER_CHECKER(MismatchedDeallocatorChecker)
+
+void ento::registerInnerPointerChecker(CheckerManager &mgr) {
+ // This checker first registers itself as well, which is why this registry
+ // function isn't generated.
+ mgr.registerChecker<InnerPointerChecker>();
+ MallocChecker *checker = mgr.registerChecker<MallocChecker>();
+ checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ "Optimistic", false, checker);
+ checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
+ checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
+ mgr.getCurrentCheckName();
+}
Index: lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
===================================================================
--- lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
+++ /dev/null
@@ -1,27 +0,0 @@
-//==--- InterCheckerAPI.h ---------------------------------------*- C++ -*-==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-// This file allows introduction of checker dependencies. It contains APIs for
-// inter-checker communications.
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
-#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
-namespace clang {
-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);
-
-}}
-#endif /* INTERCHECKERAPI_H_ */
Index: lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.h
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.h
@@ -0,0 +1,70 @@
+//=== InnerPointerChecker.cpp -------------------------------------*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// For a documentation, refer to InnerPointerChecker.cpp.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+
+namespace clang {
+namespace ento {
+
+class InnerPointerChecker
+ : public Checker<check::DeadSymbols, check::PostCall> {
+
+ CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
+ InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
+ ShrinkToFitFn, SwapFn;
+
+public:
+ InnerPointerChecker()
+ : AppendFn({"std", "basic_string", "append"}),
+ AssignFn({"std", "basic_string", "assign"}),
+ ClearFn({"std", "basic_string", "clear"}),
+ CStrFn({"std", "basic_string", "c_str"}),
+ DataFn({"std", "basic_string", "data"}),
+ EraseFn({"std", "basic_string", "erase"}),
+ InsertFn({"std", "basic_string", "insert"}),
+ PopBackFn({"std", "basic_string", "pop_back"}),
+ PushBackFn({"std", "basic_string", "push_back"}),
+ ReplaceFn({"std", "basic_string", "replace"}),
+ ReserveFn({"std", "basic_string", "reserve"}),
+ ResizeFn({"std", "basic_string", "resize"}),
+ ShrinkToFitFn({"std", "basic_string", "shrink_to_fit"}),
+ SwapFn({"std", "basic_string", "swap"}) {}
+
+ /// Check whether the called member function potentially invalidates
+ /// pointers referring to the container object's inner buffer.
+ bool isInvalidatingMemberFunction(const CallEvent &Call) const;
+
+ /// Mark pointer symbols associated with the given memory region released
+ /// in the program state.
+ void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
+ const MemRegion *ObjRegion,
+ CheckerContext &C) const;
+
+ /// Standard library functions that take a non-const `basic_string` argument by
+ /// reference may invalidate its inner pointers. Check for these cases and
+ /// mark the pointers released.
+ void checkFunctionArguments(const CallEvent &Call, ProgramStateRef State,
+ CheckerContext &C) const;
+
+ /// Record the connection between raw pointers referring to a container
+ /// object's inner buffer and the object's memory region in the program state.
+ /// Mark potentially invalidated pointers released.
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+ /// Clean up the program state map.
+ void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+};
+
+} // end of namespace ento
+} // end of namespace clang
Index: lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.cpp
@@ -13,10 +13,9 @@
//
//===----------------------------------------------------------------------===//
-#include "AllocationState.h"
+#include "InnerPointerChecker.h"
+#include "../AllocationState.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include "InterCheckerAPI.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -32,84 +31,35 @@
namespace {
-class InnerPointerChecker
- : public Checker<check::DeadSymbols, check::PostCall> {
-
- CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
- InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
- ShrinkToFitFn, SwapFn;
+class InnerPointerBRVisitor : public BugReporterVisitor {
+ SymbolRef PtrToBuf;
public:
- class InnerPointerBRVisitor : public BugReporterVisitor {
- SymbolRef PtrToBuf;
-
- public:
- InnerPointerBRVisitor(SymbolRef Sym) : PtrToBuf(Sym) {}
+ InnerPointerBRVisitor(SymbolRef Sym) : PtrToBuf(Sym) {}
- static void *getTag() {
- static int Tag = 0;
- return &Tag;
- }
+ static void *getTag() {
+ static int Tag = 0;
+ return &Tag;
+ }
- void Profile(llvm::FoldingSetNodeID &ID) const override {
- ID.AddPointer(getTag());
- }
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(getTag());
+ }
- virtual std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
- BugReporterContext &BRC,
- BugReport &BR) override;
-
- // FIXME: Scan the map once in the visitor's constructor and do a direct
- // lookup by region.
- bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
- RawPtrMapTy Map = State->get<RawPtrMap>();
- for (const auto Entry : Map) {
- if (Entry.second.contains(Sym))
- return true;
- }
- return false;
+ virtual std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+
+ // FIXME: Scan the map once in the visitor's constructor and do a direct
+ // lookup by region.
+ bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
+ RawPtrMapTy Map = State->get<RawPtrMap>();
+ for (const auto Entry : Map) {
+ if (Entry.second.contains(Sym))
+ return true;
}
- };
-
- InnerPointerChecker()
- : AppendFn({"std", "basic_string", "append"}),
- AssignFn({"std", "basic_string", "assign"}),
- ClearFn({"std", "basic_string", "clear"}),
- CStrFn({"std", "basic_string", "c_str"}),
- DataFn({"std", "basic_string", "data"}),
- EraseFn({"std", "basic_string", "erase"}),
- InsertFn({"std", "basic_string", "insert"}),
- PopBackFn({"std", "basic_string", "pop_back"}),
- PushBackFn({"std", "basic_string", "push_back"}),
- ReplaceFn({"std", "basic_string", "replace"}),
- ReserveFn({"std", "basic_string", "reserve"}),
- ResizeFn({"std", "basic_string", "resize"}),
- ShrinkToFitFn({"std", "basic_string", "shrink_to_fit"}),
- SwapFn({"std", "basic_string", "swap"}) {}
-
- /// Check whether the called member function potentially invalidates
- /// pointers referring to the container object's inner buffer.
- bool isInvalidatingMemberFunction(const CallEvent &Call) const;
-
- /// Mark pointer symbols associated with the given memory region released
- /// in the program state.
- void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
- const MemRegion *ObjRegion,
- CheckerContext &C) const;
-
- /// Standard library functions that take a non-const `basic_string` argument by
- /// reference may invalidate its inner pointers. Check for these cases and
- /// mark the pointers released.
- void checkFunctionArguments(const CallEvent &Call, ProgramStateRef State,
- CheckerContext &C) const;
-
- /// Record the connection between raw pointers referring to a container
- /// object's inner buffer and the object's memory region in the program state.
- /// Mark potentially invalidated pointers released.
- void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-
- /// Clean up the program state map.
- void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ return false;
+ }
};
} // end anonymous namespace
@@ -262,7 +212,7 @@
namespace allocation_state {
std::unique_ptr<BugReporterVisitor> getInnerPointerBRVisitor(SymbolRef Sym) {
- return llvm::make_unique<InnerPointerChecker::InnerPointerBRVisitor>(Sym);
+ return llvm::make_unique<InnerPointerBRVisitor>(Sym);
}
const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym) {
@@ -280,9 +230,8 @@
} // end namespace clang
std::shared_ptr<PathDiagnosticPiece>
-InnerPointerChecker::InnerPointerBRVisitor::VisitNode(const ExplodedNode *N,
- BugReporterContext &BRC,
- BugReport &) {
+InnerPointerBRVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+ BugReport &) {
if (!isSymbolTracked(N->getState(), PtrToBuf) ||
isSymbolTracked(N->getFirstPred()->getState(), PtrToBuf))
return nullptr;
@@ -306,7 +255,5 @@
nullptr);
}
-void ento::registerInnerPointerChecker(CheckerManager &Mgr) {
- registerInnerPointerCheckerAux(Mgr);
- Mgr.registerChecker<InnerPointerChecker>();
-}
+// The register function for this checker is in MallocChecker.cpp, as it's
+// closely tied to it.
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -13,7 +13,6 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include "InterCheckerAPI.h"
#include "clang/Basic/CharInfo.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -2419,6 +2418,6 @@
REGISTER_CHECKER(CStringBufferOverlap)
REGISTER_CHECKER(CStringNotNullTerm)
- void ento::registerCStringCheckerBasic(CheckerManager &Mgr) {
- Mgr.registerChecker<CStringChecker>();
- }
+void ento::registerCStringBase(CheckerManager &Mgr) {
+ Mgr.registerChecker<CStringChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -39,7 +39,7 @@
GenericTaintChecker.cpp
GTestChecker.cpp
IdenticalExprChecker.cpp
- InnerPointerChecker.cpp
+ InnerPointer/InnerPointerChecker.cpp
IteratorChecker.cpp
IvarInvalidationChecker.cpp
LLVMConventionsChecker.cpp
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -91,6 +91,7 @@
InitializationFunction Initialize;
StringRef FullName;
StringRef Desc;
+ llvm::SmallVector<const CheckerInfo *, 0> Dependencies;
CheckerInfo(InitializationFunction fn, StringRef name, StringRef desc)
: Initialize(fn), FullName(name), Desc(desc) {}
@@ -120,6 +121,28 @@
addChecker(&CheckerRegistry::initializeManager<T>, fullName, desc);
}
+ /// Makes the checker with the full name \p fullName depends on the checker
+ /// called \p dependency.
+ void addDependency(StringRef fullName, StringRef dependency) {
+ auto CheckerThatNeedsDeps =
+ [&fullName](const CheckerInfo &Chk) { return Chk.FullName == fullName; };
+ auto Dependency =
+ [&dependency](const CheckerInfo &Chk) {
+ return Chk.FullName == dependency;
+ };
+
+ auto CheckerIt = llvm::find_if(Checkers, CheckerThatNeedsDeps);
+ assert(CheckerIt != Checkers.end() &&
+ "Failed to find the checker while attempting to set up it's "
+ "dependencies!");
+
+ auto DependencyIt = llvm::find_if(Checkers, Dependency);
+ assert(DependencyIt != Checkers.end() &&
+ "Failed to find the dependency of a checker!");
+
+ CheckerIt->Dependencies.push_back(&*DependencyIt);
+ }
+
/// Initializes a CheckerManager by calling the initialization functions for
/// all checkers specified by the given CheckerOptInfo list. The order of this
/// list is significant; later options can be used to reverse earlier ones.
@@ -138,8 +161,9 @@
private:
CheckerInfoSet getEnabledCheckers(const AnalyzerOptions &Opts) const;
- mutable CheckerInfoList Checkers;
- mutable llvm::StringMap<size_t> Packages;
+ CheckerInfoList Checkers;
+ llvm::StringMap<size_t> Packages;
+
DiagnosticsEngine &Diags;
};
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -254,22 +254,111 @@
} // end "core.uninitialized"
+//===----------------------------------------------------------------------===//
+// Unix API checkers.
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = UnixAlpha in {
+
+def ChrootChecker : Checker<"Chroot">,
+ HelpText<"Check improper use of chroot">;
+
+def PthreadLockChecker : Checker<"PthreadLock">,
+ HelpText<"Simple lock -> unlock checker">;
+
+def StreamChecker : Checker<"Stream">,
+ HelpText<"Check stream handling functions">;
+
+def SimpleStreamChecker : Checker<"SimpleStream">,
+ HelpText<"Check for misuses of stream APIs">;
+
+def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
+ HelpText<"Check for calls to blocking functions inside a critical section">;
+
+} // end "alpha.unix"
+
+let ParentPackage = CString in {
+
+def CStringBase : Checker<"CStringBase">,
+ HelpText<"The base of several CString related checkers. On it's own it emits "
+ "no reports, but adds valuable information to the analysis when "
+ "enabled.">;
+
+def CStringNullArg : Checker<"NullArg">,
+ HelpText<"Check for null pointers being passed as arguments to C string "
+ "functions">,
+ Dependencies<[CStringBase]>;
+
+def CStringSyntaxChecker : Checker<"BadSizeArg">,
+ HelpText<"Check the size argument passed into C string functions for common "
+ "erroneous patterns">,
+ Dependencies<[CStringBase]>;
+
+} // end "unix.cstring"
+
+let ParentPackage = CStringAlpha in {
+
+def CStringOutOfBounds : Checker<"OutOfBounds">,
+ HelpText<"Check for out-of-bounds access in string functions">,
+ Dependencies<[CStringBase]>;
+
+def CStringBufferOverlap : Checker<"BufferOverlap">,
+ HelpText<"Checks for overlap in two buffer arguments">,
+ Dependencies<[CStringBase]>;
+
+def CStringNotNullTerm : Checker<"NotNullTerminated">,
+ HelpText<"Check for arguments which are not null-terminating strings">,
+ Dependencies<[CStringBase]>;
+
+} // end "alpha.unix.cstring"
+
+let ParentPackage = Unix in {
+
+def UnixAPIMisuseChecker : Checker<"API">,
+ HelpText<"Check calls to various UNIX/Posix functions">;
+
+def MallocBase: Checker<"MallocBase">,
+ HelpText<"The base of several malloc() related checkers. On it's own it "
+ "emits no reports, but adds valuable information to the analysis "
+ "when enabled.">,
+ Dependencies<[CStringBase]>;
+
+def MallocChecker: Checker<"Malloc">,
+ HelpText<"Check for memory leaks, double free, and use-after-free problems. "
+ "Traces memory managed by malloc()/free().">,
+ Dependencies<[MallocBase]>;
+
+def MallocSizeofChecker : Checker<"MallocSizeof">,
+ HelpText<"Check for dubious malloc arguments involving sizeof">;
+
+def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">,
+ HelpText<"Check for mismatched deallocators.">,
+ Dependencies<[MallocBase]>;
+
+def VforkChecker : Checker<"Vfork">,
+ HelpText<"Check for proper usage of vfork">;
+
+} // end "unix"
+
//===----------------------------------------------------------------------===//
// C++ checkers.
//===----------------------------------------------------------------------===//
let ParentPackage = Cplusplus in {
-def InnerPointerChecker : Checker<"InnerPointer">,
- HelpText<"Check for inner pointers of C++ containers used after "
- "re/deallocation">;
-
def NewDeleteChecker : Checker<"NewDelete">,
HelpText<"Check for double-free and use-after-free problems. Traces memory "
- "managed by new/delete.">;
+ "managed by new/delete.">,
+ Dependencies<[MallocBase]>;
+
+def InnerPointerChecker : Checker<"InnerPointer">,
+ HelpText<"Check for inner pointers of C++ containers used after "
+ "re/deallocation">,
+ Dependencies<[MallocBase]>;
def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
- HelpText<"Check for memory leaks. Traces memory managed by new/delete.">;
+ HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
+ Dependencies<[NewDeleteChecker]>;
def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,
HelpText<"Checks C++ copy and move assignment operators for self assignment">;
@@ -430,74 +519,6 @@
} // end "alpha.security.taint"
-//===----------------------------------------------------------------------===//
-// Unix API checkers.
-//===----------------------------------------------------------------------===//
-
-let ParentPackage = Unix in {
-
-def UnixAPIMisuseChecker : Checker<"API">,
- HelpText<"Check calls to various UNIX/Posix functions">;
-
-def MallocChecker: Checker<"Malloc">,
- HelpText<"Check for memory leaks, double free, and use-after-free problems. "
- "Traces memory managed by malloc()/free().">;
-
-def MallocSizeofChecker : Checker<"MallocSizeof">,
- HelpText<"Check for dubious malloc arguments involving sizeof">;
-
-def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">,
- HelpText<"Check for mismatched deallocators.">;
-
-def VforkChecker : Checker<"Vfork">,
- HelpText<"Check for proper usage of vfork">;
-
-} // end "unix"
-
-let ParentPackage = UnixAlpha in {
-
-def ChrootChecker : Checker<"Chroot">,
- HelpText<"Check improper use of chroot">;
-
-def PthreadLockChecker : Checker<"PthreadLock">,
- HelpText<"Simple lock -> unlock checker">;
-
-def StreamChecker : Checker<"Stream">,
- HelpText<"Check stream handling functions">;
-
-def SimpleStreamChecker : Checker<"SimpleStream">,
- HelpText<"Check for misuses of stream APIs">;
-
-def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
- HelpText<"Check for calls to blocking functions inside a critical section">;
-
-} // end "alpha.unix"
-
-let ParentPackage = CString in {
-
-def CStringNullArg : Checker<"NullArg">,
- HelpText<"Check for null pointers being passed as arguments to C string "
- "functions">;
-
-def CStringSyntaxChecker : Checker<"BadSizeArg">,
- HelpText<"Check the size argument passed into C string functions for common "
- "erroneous patterns">;
-
-} // end "unix.cstring"
-
-let ParentPackage = CStringAlpha in {
-
-def CStringOutOfBounds : Checker<"OutOfBounds">,
- HelpText<"Check for out-of-bounds access in string functions">;
-
-def CStringBufferOverlap : Checker<"BufferOverlap">,
- HelpText<"Checks for overlap in two buffer arguments">;
-
-def CStringNotNullTerm : Checker<"NotNullTerminated">,
- HelpText<"Check for arguments which are not null-terminating strings">;
-
-} // end "alpha.unix.cstring"
-
//===----------------------------------------------------------------------===//
// Mac OS X, Cocoa, and Core Foundation checkers.
//===----------------------------------------------------------------------===//
Index: include/clang/StaticAnalyzer/Checkers/CheckerBase.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/CheckerBase.td
+++ include/clang/StaticAnalyzer/Checkers/CheckerBase.td
@@ -37,8 +37,23 @@
/// Note that a checker has a name (e.g.: 'NullDereference'), and a fullname,
/// that is autogenerated with the help of the ParentPackage field, that also
/// includes package names (e.g.: 'core.NullDereference').
+/// Example:
+/// def DereferenceChecker : Checker<"NullDereference">,
+/// HelpText<"Check for dereferences of null pointers">;
class Checker<string name = ""> {
- string CheckerName = name;
- string HelpText;
- Package ParentPackage;
+ string CheckerName = name;
+ string HelpText;
+ Package ParentPackage;
+ list<Checker> Dependencies;
+}
+
+/// Describes dependencies in between checkers. For example, InnerPointerChecker
+/// relies on information MallocBase gathers.
+/// Example:
+/// def InnerPointerChecker : Checker<"InnerPointer">,
+/// HelpText<"Check for inner pointers of C++ containers used after "
+/// "re/deallocation">,
+/// Dependencies<[MallocBase]>;
+class Dependencies<list<Checker> Deps = []> {
+ list<Checker> Dependencies = Deps;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
