[analyzer] Add forwarding `addVisitor` method

The majority of all `addVisitor` callers follow the same pattern:
  addVisitor(std::make_unique<SomeVisitor>(arg1, arg2, ...));

This patches introduces additional overload for `addVisitor` to simplify
that pattern:
  addVisitor<SomeVisitor>(arg1, arg2, ...);

Differential Revision: https://reviews.llvm.org/D103457
This commit is contained in:
Valeriy Savchenko 2021-06-01 16:26:53 +03:00
parent 8fb6c31cbb
commit 92d03c20ea
6 changed files with 51 additions and 52 deletions

View File

@ -489,11 +489,16 @@ public:
/// ///
/// The visitors should be used when the default trace is not sufficient. /// The visitors should be used when the default trace is not sufficient.
/// For example, they allow constructing a more elaborate trace. /// For example, they allow constructing a more elaborate trace.
/// \sa registerConditionVisitor(), registerTrackNullOrUndefValue(), /// @{
/// registerFindLastStore(), registerNilReceiverVisitor(), and
/// registerVarDeclsLastStore().
void addVisitor(std::unique_ptr<BugReporterVisitor> visitor); void addVisitor(std::unique_ptr<BugReporterVisitor> visitor);
template <class VisitorType, class... Args>
void addVisitor(Args &&... ConstructorArgs) {
addVisitor(
std::make_unique<VisitorType>(std::forward<Args>(ConstructorArgs)...));
}
/// @}
/// Remove all visitors attached to this bug report. /// Remove all visitors attached to this bug report.
void clearVisitors(); void clearVisitors();

View File

@ -2123,7 +2123,7 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C,
os.str(), N); os.str(), N);
R->markInteresting(Sym); R->markInteresting(Sym);
R->addRange(Range); R->addRange(Range);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); R->addVisitor<MallocBugVisitor>(Sym);
C.emitReport(std::move(R)); C.emitReport(std::move(R));
} }
} }
@ -2216,7 +2216,7 @@ void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range,
R->markInteresting(Sym); R->markInteresting(Sym);
R->addRange(Range); R->addRange(Range);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); R->addVisitor<MallocBugVisitor>(Sym);
if (AF == AF_InnerBuffer) if (AF == AF_InnerBuffer)
R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym)); R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym));
@ -2252,7 +2252,7 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range,
R->markInteresting(Sym); R->markInteresting(Sym);
if (PrevSym) if (PrevSym)
R->markInteresting(PrevSym); R->markInteresting(PrevSym);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); R->addVisitor<MallocBugVisitor>(Sym);
C.emitReport(std::move(R)); C.emitReport(std::move(R));
} }
} }
@ -2278,7 +2278,7 @@ void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
*BT_DoubleDelete, "Attempt to delete released memory", N); *BT_DoubleDelete, "Attempt to delete released memory", N);
R->markInteresting(Sym); R->markInteresting(Sym);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); R->addVisitor<MallocBugVisitor>(Sym);
C.emitReport(std::move(R)); C.emitReport(std::move(R));
} }
} }
@ -2308,7 +2308,7 @@ void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range,
R->addRange(Range); R->addRange(Range);
if (Sym) { if (Sym) {
R->markInteresting(Sym); R->markInteresting(Sym);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); R->addVisitor<MallocBugVisitor>(Sym);
} }
C.emitReport(std::move(R)); C.emitReport(std::move(R));
} }
@ -2578,7 +2578,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
*BT_Leak[*CheckKind], os.str(), N, LocUsedForUniqueing, *BT_Leak[*CheckKind], os.str(), N, LocUsedForUniqueing,
AllocNode->getLocationContext()->getDecl()); AllocNode->getLocationContext()->getDecl());
R->markInteresting(Sym); R->markInteresting(Sym);
R->addVisitor(std::make_unique<MallocBugVisitor>(Sym, true)); R->addVisitor<MallocBugVisitor>(Sym, true);
C.emitReport(std::move(R)); C.emitReport(std::move(R));
} }

View File

@ -170,7 +170,7 @@ private:
auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
if (Region) { if (Region) {
R->markInteresting(Region); R->markInteresting(Region);
R->addVisitor(std::make_unique<NullabilityBugVisitor>(Region)); R->addVisitor<NullabilityBugVisitor>(Region);
} }
if (ValueExpr) { if (ValueExpr) {
R->addRange(ValueExpr->getSourceRange()); R->addRange(ValueExpr->getSourceRange());

View File

@ -846,7 +846,7 @@ RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
: PathSensitiveBugReport(D, D.getDescription(), n), Sym(sym), : PathSensitiveBugReport(D, D.getDescription(), n), Sym(sym),
isLeak(isLeak) { isLeak(isLeak) {
if (!isLeak) if (!isLeak)
addVisitor(std::make_unique<RefCountReportVisitor>(sym)); addVisitor<RefCountReportVisitor>(sym);
} }
RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
@ -854,7 +854,7 @@ RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts,
StringRef endText) StringRef endText)
: PathSensitiveBugReport(D, D.getDescription(), endText, n) { : PathSensitiveBugReport(D, D.getDescription(), endText, n) {
addVisitor(std::make_unique<RefCountReportVisitor>(sym)); addVisitor<RefCountReportVisitor>(sym);
} }
void RefLeakReport::deriveParamLocation(CheckerContext &Ctx) { void RefLeakReport::deriveParamLocation(CheckerContext &Ctx) {
@ -984,9 +984,9 @@ void RefLeakReport::findBindingToReport(CheckerContext &Ctx,
// something like derived regions if we want to construct SVal from // something like derived regions if we want to construct SVal from
// Sym. Instead, we take the value that is definitely stored in that // Sym. Instead, we take the value that is definitely stored in that
// region, thus guaranteeing that FindLastStoreBRVisitor will work. // region, thus guaranteeing that FindLastStoreBRVisitor will work.
addVisitor(std::make_unique<FindLastStoreBRVisitor>( addVisitor<FindLastStoreBRVisitor>(
AllVarBindings[0].second.castAs<KnownSVal>(), AllocBindingToReport, AllVarBindings[0].second.castAs<KnownSVal>(), AllocBindingToReport,
false, bugreporter::TrackingKind::Thorough)); false, bugreporter::TrackingKind::Thorough);
} else { } else {
AllocBindingToReport = AllocFirstBinding; AllocBindingToReport = AllocFirstBinding;
} }
@ -1005,5 +1005,5 @@ RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts,
createDescription(Ctx); createDescription(Ctx);
addVisitor(std::make_unique<RefLeakReportVisitor>(Sym, AllocBindingToReport)); addVisitor<RefLeakReportVisitor>(Sym, AllocBindingToReport);
} }

View File

@ -2810,12 +2810,12 @@ Optional<PathDiagnosticBuilder> PathDiagnosticBuilder::findValidReport(
// Register refutation visitors first, if they mark the bug invalid no // Register refutation visitors first, if they mark the bug invalid no
// further analysis is required // further analysis is required
R->addVisitor(std::make_unique<LikelyFalsePositiveSuppressionBRVisitor>()); R->addVisitor<LikelyFalsePositiveSuppressionBRVisitor>();
// Register additional node visitors. // Register additional node visitors.
R->addVisitor(std::make_unique<NilReceiverBRVisitor>()); R->addVisitor<NilReceiverBRVisitor>();
R->addVisitor(std::make_unique<ConditionBRVisitor>()); R->addVisitor<ConditionBRVisitor>();
R->addVisitor(std::make_unique<TagVisitor>()); R->addVisitor<TagVisitor>();
BugReporterContext BRC(Reporter); BugReporterContext BRC(Reporter);
@ -2828,7 +2828,7 @@ Optional<PathDiagnosticBuilder> PathDiagnosticBuilder::findValidReport(
// If crosscheck is enabled, remove all visitors, add the refutation // If crosscheck is enabled, remove all visitors, add the refutation
// visitor and check again // visitor and check again
R->clearVisitors(); R->clearVisitors();
R->addVisitor(std::make_unique<FalsePositiveRefutationBRVisitor>()); R->addVisitor<FalsePositiveRefutationBRVisitor>();
// We don't overwrite the notes inserted by other visitors because the // We don't overwrite the notes inserted by other visitors because the
// refutation manager does not add any new note to the path // refutation manager does not add any new note to the path

View File

@ -852,10 +852,10 @@ public:
bool EnableNullFPSuppression, PathSensitiveBugReport &BR, bool EnableNullFPSuppression, PathSensitiveBugReport &BR,
const SVal V) { const SVal V) {
AnalyzerOptions &Options = N->getState()->getAnalysisManager().options; AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
if (EnableNullFPSuppression && if (EnableNullFPSuppression && Options.ShouldSuppressNullReturnPaths &&
Options.ShouldSuppressNullReturnPaths && V.getAs<Loc>()) V.getAs<Loc>())
BR.addVisitor(std::make_unique<MacroNullReturnSuppressionVisitor>( BR.addVisitor<MacroNullReturnSuppressionVisitor>(R->getAs<SubRegion>(),
R->getAs<SubRegion>(), V)); V);
} }
void* getTag() const { void* getTag() const {
@ -1011,14 +1011,12 @@ public:
AnalyzerOptions &Options = State->getAnalysisManager().options; AnalyzerOptions &Options = State->getAnalysisManager().options;
bool EnableNullFPSuppression = false; bool EnableNullFPSuppression = false;
if (InEnableNullFPSuppression && if (InEnableNullFPSuppression && Options.ShouldSuppressNullReturnPaths)
Options.ShouldSuppressNullReturnPaths)
if (Optional<Loc> RetLoc = RetVal.getAs<Loc>()) if (Optional<Loc> RetLoc = RetVal.getAs<Loc>())
EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue(); EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
BR.addVisitor(std::make_unique<ReturnVisitor>(CalleeContext, BR.addVisitor<ReturnVisitor>(CalleeContext, EnableNullFPSuppression,
EnableNullFPSuppression, Options, TKind);
Options, TKind));
} }
PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N, PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N,
@ -1589,8 +1587,8 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) { dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) {
if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>()) if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>())
BR.addVisitor(std::make_unique<FindLastStoreBRVisitor>( BR.addVisitor<FindLastStoreBRVisitor>(
*KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC)); *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC);
} }
} }
} }
@ -2070,8 +2068,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
// TODO: Shouldn't we track control dependencies of every bug location, rather // TODO: Shouldn't we track control dependencies of every bug location, rather
// than only tracked expressions? // than only tracked expressions?
if (LVState->getAnalysisManager().getAnalyzerOptions().ShouldTrackConditions) if (LVState->getAnalysisManager().getAnalyzerOptions().ShouldTrackConditions)
report.addVisitor(std::make_unique<TrackControlDependencyCondBRVisitor>( report.addVisitor<TrackControlDependencyCondBRVisitor>(InputNode);
InputNode));
// The message send could be nil due to the receiver being nil. // The message send could be nil due to the receiver being nil.
// At this point in the path, the receiver should be live since we are at the // At this point in the path, the receiver should be live since we are at the
@ -2098,8 +2095,8 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
// got initialized. // got initialized.
if (RR && !LVIsNull) if (RR && !LVIsNull)
if (auto KV = LVal.getAs<KnownSVal>()) if (auto KV = LVal.getAs<KnownSVal>())
report.addVisitor(std::make_unique<FindLastStoreBRVisitor>( report.addVisitor<FindLastStoreBRVisitor>(
*KV, RR, EnableNullFPSuppression, TKind, SFC)); *KV, RR, EnableNullFPSuppression, TKind, SFC);
// In case of C++ references, we want to differentiate between a null // In case of C++ references, we want to differentiate between a null
// reference and reference to null pointer. // reference and reference to null pointer.
@ -2112,20 +2109,19 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
// Mark both the variable region and its contents as interesting. // Mark both the variable region and its contents as interesting.
SVal V = LVState->getRawSVal(loc::MemRegionVal(R)); SVal V = LVState->getRawSVal(loc::MemRegionVal(R));
report.addVisitor( report.addVisitor<NoStoreFuncVisitor>(cast<SubRegion>(R), TKind);
std::make_unique<NoStoreFuncVisitor>(cast<SubRegion>(R), TKind));
MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary( MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary(
LVNode, R, EnableNullFPSuppression, report, V); LVNode, R, EnableNullFPSuppression, report, V);
report.markInteresting(V, TKind); report.markInteresting(V, TKind);
report.addVisitor(std::make_unique<UndefOrNullArgVisitor>(R)); report.addVisitor<UndefOrNullArgVisitor>(R);
// If the contents are symbolic and null, find out when they became null. // If the contents are symbolic and null, find out when they became null.
if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true)) if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true))
if (LVState->isNull(V).isConstrainedTrue()) if (LVState->isNull(V).isConstrainedTrue())
report.addVisitor(std::make_unique<TrackConstraintBRVisitor>( report.addVisitor<TrackConstraintBRVisitor>(V.castAs<DefinedSVal>(),
V.castAs<DefinedSVal>(), false)); false);
// Add visitor, which will suppress inline defensive checks. // Add visitor, which will suppress inline defensive checks.
if (auto DV = V.getAs<DefinedSVal>()) if (auto DV = V.getAs<DefinedSVal>())
@ -2135,14 +2131,13 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
// was evaluated. InputNode may as well be too early here, because // was evaluated. InputNode may as well be too early here, because
// the symbol is already dead; this, however, is fine because we can // the symbol is already dead; this, however, is fine because we can
// still find the node in which it collapsed to null previously. // still find the node in which it collapsed to null previously.
report.addVisitor( report.addVisitor<SuppressInlineDefensiveChecksVisitor>(*DV,
std::make_unique<SuppressInlineDefensiveChecksVisitor>( InputNode);
*DV, InputNode));
} }
if (auto KV = V.getAs<KnownSVal>()) if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(std::make_unique<FindLastStoreBRVisitor>( report.addVisitor<FindLastStoreBRVisitor>(
*KV, R, EnableNullFPSuppression, TKind, SFC)); *KV, R, EnableNullFPSuppression, TKind, SFC);
return true; return true;
} }
} }
@ -2177,19 +2172,18 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
RVal = LVState->getSVal(L->getRegion()); RVal = LVState->getSVal(L->getRegion());
if (CanDereference) { if (CanDereference) {
report.addVisitor( report.addVisitor<UndefOrNullArgVisitor>(L->getRegion());
std::make_unique<UndefOrNullArgVisitor>(L->getRegion()));
if (auto KV = RVal.getAs<KnownSVal>()) if (auto KV = RVal.getAs<KnownSVal>())
report.addVisitor(std::make_unique<FindLastStoreBRVisitor>( report.addVisitor<FindLastStoreBRVisitor>(
*KV, L->getRegion(), EnableNullFPSuppression, TKind, SFC)); *KV, L->getRegion(), EnableNullFPSuppression, TKind, SFC);
} }
const MemRegion *RegionRVal = RVal.getAsRegion(); const MemRegion *RegionRVal = RVal.getAsRegion();
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) { if (isa_and_nonnull<SymbolicRegion>(RegionRVal)) {
report.markInteresting(RegionRVal, TKind); report.markInteresting(RegionRVal, TKind);
report.addVisitor(std::make_unique<TrackConstraintBRVisitor>( report.addVisitor<TrackConstraintBRVisitor>(loc::MemRegionVal(RegionRVal),
loc::MemRegionVal(RegionRVal), /*assumption=*/false)); /*assumption=*/false);
} }
} }