Skip to content

Commit 5beea18

Browse files
dbartoldbartol
committed
[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.
1 parent 8da3ab1 commit 5beea18

File tree

7 files changed

+112
-36
lines changed

7 files changed

+112
-36
lines changed

‎clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "PlistDiagnostics.h"
14+
#include "SarifDiagnostics.h"
1315
#include "clang/AST/Decl.h"
1416
#include "clang/AST/DeclBase.h"
1517
#include "clang/AST/Stmt.h"
@@ -169,6 +171,21 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ArrowMap &Indices) {
169171

170172
} // namespace
171173

174+
/// Creates and registers an HTML diagnostic consumer, without any additional
175+
/// text consumer.
176+
static void createHTMLDiagnosticConsumerImpl(
177+
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
178+
const std::string &OutputDir, const Preprocessor &PP,
179+
bool SupportMultipleFiles) {
180+
181+
// TODO: Emit an error here.
182+
if (OutputDir.empty())
183+
return;
184+
185+
C.emplace_back(std::make_unique<HTMLDiagnostics>(
186+
std::move(DiagOpts), OutputDir, PP, SupportMultipleFiles));
187+
}
188+
172189
void ento::createHTMLDiagnosticConsumer(
173190
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
174191
const std::string &OutputDir, const Preprocessor &PP,
@@ -183,12 +200,8 @@ void ento::createHTMLDiagnosticConsumer(
183200
createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU,
184201
MacroExpansions);
185202

186-
// TODO: Emit an error here.
187-
if (OutputDir.empty())
188-
return;
189-
190-
C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
191-
OutputDir, PP, true));
203+
createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP,
204+
/*SupportMultipleFiles=*/true);
192205
}
193206

194207
void ento::createHTMLSingleFileDiagnosticConsumer(
@@ -199,24 +212,19 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
199212
createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU,
200213
MacroExpansions);
201214

202-
// TODO: Emit an error here.
203-
if (OutputDir.empty())
204-
return;
205-
206-
C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
207-
OutputDir, PP, false));
215+
createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP,
216+
/*SupportMultipleFiles=*/false);
208217
}
209218

210219
void ento::createPlistHTMLDiagnosticConsumer(
211220
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
212221
const std::string &prefix, const Preprocessor &PP,
213222
const cross_tu::CrossTranslationUnitContext &CTU,
214223
const MacroExpansionContext &MacroExpansions) {
215-
createHTMLDiagnosticConsumer(
216-
DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, CTU,
217-
MacroExpansions);
218-
createPlistMultiFileDiagnosticConsumer(DiagOpts, C, prefix, PP, CTU,
219-
MacroExpansions);
224+
createHTMLDiagnosticConsumerImpl(
225+
DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, true);
226+
createPlistDiagnosticConsumerImpl(DiagOpts, C, prefix, PP, CTU,
227+
MacroExpansions, true);
220228
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, prefix, PP,
221229
CTU, MacroExpansions);
222230
}
@@ -226,11 +234,11 @@ void ento::createSarifHTMLDiagnosticConsumer(
226234
const std::string &sarif_file, const Preprocessor &PP,
227235
const cross_tu::CrossTranslationUnitContext &CTU,
228236
const MacroExpansionContext &MacroExpansions) {
229-
createHTMLDiagnosticConsumer(
237+
createHTMLDiagnosticConsumerImpl(
230238
DiagOpts, C, std::string(llvm::sys::path::parent_path(sarif_file)), PP,
231-
CTU, MacroExpansions);
232-
createSarifDiagnosticConsumer(DiagOpts, C, sarif_file, PP, CTU,
233-
MacroExpansions);
239+
true);
240+
createSarifDiagnosticConsumerImpl(DiagOpts, C, sarif_file, PP);
241+
234242
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, sarif_file,
235243
PP, CTU, MacroExpansions);
236244
}

‎clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "PlistDiagnostics.h"
1314
#include "clang/Analysis/IssueHash.h"
1415
#include "clang/Analysis/MacroExpansionContext.h"
1516
#include "clang/Analysis/PathDiagnostic.h"
@@ -528,19 +529,31 @@ PlistDiagnostics::PlistDiagnostics(
528529
(void)this->CTU;
529530
}
530531

531-
void ento::createPlistDiagnosticConsumer(
532+
/// Creates and registers a Plist diagnostic consumer, without any additional
533+
/// text consumer.
534+
void ento::createPlistDiagnosticConsumerImpl(
532535
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
533536
const std::string &OutputFile, const Preprocessor &PP,
534537
const cross_tu::CrossTranslationUnitContext &CTU,
535-
const MacroExpansionContext &MacroExpansions) {
538+
const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles) {
536539

537540
// TODO: Emit an error here.
538541
if (OutputFile.empty())
539542
return;
540543

541544
C.push_back(std::make_unique<PlistDiagnostics>(
542-
DiagOpts, OutputFile, PP, CTU, MacroExpansions,
543-
/*supportsMultipleFiles=*/false));
545+
DiagOpts, OutputFile, PP, CTU, MacroExpansions, SupportsMultipleFiles));
546+
}
547+
548+
void ento::createPlistDiagnosticConsumer(
549+
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
550+
const std::string &OutputFile, const Preprocessor &PP,
551+
const cross_tu::CrossTranslationUnitContext &CTU,
552+
const MacroExpansionContext &MacroExpansions) {
553+
554+
createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU,
555+
MacroExpansions,
556+
/*SupportsMultipleFiles=*/false);
544557
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
545558
PP, CTU, MacroExpansions);
546559
}
@@ -551,13 +564,10 @@ void ento::createPlistMultiFileDiagnosticConsumer(
551564
const cross_tu::CrossTranslationUnitContext &CTU,
552565
const MacroExpansionContext &MacroExpansions) {
553566

554-
// TODO: Emit an error here.
555-
if (OutputFile.empty())
556-
return;
567+
createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU,
568+
MacroExpansions,
569+
/*SupportsMultipleFiles=*/true);
557570

558-
C.push_back(std::make_unique<PlistDiagnostics>(
559-
DiagOpts, OutputFile, PP, CTU, MacroExpansions,
560-
/*supportsMultipleFiles=*/true));
561571
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
562572
PP, CTU, MacroExpansions);
563573
}

‎clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//==- PlistDiagnostics.h - Plist Diagnostics for Paths --*- C++ -*-//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H
10+
#define LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H
11+
12+
namespace clang {
13+
namespace ento {
14+
15+
void createPlistDiagnosticConsumerImpl(
16+
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
17+
const std::string &Output, const Preprocessor &PP,
18+
const cross_tu::CrossTranslationUnitContext &CTU,
19+
const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles);
20+
21+
} // namespace ento
22+
} // namespace clang
23+
24+
#endif

‎clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "SarifDiagnostics.h"
1314
#include "clang/Analysis/MacroExpansionContext.h"
1415
#include "clang/Analysis/PathDiagnostic.h"
1516
#include "clang/Basic/Sarif.h"
@@ -54,14 +55,24 @@ void ento::createSarifDiagnosticConsumer(
5455
const cross_tu::CrossTranslationUnitContext &CTU,
5556
const MacroExpansionContext &MacroExpansions) {
5657

58+
createSarifDiagnosticConsumerImpl(DiagOpts, C, Output, PP);
59+
60+
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP,
61+
CTU, MacroExpansions);
62+
}
63+
64+
/// Creates and registers a SARIF diagnostic consumer, without any additional
65+
/// text consumer.
66+
void ento::createSarifDiagnosticConsumerImpl(
67+
PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
68+
const std::string &Output, const Preprocessor &PP) {
69+
5770
// TODO: Emit an error here.
5871
if (Output.empty())
5972
return;
6073

6174
C.push_back(std::make_unique<SarifDiagnostics>(Output, PP.getLangOpts(),
6275
PP.getSourceManager()));
63-
createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP,
64-
CTU, MacroExpansions);
6576
}
6677

6778
static StringRef getRuleDescription(StringRef CheckName) {

‎clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//==- SarifDiagnostics.h - SARIF Diagnostics for Paths --*- C++ -*-//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H
10+
#define LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H
11+
12+
namespace clang {
13+
namespace ento {
14+
15+
void createSarifDiagnosticConsumerImpl(PathDiagnosticConsumerOptions DiagOpts,
16+
PathDiagnosticConsumers &C,
17+
const std::string &Output,
18+
const Preprocessor &PP);
19+
20+
} // namespace ento
21+
} // namespace clang
22+
23+
#endif

‎clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
{
55
"artifacts": [
66
{
7-
"length": 1071,
7+
"length": 1152,
88
"location": {
99
"index": 0,
1010
},

‎clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
// 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 -
1+
// 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
2+
// RUN: cat %t%{fs-sep}out.sarif | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
23
#include "../Inputs/system-header-simulator.h"
34
#include "../Inputs/system-header-simulator-for-malloc.h"
45
#define ERR -1
5-
66
int atoi(const char *nptr);
77

88
void f(void) {

0 commit comments

Comments
 (0)