[analyzer] Prevent triplicate warnings for `sarif-html` #158112

dbartol · 2025-09-11T16:36:10Z

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.

Fixes #158103
rdar://160383710

llvmbot · 2025-09-11T16:39:13Z

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Dave Bartolomeo (dbartol)

Changes

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.

Fixes #158103

Full diff: https://github.com/llvm/llvm-project/pull/158112.diff

7 Files Affected:

(modified) clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (+30-22)
(modified) clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (+21-9)
(added) clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h (+27)
(modified) clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (+13-2)
(added) clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h (+24)
(modified) clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif (+1-1)
(modified) clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c (+2-2)

diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 2ef98e17cf9c0..4318bce7f01a5 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -36,6 +36,8 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include "PlistDiagnostics.h"
+#include "SarifDiagnostics.h"
 #include <cassert>
 #include <map>
 #include <memory>
@@ -169,6 +171,22 @@ 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 +201,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 +213,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 +222,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 +235,10 @@ void ento::createSarifHTMLDiagnosticConsumer(
     const std::string &sarif_file, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU,
     const MacroExpansionContext &MacroExpansions) {
-  createHTMLDiagnosticConsumer(
-      DiagOpts, C, std::string(llvm::sys::path::parent_path(sarif_file)), PP,
-      CTU, MacroExpansions);
-  createSarifDiagnosticConsumer(DiagOpts, C, sarif_file, PP, CTU,
-                                MacroExpansions);
+  createHTMLDiagnosticConsumerImpl(
+      DiagOpts, C, std::string(llvm::sys::path::parent_path(sarif_file)), PP, 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..b5f292a27ffc8 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -24,6 +24,7 @@
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "PlistDiagnostics.h"
 #include <memory>
 #include <optional>
 
@@ -528,11 +529,14 @@ 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())
@@ -540,7 +544,18 @@ void ento::createPlistDiagnosticConsumer(
 
   C.push_back(std::make_unique<PlistDiagnostics>(
       DiagOpts, OutputFile, PP, CTU, MacroExpansions,
-      /*supportsMultipleFiles=*/false));
+      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 +566,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..248daf596f3a5
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h
@@ -0,0 +1,27 @@
+//==- 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);
+
+} // end ento namespace
+} // end clang namespace
+
+#endif
diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
index 94b2f486ab7d4..87985bf93512c 100644
--- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/JSON.h"
+#include "SarifDiagnostics.h"
 #include <memory>
 
 using namespace llvm;
@@ -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..aa470358c6e6f
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h
@@ -0,0 +1,24 @@
+//==- 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);
+
+} // end ento namespace
+} // end clang namespace
+
+#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) {

github-actions · 2025-09-11T16:39:35Z

✅ With the latest revision this PR passed the C/C++ code formatter.

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.

dbartol · 2025-09-11T16:50:20Z

@t-rasmud

NagyDonat

I'm not familiar with this part of the codebase, but the change looks reasonable and fixes an obvious bug.

steakhal

Makes sense. Thank you!
I had a few nits, but after those we can land this.

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h

t-rasmud

Can we have a test case for plist-html as well? Otherwise LGTM!

steakhal

Can we have a test case for plist-html as well? Otherwise LGTM!

+1

dbartol · 2025-09-16T19:19:58Z

Can we have a test case for plist-html as well? Otherwise LGTM!

Done.

jkorous-apple · 2025-09-17T16:26:20Z

Merged on behalf of @dbartol

dbartol force-pushed the html-duplicate branch from e4507a7 to 35b46a9 Compare September 11, 2025 16:36

dbartol marked this pull request as ready for review September 11, 2025 16:38

llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Sep 11, 2025

dbartol force-pushed the html-duplicate branch from 35b46a9 to 5beea18 Compare September 11, 2025 16:49

Fix build break after reformatting

18b9e09

steakhal requested review from NagyDonat and steakhal September 12, 2025 14:17

NagyDonat reviewed Sep 12, 2025

View reviewed changes

steakhal requested changes Sep 15, 2025

View reviewed changes

t-rasmud approved these changes Sep 15, 2025

View reviewed changes

dbartol added 2 commits September 16, 2025 11:51

Fix file header padding

a24dd6b

Combine namespace declarations

f4bbfbd

steakhal approved these changes Sep 16, 2025

View reviewed changes

Add test for plist-html output

f4c2efb

Generate plist and HTML to temp directory

efe6a1c

jkorous-apple merged commit 48f00e8 into llvm:main Sep 17, 2025
9 checks passed

dbartol mentioned this pull request Sep 25, 2025

Request Commit Access For dbartol #160750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[analyzer] Prevent triplicate warnings for `sarif-html` #158112

[analyzer] Prevent triplicate warnings for `sarif-html` #158112

Uh oh!

dbartol commented Sep 11, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 11, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 11, 2025 •

edited

Loading

Uh oh!

dbartol commented Sep 11, 2025

Uh oh!

NagyDonat left a comment

Uh oh!

steakhal left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-rasmud left a comment

Uh oh!

steakhal left a comment

Uh oh!

dbartol commented Sep 16, 2025

Uh oh!

Uh oh!

jkorous-apple commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[analyzer] Prevent triplicate warnings for sarif-html #158112

[analyzer] Prevent triplicate warnings for sarif-html #158112

Uh oh!

Conversation

dbartol commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dbartol commented Sep 11, 2025

Uh oh!

NagyDonat left a comment

Choose a reason for hiding this comment

Uh oh!

steakhal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-rasmud left a comment

Choose a reason for hiding this comment

Uh oh!

steakhal left a comment

Choose a reason for hiding this comment

Uh oh!

dbartol commented Sep 16, 2025

Uh oh!

Uh oh!

jkorous-apple commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[analyzer] Prevent triplicate warnings for `sarif-html` #158112

[analyzer] Prevent triplicate warnings for `sarif-html` #158112

dbartol commented Sep 11, 2025 •

edited

Loading

llvmbot commented Sep 11, 2025 •

edited

Loading

github-actions bot commented Sep 11, 2025 •

edited

Loading