[clang] 48f00e8 - [analyzer] Prevent triplicate warnings for `sarif-html` (#158112)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 17 09:25:55 PDT 2025
Author: Dave Bartolomeo
Date: 2025-09-17T09:25:51-07:00
New Revision: 48f00e81338e637b8210d3320527d4f389d5343b
URL: https://github.com/llvm/llvm-project/commit/48f00e81338e637b8210d3320527d4f389d5343b
DIFF: https://github.com/llvm/llvm-project/commit/48f00e81338e637b8210d3320527d4f389d5343b.diff
LOG: [analyzer] Prevent triplicate warnings for `sarif-html` (#158112)
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
Added:
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h
clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-html.c.plist
clang/test/Analysis/diagnostics/plist-html.c
Modified:
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
Removed:
################################################################################
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..d4ec998ad7d2d
--- /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
+
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include <string>
+
+namespace clang::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 clang::ento
+
+#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..533ee99191926
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h
@@ -0,0 +1,25 @@
+//==- 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
+
+#include "clang/Lex/Preprocessor.h"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include <string>
+
+namespace clang::ento {
+
+void createSarifDiagnosticConsumerImpl(PathDiagnosticConsumerOptions DiagOpts,
+ PathDiagnosticConsumers &C,
+ const std::string &Output,
+ const Preprocessor &PP);
+
+} // namespace clang::ento
+
+#endif
diff --git a/clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-html.c.plist b/clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-html.c.plist
new file mode 100644
index 0000000000000..e12a5a688f648
--- /dev/null
+++ b/clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-html.c.plist
@@ -0,0 +1,197 @@
+ <array>
+ <dict>
+ <key>kind</key><string>control</string>
+ <key>edges</key>
+ <array>
+ <dict>
+ <key>start</key>
+ <array>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>5</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>6</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>end</key>
+ <array>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </dict>
+ </array>
+ </dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>Assuming 'p' is null</string>
+ <key>message</key>
+ <string>Assuming 'p' is null</string>
+ </dict>
+ <dict>
+ <key>kind</key><string>control</string>
+ <key>edges</key>
+ <array>
+ <dict>
+ <key>start</key>
+ <array>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>5</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>end</key>
+ <array>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>14</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </dict>
+ </array>
+ </dict>
+ <dict>
+ <key>kind</key><string>control</string>
+ <key>edges</key>
+ <array>
+ <dict>
+ <key>start</key>
+ <array>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>9</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>14</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>end</key>
+ <array>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </dict>
+ </array>
+ </dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>17</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>17</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>Dereference of null pointer (loaded from variable 'p')</string>
+ <key>message</key>
+ <string>Dereference of null pointer (loaded from variable 'p')</string>
+ </dict>
+ </array>
+ <key>description</key><string>Dereference of null pointer (loaded from variable 'p')</string>
+ <key>category</key><string>Logic error</string>
+ <key>type</key><string>Dereference of null pointer</string>
+ <key>check_name</key><string>core.NullDereference</string>
+ <!-- This hash is experimental and going to change! -->
+ <key>issue_hash_content_of_line_in_context</key><string>665591022ee1cf653566ea441043d888</string>
+ <key>issue_context_kind</key><string>function</string>
+ <key>issue_context</key><string>foo</string>
+ <key>issue_hash_function_offset</key><string>4</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>8</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>HTMLDiagnostics_files</key>
+ <array>
+ <string>report-665591.html</string>
+ </array>
+ <key>ExecutedLines</key>
+ <dict>
+ <key>0</key>
+ <array>
+ <integer>4</integer>
+ <integer>5</integer>
+ <integer>8</integer>
+ </array>
+ </dict>
+ </dict>
+ </array>
+ <key>files</key>
+ <array>
+ </array>
+</dict>
+</plist>
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/plist-html.c b/clang/test/Analysis/diagnostics/plist-html.c
new file mode 100644
index 0000000000000..7ee0aa5681d3a
--- /dev/null
+++ b/clang/test/Analysis/diagnostics/plist-html.c
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t && mkdir %t && %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=plist-html %s -o %t/plist-html.c.plist -verify
+// RUN: tail -n +11 %t/plist-html.c.plist | %normalize_plist |
diff -ub %S/Inputs/expected-plists/plist-html.c.plist -
+
+int foo(int *p) {
+ if (p) {
+ return 0;
+ } else {
+ return *p; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+ }
+}
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) {
More information about the cfe-commits
mailing list