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 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". @@ -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 + 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: 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 +static void insertToInterestingnessMap( + llvm::DenseMap &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(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) { @@ -2133,28 +2164,68 @@ InterestingLocationContexts.insert(LC); } -bool BugReport::isInteresting(SVal V) const { - return isInteresting(V.getAsRegion()) || isInteresting(V.getAsSymbol()); +Optional +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 +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: 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(R)); // If the contents are symbolic, find out when they became null. @@ -2011,7 +2011,7 @@ const MemRegion *RegionRVal = RVal.getAsRegion(); if (RegionRVal && isa(RegionRVal)) { - report.markInteresting(RegionRVal); + report.markInteresting(RegionRVal, TKind); report.addVisitor(std::make_unique( loc::MemRegionVal(RegionRVal), /*assumption=*/false)); }