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 @@ -114,14 +114,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 InterestingSymbols; + llvm::DenseMap 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 InterestingRegions; + llvm::DenseMap + InterestingRegions; /// A set of location contexts that correspoind to call sites which should be /// considered "interesting". @@ -219,9 +220,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; @@ -229,6 +245,14 @@ bool isInteresting(SVal V) const; bool isInteresting(const LocationContext *LC) const; + Optional + getInterestingnessKind(SymbolRef sym) const; + + Optional + getInterestingnessKind(const MemRegion *R) const; + + Optional 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 Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -91,12 +91,14 @@ enum class TrackingKind { /// Default tracking kind -- specifies that as much information should be /// gathered about the tracked expression value as possible. - Thorough, + Thorough = 0, /// Specifies that a more moderate tracking should be used for the expression /// value. This will essentially make sure that functions relevant to the it /// aren't pruned, but otherwise relies on the user reading the code or /// following the arrows. - Condition + Condition = 1, + /// Number of different tracking kinds for sanity checks. + NumTrackingKinds = 2 }; /// Attempts to add visitors to track expression value back to its point of Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -2091,30 +2091,53 @@ } } -void BugReport::markInteresting(SymbolRef sym) { +template +static void insertToInterestingnessMap( + llvm::DenseMap &InterestingnessMap, T Val, + bugreporter::TrackingKind TKind) { + auto Result = InterestingnessMap.insert({Val, TKind}); + + static_assert( + static_cast(bugreporter::TrackingKind::NumTrackingKinds) == 2, + "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!"); + // 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. + if (!Result.second) + if (TKind == bugreporter::TrackingKind::Thorough) + Result.first->getSecond() = bugreporter::TrackingKind::Thorough; +} + +void BugReport::markInteresting(SymbolRef sym, + bugreporter::TrackingKind TKind) { if (!sym) return; - InterestingSymbols.insert(sym); + insertToInterestingnessMap(InterestingSymbols, sym, TKind); if (const auto *meta = dyn_cast(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(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) { @@ -2123,28 +2146,61 @@ InterestingLocationContexts.insert(LC); } -bool BugReport::isInteresting(SVal V) const { - return isInteresting(V.getAsRegion()) || isInteresting(V.getAsSymbol()); -} - -bool BugReport::isInteresting(SymbolRef sym) const { +Optional +BugReport::getInterestingnessKind(SVal V) const { + auto RKind = getInterestingnessKind(V.getAsRegion()); + auto SKind = getInterestingnessKind(V.getAsSymbol()); + static_assert( + static_cast(bugreporter::TrackingKind::NumTrackingKinds) == 2, + "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!"); + 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. + return *RKind != *SKind ? bugreporter::TrackingKind::Thorough : RKind; +} + +Optional +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 +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(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: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1983,7 +1983,7 @@ MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary( LVNode, R, EnableNullFPSuppression, report, V); - report.markInteresting(V); + report.markInteresting(V, TKind); report.addVisitor(llvm::make_unique(R)); // If the contents are symbolic, find out when they became null. @@ -2045,7 +2045,7 @@ const MemRegion *RegionRVal = RVal.getAsRegion(); if (RegionRVal && isa(RegionRVal)) { - report.markInteresting(RegionRVal); + report.markInteresting(RegionRVal, TKind); report.addVisitor(llvm::make_unique( loc::MemRegionVal(RegionRVal), /*assumption=*/false)); }