[clang] da11ede - [analyzer] Remove overzealous "No dispatcher registered" assertion (#107294)

[via cfe-commits]

Author: vabridgers
Date: 2024-09-09T03:47:39-05:00
New Revision: da11ede57d034767a6f5d5e211c06c1c3089d7fd

URL: https://github.com/llvm/llvm-project/commit/da11ede57d034767a6f5d5e211c06c1c3089d7fd
DIFF: https://github.com/llvm/llvm-project/commit/da11ede57d034767a6f5d5e211c06c1c3089d7fd.diff

LOG: [analyzer] Remove overzealous "No dispatcher registered" assertion (#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 <vince.a.bridgers at ericsson.com>

Added: 
    clang/test/Analysis/nullability-nocrash.c

Modified: 
    clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
    clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
    clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index ad25d18f280700..24c5b66fd58220 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -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 {

diff  --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 6fc16223ea8287..524a4c43abf243 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -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 {

diff  --git a/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp b/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
index 21a60785eb5253..f60221ad7587e4 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -28,7 +28,6 @@ CheckerManager::CheckerManager(
                            AOptions, checkerRegistrationFns);
   Registry.initializeRegistry(*this);
   Registry.initializeManager(*this);
-  finishedCheckerRegistration();
 }
 
 CheckerManager::CheckerManager(AnalyzerOptions &AOptions,

diff  --git a/clang/test/Analysis/nullability-nocrash.c b/clang/test/Analysis/nullability-nocrash.c
new file mode 100644
index 00000000000000..209b7708250676
--- /dev/null
+++ b/clang/test/Analysis/nullability-nocrash.c
@@ -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.