Skip to content

Commit 7bf871c

Browse files
committed
[analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions
TableGen and .def files (which are meant to be used with the preprocessor) come with obvious downsides. One of those issues is that generated switch-case branches have to be identical. This pushes corner cases either to an outer code block, or into the generated code. Inspect the removed code in AnalysisConsumer::DigestAnalyzerOptions. You can see how corner cases like a not existing output file, the analysis output type being set to PD_NONE, or whether to complement the output with additional diagnostics on stderr lay around the preprocessor generated code. This is a bit problematic, as to how to deal with such errors is not in the hands of the users of this interface (those implementing output types, like PlistDiagnostics etc). This patch changes this by moving these corner cases into the generated code, more specifically, into the called functions. In addition, I introduced a new output type for convenience purposes, PD_TEXT_MINIMAL, which always existed conceptually, but never in the actual Analyses.def file. This refactoring allowed me to move TextDiagnostics (renamed from ClangDiagPathDiagConsumer) to its own file, which it really deserved. Also, those that had the misfortune to gaze upon Analyses.def will probably enjoy the sight that a clang-format did on it. Differential Revision: https://reviews.llvm.org/D76509
1 parent f2f96eb commit 7bf871c

File tree

8 files changed

+282
-186
lines changed

8 files changed

+282
-186
lines changed

clang/include/clang/StaticAnalyzer/Core/Analyses.def

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,46 +14,84 @@
1414
#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)
1515
#endif
1616

17-
ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store", CreateRegionStoreManager)
17+
ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store",
18+
CreateRegionStoreManager)
1819

1920
#ifndef ANALYSIS_CONSTRAINTS
2021
#define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)
2122
#endif
2223

23-
ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of concrete value ranges", CreateRangeConstraintManager)
24-
ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", CreateZ3ConstraintManager)
24+
ANALYSIS_CONSTRAINTS(RangeConstraints, "range",
25+
"Use constraint tracking of concrete value ranges",
26+
CreateRangeConstraintManager)
27+
28+
ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver",
29+
CreateZ3ConstraintManager)
2530

2631
#ifndef ANALYSIS_DIAGNOSTICS
2732
#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
2833
#endif
2934

30-
ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML", createHTMLDiagnosticConsumer)
31-
ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "html-single-file", "Output analysis results using HTML (not allowing for multi-file bugs)", createHTMLSingleFileDiagnosticConsumer)
32-
ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", createPlistDiagnosticConsumer)
33-
ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis results using Plists (allowing for multi-file bugs)", createPlistMultiFileDiagnosticConsumer)
34-
ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
35-
ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF file", createSarifDiagnosticConsumer)
36-
ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", createTextPathDiagnosticConsumer)
35+
ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
36+
createHTMLDiagnosticConsumer)
37+
38+
ANALYSIS_DIAGNOSTICS(
39+
HTML_SINGLE_FILE, "html-single-file",
40+
"Output analysis results using HTML (not allowing for multi-file bugs)",
41+
createHTMLSingleFileDiagnosticConsumer)
42+
43+
ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists",
44+
createPlistDiagnosticConsumer)
45+
46+
ANALYSIS_DIAGNOSTICS(
47+
PLIST_MULTI_FILE, "plist-multi-file",
48+
"Output analysis results using Plists (allowing for multi-file bugs)",
49+
createPlistMultiFileDiagnosticConsumer)
50+
51+
ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html",
52+
"Output analysis results using HTML wrapped with Plists",
53+
createPlistHTMLDiagnosticConsumer)
54+
55+
ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF file",
56+
createSarifDiagnosticConsumer)
57+
58+
ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results to stderr",
59+
createTextPathDiagnosticConsumer)
60+
61+
ANALYSIS_DIAGNOSTICS(TEXT_MINIMAL, "text-minimal",
62+
"Emits minimal diagnostics to stderr, stating only the "
63+
"warning message and the associated notes. Usually "
64+
"used in addition to other analysis types",
65+
createTextMinimalPathDiagnosticConsumer)
3766

3867
#ifndef ANALYSIS_PURGE
3968
#define ANALYSIS_PURGE(NAME, CMDFLAG, DESC)
4069
#endif
4170

42-
ANALYSIS_PURGE(PurgeStmt, "statement", "Purge symbols, bindings, and constraints before every statement")
43-
ANALYSIS_PURGE(PurgeBlock, "block", "Purge symbols, bindings, and constraints before every basic block")
44-
ANALYSIS_PURGE(PurgeNone, "none", "Do not purge symbols, bindings, or constraints")
71+
ANALYSIS_PURGE(
72+
PurgeStmt, "statement",
73+
"Purge symbols, bindings, and constraints before every statement")
74+
75+
ANALYSIS_PURGE(
76+
PurgeBlock, "block",
77+
"Purge symbols, bindings, and constraints before every basic block")
78+
79+
ANALYSIS_PURGE(PurgeNone, "none",
80+
"Do not purge symbols, bindings, or constraints")
4581

4682
#ifndef ANALYSIS_INLINING_MODE
4783
#define ANALYSIS_INLINING_MODE(NAME, CMDFLAG, DESC)
4884
#endif
4985

50-
ANALYSIS_INLINING_MODE(All, "all", "Analyze all functions as top level")
51-
ANALYSIS_INLINING_MODE(NoRedundancy, "noredundancy", "Do not analyze a function which has been previously inlined")
86+
ANALYSIS_INLINING_MODE(All, "all", "Analyze all functions as top level")
87+
88+
ANALYSIS_INLINING_MODE(
89+
NoRedundancy, "noredundancy",
90+
"Do not analyze a function which has been previously inlined")
5291

5392
#undef ANALYSIS_STORE
5493
#undef ANALYSIS_CONSTRAINTS
5594
#undef ANALYSIS_DIAGNOSTICS
5695
#undef ANALYSIS_PURGE
5796
#undef ANALYSIS_INLINING_MODE
5897
#undef ANALYSIS_IPA
59-

clang/lib/StaticAnalyzer/Core/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ add_clang_library(clangStaticAnalyzerCore
4545
SValBuilder.cpp
4646
SVals.cpp
4747
SymbolManager.cpp
48+
TextDiagnostics.cpp
4849
WorkList.cpp
4950

5051
LINK_LIBS
@@ -56,5 +57,6 @@ add_clang_library(clangStaticAnalyzerCore
5657
clangFrontend
5758
clangLex
5859
clangRewrite
60+
clangToolingCore
5961
)
6062

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,9 @@ class HTMLDiagnostics : public PathDiagnosticConsumer {
6666
const bool SupportsCrossFileDiagnostics;
6767

6868
public:
69-
HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts,
70-
const std::string& prefix,
71-
const Preprocessor &pp,
72-
bool supportsMultipleFiles)
73-
: Directory(prefix), PP(pp), AnalyzerOpts(AnalyzerOpts),
69+
HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &OutputDir,
70+
const Preprocessor &pp, bool supportsMultipleFiles)
71+
: Directory(OutputDir), PP(pp), AnalyzerOpts(AnalyzerOpts),
7472
SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
7573

7674
~HTMLDiagnostics() override { FlushDiagnostics(nullptr); }
@@ -136,16 +134,45 @@ class HTMLDiagnostics : public PathDiagnosticConsumer {
136134

137135
void ento::createHTMLDiagnosticConsumer(
138136
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
139-
const std::string &prefix, const Preprocessor &PP,
140-
const cross_tu::CrossTranslationUnitContext &) {
141-
C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, true));
137+
const std::string &OutputDir, const Preprocessor &PP,
138+
const cross_tu::CrossTranslationUnitContext &CTU) {
139+
140+
// FIXME: HTML is currently our default output type, but if the output
141+
// directory isn't specified, it acts like if it was in the minimal text
142+
// output mode. This doesn't make much sense, we should have the minimal text
143+
// as our default. In the case of backward compatibility concerns, this could
144+
// be preserved with -analyzer-config-compatibility-mode=true.
145+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
146+
147+
// TODO: Emit an error here.
148+
if (OutputDir.empty())
149+
return;
150+
151+
C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
142152
}
143153

144154
void ento::createHTMLSingleFileDiagnosticConsumer(
155+
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
156+
const std::string &OutputDir, const Preprocessor &PP,
157+
const cross_tu::CrossTranslationUnitContext &CTU) {
158+
159+
// TODO: Emit an error here.
160+
if (OutputDir.empty())
161+
return;
162+
163+
C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, false));
164+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
165+
}
166+
167+
void ento::createPlistHTMLDiagnosticConsumer(
145168
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
146169
const std::string &prefix, const Preprocessor &PP,
147-
const cross_tu::CrossTranslationUnitContext &) {
148-
C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, false));
170+
const cross_tu::CrossTranslationUnitContext &CTU) {
171+
createHTMLDiagnosticConsumer(
172+
AnalyzerOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
173+
CTU);
174+
createPlistMultiFileDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
175+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
149176
}
150177

151178
//===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ namespace {
4545
AnalyzerOptions &AnOpts;
4646
const bool SupportsCrossFileDiagnostics;
4747
public:
48-
PlistDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &prefix,
49-
const Preprocessor &PP,
48+
PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
49+
const std::string &OutputFile, const Preprocessor &PP,
5050
const cross_tu::CrossTranslationUnitContext &CTU,
5151
bool supportsMultipleFiles);
5252

@@ -582,19 +582,32 @@ PlistDiagnostics::PlistDiagnostics(
582582

583583
void ento::createPlistDiagnosticConsumer(
584584
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
585-
const std::string &s, const Preprocessor &PP,
585+
const std::string &OutputFile, const Preprocessor &PP,
586586
const cross_tu::CrossTranslationUnitContext &CTU) {
587-
C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU,
587+
588+
// TODO: Emit an error here.
589+
if (OutputFile.empty())
590+
return;
591+
592+
C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
588593
/*supportsMultipleFiles*/ false));
594+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputFile, PP, CTU);
589595
}
590596

591597
void ento::createPlistMultiFileDiagnosticConsumer(
592598
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
593-
const std::string &s, const Preprocessor &PP,
599+
const std::string &OutputFile, const Preprocessor &PP,
594600
const cross_tu::CrossTranslationUnitContext &CTU) {
595-
C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU,
601+
602+
// TODO: Emit an error here.
603+
if (OutputFile.empty())
604+
return;
605+
606+
C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
596607
/*supportsMultipleFiles*/ true));
608+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputFile, PP, CTU);
597609
}
610+
598611
void PlistDiagnostics::FlushDiagnosticsImpl(
599612
std::vector<const PathDiagnostic *> &Diags,
600613
FilesMade *filesMade) {

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,14 @@ class SarifDiagnostics : public PathDiagnosticConsumer {
5050
void ento::createSarifDiagnosticConsumer(
5151
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
5252
const std::string &Output, const Preprocessor &PP,
53-
const cross_tu::CrossTranslationUnitContext &) {
53+
const cross_tu::CrossTranslationUnitContext &CTU) {
54+
55+
// TODO: Emit an error here.
56+
if (Output.empty())
57+
return;
58+
5459
C.push_back(new SarifDiagnostics(AnalyzerOpts, Output, PP.getLangOpts()));
60+
createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, Output, PP, CTU);
5561
}
5662

5763
static StringRef getFileName(const FileEntry &FE) {
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
//===--- TextDiagnostics.cpp - Text 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+
// This file defines the TextDiagnostics object.
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "clang/Analysis/PathDiagnostic.h"
14+
#include "clang/Basic/SourceManager.h"
15+
#include "clang/Basic/Version.h"
16+
#include "clang/CrossTU/CrossTranslationUnit.h"
17+
#include "clang/Frontend/ASTUnit.h"
18+
#include "clang/Lex/Preprocessor.h"
19+
#include "clang/Rewrite/Core/Rewriter.h"
20+
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
21+
#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
22+
#include "clang/Tooling/Core/Replacement.h"
23+
#include "clang/Tooling/Tooling.h"
24+
#include "llvm/ADT/SmallPtrSet.h"
25+
#include "llvm/ADT/SmallVector.h"
26+
#include "llvm/Support/Casting.h"
27+
28+
using namespace clang;
29+
using namespace ento;
30+
using namespace tooling;
31+
32+
namespace {
33+
/// Emitsd minimal diagnostics (report message + notes) for the 'none' output
34+
/// type to the standard error, or to to compliment many others. Emits detailed
35+
/// diagnostics in textual format for the 'text' output type.
36+
class TextDiagnostics : public PathDiagnosticConsumer {
37+
DiagnosticsEngine &DiagEng;
38+
LangOptions LO;
39+
const bool IncludePath = false;
40+
const bool ShouldEmitAsError = false;
41+
const bool ApplyFixIts = false;
42+
43+
public:
44+
TextDiagnostics(DiagnosticsEngine &DiagEng, LangOptions LO,
45+
bool ShouldIncludePath, const AnalyzerOptions &AnOpts)
46+
: DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
47+
ShouldEmitAsError(AnOpts.AnalyzerWerror),
48+
ApplyFixIts(AnOpts.ShouldApplyFixIts) {}
49+
~TextDiagnostics() override {}
50+
51+
StringRef getName() const override { return "TextDiagnostics"; }
52+
53+
bool supportsLogicalOpControlFlow() const override { return true; }
54+
bool supportsCrossFileDiagnostics() const override { return true; }
55+
56+
PathGenerationScheme getGenerationScheme() const override {
57+
return IncludePath ? Minimal : None;
58+
}
59+
60+
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
61+
FilesMade *filesMade) override {
62+
unsigned WarnID =
63+
ShouldEmitAsError
64+
? DiagEng.getCustomDiagID(DiagnosticsEngine::Error, "%0")
65+
: DiagEng.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
66+
unsigned NoteID = DiagEng.getCustomDiagID(DiagnosticsEngine::Note, "%0");
67+
SourceManager &SM = DiagEng.getSourceManager();
68+
69+
Replacements Repls;
70+
auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
71+
ArrayRef<SourceRange> Ranges,
72+
ArrayRef<FixItHint> Fixits) {
73+
if (!ApplyFixIts) {
74+
DiagEng.Report(Loc, ID) << String << Ranges << Fixits;
75+
return;
76+
}
77+
78+
DiagEng.Report(Loc, ID) << String << Ranges;
79+
for (const FixItHint &Hint : Fixits) {
80+
Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
81+
82+
if (llvm::Error Err = Repls.add(Repl)) {
83+
llvm::errs() << "Error applying replacement " << Repl.toString()
84+
<< ": " << Err << "\n";
85+
}
86+
}
87+
};
88+
89+
for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
90+
E = Diags.end();
91+
I != E; ++I) {
92+
const PathDiagnostic *PD = *I;
93+
reportPiece(WarnID, PD->getLocation().asLocation(),
94+
PD->getShortDescription(), PD->path.back()->getRanges(),
95+
PD->path.back()->getFixits());
96+
97+
// First, add extra notes, even if paths should not be included.
98+
for (const auto &Piece : PD->path) {
99+
if (!isa<PathDiagnosticNotePiece>(Piece.get()))
100+
continue;
101+
102+
reportPiece(NoteID, Piece->getLocation().asLocation(),
103+
Piece->getString(), Piece->getRanges(), Piece->getFixits());
104+
}
105+
106+
if (!IncludePath)
107+
continue;
108+
109+
// Then, add the path notes if necessary.
110+
PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
111+
for (const auto &Piece : FlatPath) {
112+
if (isa<PathDiagnosticNotePiece>(Piece.get()))
113+
continue;
114+
115+
reportPiece(NoteID, Piece->getLocation().asLocation(),
116+
Piece->getString(), Piece->getRanges(), Piece->getFixits());
117+
}
118+
}
119+
120+
if (!ApplyFixIts || Repls.empty())
121+
return;
122+
123+
Rewriter Rewrite(SM, LO);
124+
if (!applyAllReplacements(Repls, Rewrite)) {
125+
llvm::errs() << "An error occured during applying fix-it.\n";
126+
}
127+
128+
Rewrite.overwriteChangedFiles();
129+
}
130+
};
131+
} // end anonymous namespace
132+
133+
void ento::createTextPathDiagnosticConsumer(
134+
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
135+
const std::string &Prefix, const clang::Preprocessor &PP,
136+
const cross_tu::CrossTranslationUnitContext &CTU) {
137+
C.emplace_back(new TextDiagnostics(PP.getDiagnostics(), PP.getLangOpts(),
138+
/*ShouldIncludePath*/ true, AnalyzerOpts));
139+
}
140+
141+
void ento::createTextMinimalPathDiagnosticConsumer(
142+
AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
143+
const std::string &Prefix, const clang::Preprocessor &PP,
144+
const cross_tu::CrossTranslationUnitContext &CTU) {
145+
C.emplace_back(new TextDiagnostics(PP.getDiagnostics(), PP.getLangOpts(),
146+
/*ShouldIncludePath*/ false,
147+
AnalyzerOpts));
148+
}

0 commit comments

Comments
 (0)