https://github.com/dbartol updated https://github.com/llvm/llvm-project/pull/158112
>From 5beea181cc4c50218bf91a3d47aa13e6ff9c9fa4 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <dave_bartolo...@apple.com> Date: Thu, 11 Sep 2025 12:48:20 -0400 Subject: [PATCH 1/4] [analyzer] Prevent triplicate warnings for sarif-html When `-analyzer-output=sarif-html` is specified, the analyzer was reporting each warning to the console three times. This is because the code to create the diagnostic consumers for `sarif-html` was calling the code for `sarif` and `html` separately, each of which also creates its own console text consumer. Then the `sarif-html` code itself created a third. The fix is to factor out the creation of the SARIF and HTML consumers from the text consumers, so `sarif-html` just calls the code to create the SARIF and HTML consumers without the text consumers. The same fix applies for `plist-html`. I've updated one of the SARIF tests to specify `sarif-html`. This test would fail in the regular `-verify` validation due to the triplicated warnings, but now passes with my fix. --- .../StaticAnalyzer/Core/HTMLDiagnostics.cpp | 50 +++++++++++-------- .../StaticAnalyzer/Core/PlistDiagnostics.cpp | 30 +++++++---- .../StaticAnalyzer/Core/PlistDiagnostics.h | 24 +++++++++ .../StaticAnalyzer/Core/SarifDiagnostics.cpp | 15 +++++- .../StaticAnalyzer/Core/SarifDiagnostics.h | 23 +++++++++ .../sarif-multi-diagnostic-test.c.sarif | 2 +- .../diagnostics/sarif-multi-diagnostic-test.c | 4 +- 7 files changed, 112 insertions(+), 36 deletions(-) create mode 100644 clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h create mode 100644 clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp index 2ef98e17cf9c0..4c9c619f2487a 100644 --- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +#include "PlistDiagnostics.h" +#include "SarifDiagnostics.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/Stmt.h" @@ -169,6 +171,21 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ArrowMap &Indices) { } // namespace +/// Creates and registers an HTML diagnostic consumer, without any additional +/// text consumer. +static void createHTMLDiagnosticConsumerImpl( + PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, + const std::string &OutputDir, const Preprocessor &PP, + bool SupportMultipleFiles) { + + // TODO: Emit an error here. + if (OutputDir.empty()) + return; + + C.emplace_back(std::make_unique<HTMLDiagnostics>( + std::move(DiagOpts), OutputDir, PP, SupportMultipleFiles)); +} + void ento::createHTMLDiagnosticConsumer( PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, const std::string &OutputDir, const Preprocessor &PP, @@ -183,12 +200,8 @@ void ento::createHTMLDiagnosticConsumer( createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU, MacroExpansions); - // TODO: Emit an error here. - if (OutputDir.empty()) - return; - - C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts), - OutputDir, PP, true)); + createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP, + /*SupportMultipleFiles=*/true); } void ento::createHTMLSingleFileDiagnosticConsumer( @@ -199,12 +212,8 @@ void ento::createHTMLSingleFileDiagnosticConsumer( createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU, MacroExpansions); - // TODO: Emit an error here. - if (OutputDir.empty()) - return; - - C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts), - OutputDir, PP, false)); + createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP, + /*SupportMultipleFiles=*/false); } void ento::createPlistHTMLDiagnosticConsumer( @@ -212,11 +221,10 @@ void ento::createPlistHTMLDiagnosticConsumer( const std::string &prefix, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU, const MacroExpansionContext &MacroExpansions) { - createHTMLDiagnosticConsumer( - DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, CTU, - MacroExpansions); - createPlistMultiFileDiagnosticConsumer(DiagOpts, C, prefix, PP, CTU, - MacroExpansions); + createHTMLDiagnosticConsumerImpl( + DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, true); + createPlistDiagnosticConsumerImpl(DiagOpts, C, prefix, PP, CTU, + MacroExpansions, true); createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, prefix, PP, CTU, MacroExpansions); } @@ -226,11 +234,11 @@ void ento::createSarifHTMLDiagnosticConsumer( const std::string &sarif_file, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU, const MacroExpansionContext &MacroExpansions) { - createHTMLDiagnosticConsumer( + createHTMLDiagnosticConsumerImpl( DiagOpts, C, std::string(llvm::sys::path::parent_path(sarif_file)), PP, - CTU, MacroExpansions); - createSarifDiagnosticConsumer(DiagOpts, C, sarif_file, PP, CTU, - MacroExpansions); + true); + createSarifDiagnosticConsumerImpl(DiagOpts, C, sarif_file, PP); + createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, sarif_file, PP, CTU, MacroExpansions); } diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index 771d09e19f178..f4b08e265f9e7 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "PlistDiagnostics.h" #include "clang/Analysis/IssueHash.h" #include "clang/Analysis/MacroExpansionContext.h" #include "clang/Analysis/PathDiagnostic.h" @@ -528,19 +529,31 @@ PlistDiagnostics::PlistDiagnostics( (void)this->CTU; } -void ento::createPlistDiagnosticConsumer( +/// Creates and registers a Plist diagnostic consumer, without any additional +/// text consumer. +void ento::createPlistDiagnosticConsumerImpl( PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, const std::string &OutputFile, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU, - const MacroExpansionContext &MacroExpansions) { + const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles) { // TODO: Emit an error here. if (OutputFile.empty()) return; C.push_back(std::make_unique<PlistDiagnostics>( - DiagOpts, OutputFile, PP, CTU, MacroExpansions, - /*supportsMultipleFiles=*/false)); + DiagOpts, OutputFile, PP, CTU, MacroExpansions, SupportsMultipleFiles)); +} + +void ento::createPlistDiagnosticConsumer( + PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, + const std::string &OutputFile, const Preprocessor &PP, + const cross_tu::CrossTranslationUnitContext &CTU, + const MacroExpansionContext &MacroExpansions) { + + createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU, + MacroExpansions, + /*SupportsMultipleFiles=*/false); createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile, PP, CTU, MacroExpansions); } @@ -551,13 +564,10 @@ void ento::createPlistMultiFileDiagnosticConsumer( const cross_tu::CrossTranslationUnitContext &CTU, const MacroExpansionContext &MacroExpansions) { - // TODO: Emit an error here. - if (OutputFile.empty()) - return; + createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU, + MacroExpansions, + /*SupportsMultipleFiles=*/true); - C.push_back(std::make_unique<PlistDiagnostics>( - DiagOpts, OutputFile, PP, CTU, MacroExpansions, - /*supportsMultipleFiles=*/true)); createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile, PP, CTU, MacroExpansions); } diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h new file mode 100644 index 0000000000000..44586ec75b3de --- /dev/null +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h @@ -0,0 +1,24 @@ +//==- PlistDiagnostics.h - Plist Diagnostics for Paths --*- C++ -*-// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H +#define LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H + +namespace clang { +namespace ento { + +void createPlistDiagnosticConsumerImpl( + PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, + const std::string &Output, const Preprocessor &PP, + const cross_tu::CrossTranslationUnitContext &CTU, + const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles); + +} // namespace ento +} // namespace clang + +#endif diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp index 94b2f486ab7d4..d2c46cf00ac80 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "SarifDiagnostics.h" #include "clang/Analysis/MacroExpansionContext.h" #include "clang/Analysis/PathDiagnostic.h" #include "clang/Basic/Sarif.h" @@ -54,14 +55,24 @@ void ento::createSarifDiagnosticConsumer( const cross_tu::CrossTranslationUnitContext &CTU, const MacroExpansionContext &MacroExpansions) { + createSarifDiagnosticConsumerImpl(DiagOpts, C, Output, PP); + + createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP, + CTU, MacroExpansions); +} + +/// Creates and registers a SARIF diagnostic consumer, without any additional +/// text consumer. +void ento::createSarifDiagnosticConsumerImpl( + PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, + const std::string &Output, const Preprocessor &PP) { + // TODO: Emit an error here. if (Output.empty()) return; C.push_back(std::make_unique<SarifDiagnostics>(Output, PP.getLangOpts(), PP.getSourceManager())); - createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP, - CTU, MacroExpansions); } static StringRef getRuleDescription(StringRef CheckName) { diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h new file mode 100644 index 0000000000000..bfbb995fe5d8f --- /dev/null +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h @@ -0,0 +1,23 @@ +//==- SarifDiagnostics.h - SARIF Diagnostics for Paths --*- C++ -*-// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H +#define LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H + +namespace clang { +namespace ento { + +void createSarifDiagnosticConsumerImpl(PathDiagnosticConsumerOptions DiagOpts, + PathDiagnosticConsumers &C, + const std::string &Output, + const Preprocessor &PP); + +} // namespace ento +} // namespace clang + +#endif diff --git a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif index 7f9deea304832..e35ab695bb38e 100644 --- a/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ b/clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -4,7 +4,7 @@ { "artifacts": [ { - "length": 1071, + "length": 1152, "location": { "index": 0, }, diff --git a/clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c b/clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c index eeafd178628b3..5842574793bce 100644 --- a/clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c +++ b/clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c @@ -1,8 +1,8 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.taint,debug.TaintTest,unix.Malloc %s -verify -analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif - +// RUN: rm -rf %t && mkdir %t && %clang_analyze_cc1 -analyzer-checker=core,optin.taint,debug.TaintTest,unix.Malloc %s -verify -analyzer-output=sarif-html -o %t%{fs-sep}out.sarif +// RUN: cat %t%{fs-sep}out.sarif | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif - #include "../Inputs/system-header-simulator.h" #include "../Inputs/system-header-simulator-for-malloc.h" #define ERR -1 - int atoi(const char *nptr); void f(void) { >From 18b9e09033371c33e5bb22a1da58bff0181673be Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <dave_bartolo...@apple.com> Date: Thu, 11 Sep 2025 13:21:46 -0400 Subject: [PATCH 2/4] Fix build break after reformatting --- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h | 6 ++++++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h index 44586ec75b3de..7df0c2ee21d0b 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h @@ -9,7 +9,13 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H #define LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H +#include "clang/CrossTU/CrossTranslationUnit.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" +#include <string> + namespace clang { + namespace ento { void createPlistDiagnosticConsumerImpl( diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h index bfbb995fe5d8f..8de4a46edf835 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h @@ -9,6 +9,10 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H #define LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H +#include "clang/Lex/Preprocessor.h" +#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" +#include <string> + namespace clang { namespace ento { >From a24dd6b61c93e5aa9c90bf08dfb1502f9b82f67a Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <dave_bartolo...@apple.com> Date: Tue, 16 Sep 2025 11:51:07 -0400 Subject: [PATCH 3/4] Fix file header padding --- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h | 2 +- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h index 7df0c2ee21d0b..5a59180f56913 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h @@ -1,4 +1,4 @@ -//==- PlistDiagnostics.h - Plist Diagnostics for Paths --*- C++ -*-// +//==- PlistDiagnostics.h - Plist Diagnostics for Paths -------------*- C++ -*-// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h index 8de4a46edf835..2538ae8e5f0bd 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h @@ -1,4 +1,4 @@ -//==- SarifDiagnostics.h - SARIF Diagnostics for Paths --*- C++ -*-// +//==- SarifDiagnostics.h - SARIF Diagnostics for Paths -------------*- C++ -*-// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. >From f4bbfbd180fa466a98cdcc537f3f886b13c47d0d Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <dave_bartolo...@apple.com> Date: Tue, 16 Sep 2025 11:53:46 -0400 Subject: [PATCH 4/4] Combine namespace declarations --- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h | 7 ++----- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h | 6 ++---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h index 5a59180f56913..d4ec998ad7d2d 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h @@ -14,9 +14,7 @@ #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" #include <string> -namespace clang { - -namespace ento { +namespace clang::ento { void createPlistDiagnosticConsumerImpl( PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, @@ -24,7 +22,6 @@ void createPlistDiagnosticConsumerImpl( const cross_tu::CrossTranslationUnitContext &CTU, const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles); -} // namespace ento -} // namespace clang +} // namespace clang::ento #endif diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h index 2538ae8e5f0bd..533ee99191926 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h @@ -13,15 +13,13 @@ #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" #include <string> -namespace clang { -namespace ento { +namespace clang::ento { void createSarifDiagnosticConsumerImpl(PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C, const std::string &Output, const Preprocessor &PP); -} // namespace ento -} // namespace clang +} // namespace clang::ento #endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits