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.bridg...@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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
