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

[via cfe-commits]

https://github.com/vabridgers updated https://github.com/llvm/llvm-project/pull/107294

>From d83ef8d13a2a9d242995eec16d82ca2cf08d8700 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridgers at ericsson.com>
Date: Wed, 4 Sep 2024 20:36:06 +0200
Subject: [PATCH] [analyzer] Remove overzealous "No dispatcher registered"
 assertion

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

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

The assert in CheckerManager.cpp was deemed to be too strict so is removed.

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.
---
 .../clang/StaticAnalyzer/Core/CheckerManager.h      |  2 --
 clang/lib/StaticAnalyzer/Core/CheckerManager.cpp    | 10 ----------
 .../Frontend/CreateCheckerManager.cpp               |  1 -
 clang/test/Analysis/nullability-nocrash.c           | 13 +++++++++++++
 4 files changed, 13 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Analysis/nullability-nocrash.c

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.