OikawaKirie updated this revision to Diff 397207.
OikawaKirie edited the summary of this revision.
OikawaKirie added a comment.
In D115716#3217786 <https://reviews.llvm.org/D115716#3217786>, @Szelethus wrote:
> First off, your patch is great, and I'm pretty sure we want it!
>
> I remember working around here, and I either have never quite understood what
> makes `exampleReport` an example, or have long forgotten. Can we not just
> rename it to `chosenReport`, or simply `report`?
Thanks a lot.
New updates:
1. add a new `BugReporter::generateDiagnosticForConsumerMap` method to handle
the job before adding notes
2. add a `BugReport` field to `DiagnosticForConsumerMapTy`
3. move `findReportInEquivalenceClass` calls to
`generateDiagnosticForConsumerMap` methods
4. the `exampleReport` variable in method `findReportInEquivalenceClass` is
unchanged, others are removed as they are no longer required
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115716/new/
https://reviews.llvm.org/D115716
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
clang/test/Analysis/keychainAPI.m
clang/test/Analysis/malloc-plist.c
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -261,9 +261,7 @@
if (!F1)
return;
if (Test == 1) {
- return; // no warning
+ return; // expected-warning {{Opened stream never closed. Potential resource leak}}
}
rewind(F1);
-} // expected-warning {{Opened stream never closed. Potential resource leak}}
-// FIXME: This warning should be placed at the `return` above.
-// See https://reviews.llvm.org/D83120 about details.
+} // no warning
Index: clang/test/Analysis/malloc-plist.c
===================================================================
--- clang/test/Analysis/malloc-plist.c
+++ clang/test/Analysis/malloc-plist.c
@@ -135,8 +135,8 @@
static void function_with_leak3(int y) {
char *x = (char*)malloc(12);
if (y)
- y++;
-}//expected-warning{{Potential leak}}
+ y++; // expected-warning{{Potential leak}}
+}
void use_function_with_leak3(int y) {
function_with_leak3(y);
}
Index: clang/test/Analysis/keychainAPI.m
===================================================================
--- clang/test/Analysis/keychainAPI.m
+++ clang/test/Analysis/keychainAPI.m
@@ -402,10 +402,10 @@
OSStatus st = 0;
void *outData = my_AllocateReturn(&st);
if (x) {
- consumeChar(*(char*)outData); // expected-warning{{Allocated data is not released:}}
+ consumeChar(*(char *)outData);
return;
} else {
- consumeChar(*(char*)outData);
+ consumeChar(*(char *)outData); // expected-warning{{Allocated data is not released:}}
}
return;
}
Index: clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
+++ clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist
@@ -3764,12 +3764,12 @@
<array>
<dict>
<key>line</key><integer>138</integer>
- <key>col</key><integer>9</integer>
+ <key>col</key><integer>7</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
<key>line</key><integer>138</integer>
- <key>col</key><integer>9</integer>
+ <key>col</key><integer>7</integer>
<key>file</key><integer>0</integer>
</dict>
</array>
@@ -3781,7 +3781,7 @@
<key>location</key>
<dict>
<key>line</key><integer>138</integer>
- <key>col</key><integer>9</integer>
+ <key>col</key><integer>7</integer>
<key>file</key><integer>0</integer>
</dict>
<key>depth</key><integer>1</integer>
@@ -3803,7 +3803,7 @@
<key>location</key>
<dict>
<key>line</key><integer>138</integer>
- <key>col</key><integer>9</integer>
+ <key>col</key><integer>7</integer>
<key>file</key><integer>0</integer>
</dict>
<key>ExecutedLines</key>
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -244,6 +244,9 @@
std::unique_ptr<PathDiagnostic>
generate(const PathDiagnosticConsumer *PDC) const;
+ /// Get the bug report used to generate the PathDiagnostic.
+ const PathSensitiveBugReport *getBugReport() const { return R; }
+
private:
void updateStackPiecesWithMessage(PathDiagnosticPieceRef P,
const CallWithEntryStack &CallStack) const;
@@ -271,8 +274,6 @@
PathDiagnosticLocation
ExecutionContinues(llvm::raw_string_ostream &os,
const PathDiagnosticConstruct &C) const;
-
- const PathSensitiveBugReport *getBugReport() const { return R; }
};
} // namespace
@@ -2889,6 +2890,7 @@
(*Out)[PC] = std::move(PD);
}
}
+ Out->Report = PDB->getBugReport();
}
return Out;
@@ -3060,24 +3062,28 @@
return exampleReport;
}
-void BugReporter::FlushReport(BugReportEquivClass& EQ) {
- SmallVector<BugReport*, 10> bugReports;
- BugReport *report = findReportInEquivalenceClass(EQ, bugReports);
- if (!report)
- return;
-
+std::unique_ptr<DiagnosticForConsumerMapTy>
+BugReporter::generateDiagnosticForConsumerMap(BugReportEquivClass &EQ) {
// See whether we need to silence the checker/package.
+ auto *report = EQ.getReports()[0].get();
for (const std::string &CheckerOrPackage :
getAnalyzerOptions().SilencedCheckersAndPackages) {
if (report->getBugType().getCheckerName().startswith(
CheckerOrPackage))
- return;
+ return nullptr;
}
ArrayRef<PathDiagnosticConsumer*> Consumers = getPathDiagnosticConsumers();
+ return generateDiagnosticForConsumerMap(EQ, Consumers);
+}
+
+void BugReporter::FlushReport(BugReportEquivClass &EQ) {
std::unique_ptr<DiagnosticForConsumerMapTy> Diagnostics =
- generateDiagnosticForConsumerMap(report, Consumers, bugReports);
+ generateDiagnosticForConsumerMap(EQ);
+ if (!Diagnostics)
+ return;
+ auto *report = Diagnostics->Report;
for (auto &P : *Diagnostics) {
PathDiagnosticConsumer *Consumer = P.first;
std::unique_ptr<PathDiagnostic> &PD = P.second;
@@ -3200,12 +3206,17 @@
std::unique_ptr<DiagnosticForConsumerMapTy>
BugReporter::generateDiagnosticForConsumerMap(
- BugReport *exampleReport, ArrayRef<PathDiagnosticConsumer *> consumers,
- ArrayRef<BugReport *> bugReports) {
- auto *basicReport = cast<BasicBugReport>(exampleReport);
+ BugReportEquivClass &EQ, ArrayRef<PathDiagnosticConsumer *> consumers) {
+ SmallVector<BugReport *> _;
+ auto *report = BugReporter::findReportInEquivalenceClass(EQ, _);
+ if (!report)
+ return nullptr;
+
+ auto *basicReport = cast<BasicBugReport>(report);
auto Out = std::make_unique<DiagnosticForConsumerMapTy>();
for (auto *Consumer : consumers)
(*Out)[Consumer] = generateDiagnosticForBasicReport(basicReport);
+ Out->Report = report;
return Out;
}
@@ -3272,17 +3283,18 @@
}
}
-
-
std::unique_ptr<DiagnosticForConsumerMapTy>
PathSensitiveBugReporter::generateDiagnosticForConsumerMap(
- BugReport *exampleReport, ArrayRef<PathDiagnosticConsumer *> consumers,
- ArrayRef<BugReport *> bugReports) {
+ BugReportEquivClass &EQ, ArrayRef<PathDiagnosticConsumer *> consumers) {
+ SmallVector<BugReport *, 10> bugReports;
+ const BugReport *report = findReportInEquivalenceClass(EQ, bugReports);
+ if (!report)
+ return nullptr;
+
std::vector<BasicBugReport *> BasicBugReports;
std::vector<PathSensitiveBugReport *> PathSensitiveBugReports;
- if (isa<BasicBugReport>(exampleReport))
- return BugReporter::generateDiagnosticForConsumerMap(exampleReport,
- consumers, bugReports);
+ if (isa<BasicBugReport>(report))
+ return BugReporter::generateDiagnosticForConsumerMap(EQ, consumers);
// Generate the full path sensitive diagnostic, using the generation scheme
// specified by the PathDiagnosticConsumer. Note that we have to generate
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -55,6 +55,7 @@
namespace ento {
+class BugReport;
class BugType;
class CheckerBase;
class ExplodedGraph;
@@ -69,8 +70,11 @@
/// A mapping from diagnostic consumers to the diagnostics they should
/// consume.
-using DiagnosticForConsumerMapTy =
- llvm::DenseMap<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>>;
+struct DiagnosticForConsumerMapTy
+ : llvm::DenseMap<PathDiagnosticConsumer *,
+ std::unique_ptr<PathDiagnostic>> {
+ const BugReport *Report = nullptr;
+};
/// Interface for classes constructing Stack hints.
///
@@ -210,7 +214,7 @@
Notes.push_back(std::move(P));
}
- ArrayRef<std::shared_ptr<PathDiagnosticNotePiece>> getNotes() {
+ ArrayRef<std::shared_ptr<PathDiagnosticNotePiece>> getNotes() const {
return Notes;
}
@@ -654,12 +658,14 @@
return eqClass.getReports()[0].get();
}
+ std::unique_ptr<DiagnosticForConsumerMapTy>
+ generateDiagnosticForConsumerMap(BugReportEquivClass &EQ);
+
protected:
/// Generate the diagnostics for the given bug report.
virtual std::unique_ptr<DiagnosticForConsumerMapTy>
- generateDiagnosticForConsumerMap(BugReport *exampleReport,
- ArrayRef<PathDiagnosticConsumer *> consumers,
- ArrayRef<BugReport *> bugReports);
+ generateDiagnosticForConsumerMap(
+ BugReportEquivClass &EQ, ArrayRef<PathDiagnosticConsumer *> consumers);
};
/// GRBugReporter is used for generating path-sensitive reports.
@@ -671,10 +677,10 @@
SmallVectorImpl<BugReport *> &bugReports) override;
/// Generate the diagnostics for the given bug report.
- std::unique_ptr<DiagnosticForConsumerMapTy>
- generateDiagnosticForConsumerMap(BugReport *exampleReport,
- ArrayRef<PathDiagnosticConsumer *> consumers,
- ArrayRef<BugReport *> bugReports) override;
+ std::unique_ptr<DiagnosticForConsumerMapTy> generateDiagnosticForConsumerMap(
+ BugReportEquivClass &EQ,
+ ArrayRef<PathDiagnosticConsumer *> consumers) override;
+
public:
PathSensitiveBugReporter(BugReporterData& d, ExprEngine& eng)
: BugReporter(d), Eng(eng) {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
