This revision was automatically updated to reflect the committed changes.
Closed by commit rL369583: [analyzer][NFC] Add different interestingness kinds
(authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65723?vs=213258&id=216483#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65723/new/
https://reviews.llvm.org/D65723
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1950,7 +1950,7 @@
MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary(
LVNode, R, EnableNullFPSuppression, report, V);
- report.markInteresting(V);
+ report.markInteresting(V, TKind);
report.addVisitor(std::make_unique<UndefOrNullArgVisitor>(R));
// If the contents are symbolic, find out when they became null.
@@ -2011,7 +2011,7 @@
const MemRegion *RegionRVal = RVal.getAsRegion();
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
- report.markInteresting(RegionRVal);
+ report.markInteresting(RegionRVal, TKind);
report.addVisitor(std::make_unique<TrackConstraintBRVisitor>(
loc::MemRegionVal(RegionRVal), /*assumption=*/false));
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2101,30 +2101,61 @@
}
}
-void BugReport::markInteresting(SymbolRef sym) {
+template <class T>
+static void insertToInterestingnessMap(
+ llvm::DenseMap<T, bugreporter::TrackingKind> &InterestingnessMap, T Val,
+ bugreporter::TrackingKind TKind) {
+ auto Result = InterestingnessMap.insert({Val, TKind});
+
+ if (Result.second)
+ return;
+
+ // Even if this symbol/region was already marked as interesting as a
+ // condition, if we later mark it as interesting again but with
+ // thorough tracking, overwrite it. Entities marked with thorough
+ // interestiness are the most important (or most interesting, if you will),
+ // and we wouldn't like to downplay their importance.
+
+ switch (TKind) {
+ case bugreporter::TrackingKind::Thorough:
+ Result.first->getSecond() = bugreporter::TrackingKind::Thorough;
+ return;
+ case bugreporter::TrackingKind::Condition:
+ return;
+ }
+
+ llvm_unreachable(
+ "BugReport::markInteresting currently can only handle 2 different "
+ "tracking kinds! Please define what tracking kind should this entitiy"
+ "have, if it was already marked as interesting with a different kind!");
+}
+
+void BugReport::markInteresting(SymbolRef sym,
+ bugreporter::TrackingKind TKind) {
if (!sym)
return;
- InterestingSymbols.insert(sym);
+ insertToInterestingnessMap(InterestingSymbols, sym, TKind);
if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
- InterestingRegions.insert(meta->getRegion());
+ markInteresting(meta->getRegion(), TKind);
}
-void BugReport::markInteresting(const MemRegion *R) {
+void BugReport::markInteresting(const MemRegion *R,
+ bugreporter::TrackingKind TKind) {
if (!R)
return;
R = R->getBaseRegion();
- InterestingRegions.insert(R);
+ insertToInterestingnessMap(InterestingRegions, R, TKind);
if (const auto *SR = dyn_cast<SymbolicRegion>(R))
- InterestingSymbols.insert(SR->getSymbol());
+ markInteresting(SR->getSymbol(), TKind);
}
-void BugReport::markInteresting(SVal V) {
- markInteresting(V.getAsRegion());
- markInteresting(V.getAsSymbol());
+void BugReport::markInteresting(SVal V, bugreporter::TrackingKind TKind) {
+ markInteresting(V.getAsRegion(), TKind);
+ markInteresting(V.getAsSymbol(), TKind);
}
void BugReport::markInteresting(const LocationContext *LC) {
@@ -2133,28 +2164,68 @@
InterestingLocationContexts.insert(LC);
}
-bool BugReport::isInteresting(SVal V) const {
- return isInteresting(V.getAsRegion()) || isInteresting(V.getAsSymbol());
+Optional<bugreporter::TrackingKind>
+BugReport::getInterestingnessKind(SVal V) const {
+ auto RKind = getInterestingnessKind(V.getAsRegion());
+ auto SKind = getInterestingnessKind(V.getAsSymbol());
+ if (!RKind)
+ return SKind;
+ if (!SKind)
+ return RKind;
+
+ // If either is marked with throrough tracking, return that, we wouldn't like
+ // to downplay a note's importance by 'only' mentioning it as a condition.
+ switch(*RKind) {
+ case bugreporter::TrackingKind::Thorough:
+ return RKind;
+ case bugreporter::TrackingKind::Condition:
+ return SKind;
+ }
+
+ llvm_unreachable(
+ "BugReport::getInterestingnessKind currently can only handle 2 different "
+ "tracking kinds! Please define what tracking kind should we return here "
+ "when the kind of getAsRegion() and getAsSymbol() is different!");
+ return None;
}
-bool BugReport::isInteresting(SymbolRef sym) const {
+Optional<bugreporter::TrackingKind>
+BugReport::getInterestingnessKind(SymbolRef sym) const {
if (!sym)
- return false;
+ return None;
// We don't currently consider metadata symbols to be interesting
// even if we know their region is interesting. Is that correct behavior?
- return InterestingSymbols.count(sym);
+ auto It = InterestingSymbols.find(sym);
+ if (It == InterestingSymbols.end())
+ return None;
+ return It->getSecond();
}
-bool BugReport::isInteresting(const MemRegion *R) const {
+Optional<bugreporter::TrackingKind>
+BugReport::getInterestingnessKind(const MemRegion *R) const {
if (!R)
- return false;
+ return None;
+
R = R->getBaseRegion();
- bool b = InterestingRegions.count(R);
- if (b)
- return true;
+ auto It = InterestingRegions.find(R);
+ if (It != InterestingRegions.end())
+ return It->getSecond();
+
if (const auto *SR = dyn_cast<SymbolicRegion>(R))
- return InterestingSymbols.count(SR->getSymbol());
- return false;
+ return getInterestingnessKind(SR->getSymbol());
+ return None;
+}
+
+bool BugReport::isInteresting(SVal V) const {
+ return getInterestingnessKind(V).hasValue();
+}
+
+bool BugReport::isInteresting(SymbolRef sym) const {
+ return getInterestingnessKind(sym).hasValue();
+}
+
+bool BugReport::isInteresting(const MemRegion *R) const {
+ return getInterestingnessKind(R).hasValue();
}
bool BugReport::isInteresting(const LocationContext *LC) const {
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -102,14 +102,15 @@
/// diagnostics to include when constructing the final path diagnostic.
/// The stack is largely used by BugReporter when generating PathDiagnostics
/// for multiple PathDiagnosticConsumers.
- llvm::DenseSet<SymbolRef> InterestingSymbols;
+ llvm::DenseMap<SymbolRef, bugreporter::TrackingKind> InterestingSymbols;
/// A (stack of) set of regions that are registered with this report as being
/// "interesting", and thus used to help decide which diagnostics
/// to include when constructing the final path diagnostic.
/// The stack is largely used by BugReporter when generating PathDiagnostics
/// for multiple PathDiagnosticConsumers.
- llvm::DenseSet<const MemRegion *> InterestingRegions;
+ llvm::DenseMap<const MemRegion *, bugreporter::TrackingKind>
+ InterestingRegions;
/// A set of location contexts that correspoind to call sites which should be
/// considered "interesting".
@@ -209,9 +210,24 @@
/// Disable all path pruning when generating a PathDiagnostic.
void disablePathPruning() { DoNotPrunePath = true; }
- void markInteresting(SymbolRef sym);
- void markInteresting(const MemRegion *R);
- void markInteresting(SVal V);
+ /// Marks a symbol as interesting. Different kinds of interestingness will
+ /// be processed differently by visitors (e.g. if the tracking kind is
+ /// condition, will append "will be used as a condition" to the message).
+ void markInteresting(SymbolRef sym, bugreporter::TrackingKind TKind =
+ bugreporter::TrackingKind::Thorough);
+
+ /// Marks a region as interesting. Different kinds of interestingness will
+ /// be processed differently by visitors (e.g. if the tracking kind is
+ /// condition, will append "will be used as a condition" to the message).
+ void markInteresting(
+ const MemRegion *R,
+ bugreporter::TrackingKind TKind = bugreporter::TrackingKind::Thorough);
+
+ /// Marks a symbolic value as interesting. Different kinds of interestingness
+ /// will be processed differently by visitors (e.g. if the tracking kind is
+ /// condition, will append "will be used as a condition" to the message).
+ void markInteresting(SVal V, bugreporter::TrackingKind TKind =
+ bugreporter::TrackingKind::Thorough);
void markInteresting(const LocationContext *LC);
bool isInteresting(SymbolRef sym) const;
@@ -219,6 +235,14 @@
bool isInteresting(SVal V) const;
bool isInteresting(const LocationContext *LC) const;
+ Optional<bugreporter::TrackingKind>
+ getInterestingnessKind(SymbolRef sym) const;
+
+ Optional<bugreporter::TrackingKind>
+ getInterestingnessKind(const MemRegion *R) const;
+
+ Optional<bugreporter::TrackingKind> getInterestingnessKind(SVal V) const;
+
/// Returns whether or not this report should be considered valid.
///
/// Invalid reports are those that have been classified as likely false
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
