Skip to content

Commit

Permalink
[analyzer] Remove overzealous "No dispatcher registered" assertion (#…
Browse files Browse the repository at this point in the history
…107294)

Random testing revealed it's possible to crash the analyzer with the
command line invocation:

clang -cc1 -analyze -analyzer-checker=nullability empty.c

where the source file, empty.c is an empty source file.

```
clang: <root>/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:56:
   void clang::ento::CheckerManager::finishedCheckerRegistration():
     Assertion `Event.second.HasDispatcher && "No dispatcher registered for an event"' failed.

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/

Stack dump:
0.      Program arguments: clang -cc1 -analyze -analyzer-checker=nullability nullability-nocrash.c
 #0 ...
 ...
 #7 <addr> clang::ento::CheckerManager::finishedCheckerRegistration()
 #8 <addr> clang::ento::CheckerManager::CheckerManager(clang::ASTContext&,
             clang::AnalyzerOptions&, clang::Preprocessor const&,
             llvm::ArrayRef<std::__cxx11::basic_string<char, std::char_traits<char>,
             std::allocator<char>>>, llvm::ArrayRef<std::function<void (clang::ento::CheckerRegistry&)>>)
```

This commit removes the assertion which failed here, because it was
logically incorrect: it required that if an Event is handled by some
(enabled) checker, then there must be an **enabled** checker which can
emit that kind of Event. It should be OK to disable the event-producing
checkers but enable an event-consuming checker which has different
responsibilities in addition to handling the events.
 
Note that this assertion was in an `#ifndef NDEBUG` block, so this
change does not impact the non-debug builds.

Co-authored-by: Vince Bridgers <[email protected]>
  • Loading branch information
vabridgers and Vince Bridgers authored Sep 9, 2024
1 parent 04742f3 commit da11ede
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 13 deletions.
2 changes: 0 additions & 2 deletions clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ class CheckerManager {

bool hasPathSensitiveCheckers() const;

void finishedCheckerRegistration();

const LangOptions &getLangOpts() const { return LangOpts; }
const AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
const Preprocessor &getPreprocessor() const {
Expand Down
10 changes: 0 additions & 10 deletions clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ bool CheckerManager::hasPathSensitiveCheckers() const {
EvalCallCheckers, EndOfTranslationUnitCheckers);
}

void CheckerManager::finishedCheckerRegistration() {
#ifndef NDEBUG
// Make sure that for every event that has listeners, there is at least
// one dispatcher registered for it.
for (const auto &Event : Events)
assert(Event.second.HasDispatcher &&
"No dispatcher registered for an event");
#endif
}

void CheckerManager::reportInvalidCheckerOptionValue(
const CheckerBase *C, StringRef OptionName,
StringRef ExpectedValueDesc) const {
Expand Down
1 change: 0 additions & 1 deletion clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ CheckerManager::CheckerManager(
AOptions, checkerRegistrationFns);
Registry.initializeRegistry(*this);
Registry.initializeManager(*this);
finishedCheckerRegistration();
}

CheckerManager::CheckerManager(AnalyzerOptions &AOptions,
Expand Down
13 changes: 13 additions & 0 deletions clang/test/Analysis/nullability-nocrash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=nullability \
// RUN: -analyzer-output=text -verify %s
//
// expected-no-diagnostics
//
// Previously there was an assertion requiring that if an Event is handled by
// some enabled checker, then there must be at least one enabled checker which
// can emit that kind of Event.
// This assertion failed when NullabilityChecker (which is a subclass of
// check::Event<ImplicitNullDerefEvent>) was enabled, but the checkers
// inheriting from EventDispatcher<ImplicitNullDerefEvent> were all disabled.
// This test file validates that enabling the nullability checkers (without any
// other checkers) no longer causes a crash.

0 comments on commit da11ede

Please sign in to comment.