Skip to content

Commit 93aaeac

Browse files
DiscookieDiscookie
committed
Use new caller interface for runDataflowAnalysis
1 parent 3447dc4 commit 93aaeac

File tree

3 files changed

+68
-83
lines changed

3 files changed

+68
-83
lines changed

‎clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp

+39-58
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,16 @@ namespace clang::tidy::bugprone {
3131
using ast_matchers::MatchFinder;
3232
using dataflow::NullCheckAfterDereferenceDiagnoser;
3333
using dataflow::NullPointerAnalysisModel;
34+
using Diagnoser = NullCheckAfterDereferenceDiagnoser;
3435

3536
static constexpr llvm::StringLiteral FuncID("fun");
3637

3738
struct ExpandedResult {
38-
SourceLocation WarningLoc;
39+
Diagnoser::DiagnosticEntry Entry;
3940
std::optional<SourceLocation> DerefLoc;
4041
};
4142

42-
using ExpandedResultType =
43-
std::pair<std::vector<ExpandedResult>, std::vector<ExpandedResult>>;
43+
using ExpandedResultType = llvm::SmallVector<ExpandedResult>;
4444

4545
static std::optional<ExpandedResultType>
4646
analyzeFunction(const FunctionDecl &FuncDecl) {
@@ -63,51 +63,30 @@ analyzeFunction(const FunctionDecl &FuncDecl) {
6363
std::make_unique<dataflow::WatchedLiteralsSolver>());
6464
dataflow::Environment Env(AnalysisContext, FuncDecl);
6565
NullPointerAnalysisModel Analysis(ASTCtx);
66-
NullCheckAfterDereferenceDiagnoser Diagnoser;
67-
NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
66+
Diagnoser Diagnoser;
6867

69-
using State = DataflowAnalysisState<NullPointerAnalysisModel::Lattice>;
70-
using DetailMaybeStates = std::vector<std::optional<State>>;
68+
Expected<Diagnoser::ResultType> Diagnostics =
69+
dataflow::diagnoseFunction<NullPointerAnalysisModel, Diagnoser::DiagnosticEntry>(
70+
FuncDecl, ASTCtx, Diagnoser);
7171

72-
auto DiagnoserImpl = [&ASTCtx, &Diagnoser,
73-
&Diagnostics](const CFGElement &Elt,
74-
const State &S) mutable -> void {
75-
auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, S.Env);
76-
llvm::move(EltDiagnostics.first, std::back_inserter(Diagnostics.first));
77-
llvm::move(EltDiagnostics.second, std::back_inserter(Diagnostics.second));
78-
};
79-
80-
Expected<DetailMaybeStates> BlockToOutputState =
81-
dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
82-
83-
if (llvm::Error E = BlockToOutputState.takeError()) {
72+
73+
if (llvm::Error E = Diagnostics.takeError()) {
8474
llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
8575
<< ".\n";
8676
return std::nullopt;
8777
}
8878

8979
ExpandedResultType ExpandedDiagnostics;
9080

91-
llvm::transform(Diagnostics.first,
92-
std::back_inserter(ExpandedDiagnostics.first),
93-
[&](SourceLocation WarningLoc) -> ExpandedResult {
94-
if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
95-
auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
96-
return {WarningLoc, DerefExpr->getBeginLoc()};
97-
}
98-
99-
return {WarningLoc, std::nullopt};
100-
});
101-
102-
llvm::transform(Diagnostics.second,
103-
std::back_inserter(ExpandedDiagnostics.second),
104-
[&](SourceLocation WarningLoc) -> ExpandedResult {
105-
if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
81+
llvm::transform(*Diagnostics,
82+
std::back_inserter(ExpandedDiagnostics),
83+
[&](Diagnoser::DiagnosticEntry Entry) -> ExpandedResult {
84+
if (auto Val = Diagnoser.WarningLocToVal[Entry.Location];
10685
auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
107-
return {WarningLoc, DerefExpr->getBeginLoc()};
86+
return {Entry, DerefExpr->getBeginLoc()};
10887
}
10988

110-
return {WarningLoc, std::nullopt};
89+
return {Entry, std::nullopt};
11190
});
11291

11392
return ExpandedDiagnostics;
@@ -143,28 +122,30 @@ void NullCheckAfterDereferenceCheck::check(
143122
return;
144123

145124
if (const auto Diagnostics = analyzeFunction(*FuncDecl)) {
146-
const auto &[CheckWhenNullLocations, CheckWhenNonnullLocations] =
147-
*Diagnostics;
148-
149-
for (const auto [WarningLoc, DerefLoc] : CheckWhenNonnullLocations) {
150-
diag(WarningLoc, "pointer value is checked even though "
151-
"it cannot be null at this point");
152-
153-
if (DerefLoc) {
154-
diag(*DerefLoc,
155-
"one of the locations where the pointer's value cannot be null",
156-
DiagnosticIDs::Note);
157-
}
158-
}
159-
160-
for (const auto [WarningLoc, DerefLoc] : CheckWhenNullLocations) {
161-
diag(WarningLoc,
162-
"pointer value is checked but it can only be null at this point");
163-
164-
if (DerefLoc) {
165-
diag(*DerefLoc,
166-
"one of the locations where the pointer's value can only be null",
167-
DiagnosticIDs::Note);
125+
for (const auto [Entry, DerefLoc] : *Diagnostics) {
126+
const auto [WarningLoc, Type] = Entry;
127+
128+
switch (Type) {
129+
case Diagnoser::DiagnosticType::CheckAfterDeref:
130+
diag(WarningLoc, "pointer value is checked even though "
131+
"it cannot be null at this point");
132+
133+
if (DerefLoc) {
134+
diag(*DerefLoc,
135+
"one of the locations where the pointer's value cannot be null",
136+
DiagnosticIDs::Note);
137+
}
138+
break;
139+
case Diagnoser::DiagnosticType::CheckWhenNull:
140+
diag(WarningLoc,
141+
"pointer value is checked but it can only be null at this point");
142+
143+
if (DerefLoc) {
144+
diag(*DerefLoc,
145+
"one of the locations where the pointer's value can only be null",
146+
DiagnosticIDs::Note);
147+
}
148+
break;
168149
}
169150
}
170151
}

‎clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h

+9-4
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,13 @@ class NullCheckAfterDereferenceDiagnoser {
8181

8282
/// Returns source locations for pointers that were checked when known to be
8383
// null, and checked after already dereferenced, respectively.
84-
using ResultType =
85-
std::pair<std::vector<SourceLocation>, std::vector<SourceLocation>>;
84+
enum class DiagnosticType { CheckWhenNull, CheckAfterDeref };
85+
86+
struct DiagnosticEntry {
87+
SourceLocation Location;
88+
DiagnosticType Type;
89+
};
90+
using ResultType = llvm::SmallVector<DiagnosticEntry>;
8691

8792
// Maps a pointer's Value to a dereference, null-assignment, etc.
8893
// This is used later to construct the Note tag.
@@ -96,8 +101,8 @@ class NullCheckAfterDereferenceDiagnoser {
96101
public:
97102
NullCheckAfterDereferenceDiagnoser();
98103

99-
ResultType diagnose(ASTContext &Ctx, const CFGElement *Elt,
100-
const Environment &Env);
104+
ResultType operator()(const CFGElement &Elt, ASTContext &Ctx,
105+
const TransferStateForDiagnostics<NoopLattice> &State);
101106
};
102107

103108
} // namespace clang::dataflow

‎clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp

+20-21
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ namespace clang::dataflow {
3636

3737
namespace {
3838
using namespace ast_matchers;
39+
using Diagnoser = NullCheckAfterDereferenceDiagnoser;
3940

4041
constexpr char kCond[] = "condition";
4142
constexpr char kVar[] = "var";
@@ -423,9 +424,9 @@ void matchAnyPointerExpr(const Expr *fncall,
423424
setUnknownValue(*Var, *RootValue, Env);
424425
}
425426

426-
NullCheckAfterDereferenceDiagnoser::ResultType
427+
Diagnoser::ResultType
427428
diagnoseDerefLocation(const Expr *Deref, const MatchFinder::MatchResult &Result,
428-
NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
429+
Diagnoser::DiagnoseArgs &Data) {
429430
auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
430431

431432
const auto *Var = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -444,10 +445,9 @@ diagnoseDerefLocation(const Expr *Deref, const MatchFinder::MatchResult &Result,
444445
return {};
445446
}
446447

447-
NullCheckAfterDereferenceDiagnoser::ResultType
448-
diagnoseAssignLocation(const Expr *Assign,
449-
const MatchFinder::MatchResult &Result,
450-
NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
448+
Diagnoser::ResultType diagnoseAssignLocation(const Expr *Assign,
449+
const MatchFinder::MatchResult &Result,
450+
Diagnoser::DiagnoseArgs &Data) {
451451
auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
452452

453453
const auto *RHSVar = Result.Nodes.getNodeAs<Expr>(kValue);
@@ -468,7 +468,7 @@ diagnoseAssignLocation(const Expr *Assign,
468468
NullCheckAfterDereferenceDiagnoser::ResultType
469469
diagnoseNullCheckExpr(const Expr *NullCheck,
470470
const MatchFinder::MatchResult &Result,
471-
const NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
471+
const Diagnoser::DiagnoseArgs &Data) {
472472
auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
473473

474474
const auto *Var = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -488,7 +488,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
488488
assert(Inserted && "multiple warnings at the same source location");
489489
(void)Inserted;
490490

491-
return {{}, {Var->getBeginLoc()}};
491+
return {{Var->getBeginLoc(), Diagnoser::DiagnosticType::CheckAfterDeref}};
492492
}
493493

494494
if (IsNull && !IsNonnull) {
@@ -497,7 +497,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
497497
assert(Inserted && "multiple warnings at the same source location");
498498
(void)Inserted;
499499

500-
return {{Var->getBeginLoc()}, {}};
500+
return {{Var->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull}};
501501
}
502502
}
503503

@@ -513,7 +513,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
513513

514514
NullCheckAfterDereferenceDiagnoser::ResultType
515515
diagnoseEqualExpr(const Expr *PtrCheck, const MatchFinder::MatchResult &Result,
516-
NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
516+
Diagnoser::DiagnoseArgs &Data) {
517517
auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
518518

519519
const auto *LHSVar = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -522,23 +522,23 @@ diagnoseEqualExpr(const Expr *PtrCheck, const MatchFinder::MatchResult &Result,
522522
assert(RHSVar != nullptr);
523523

524524
Arena &A = Env.arena();
525-
std::vector<SourceLocation> NullVarLocations;
525+
llvm::SmallVector<Diagnoser::DiagnosticEntry> NullVarLocations;
526526

527527
if (Value *LHSValue = Env.getValue(*LHSVar);
528528
LHSValue->getProperty(kIsNonnull) &&
529529
Env.proves(A.makeNot(getVal(kIsNonnull, *LHSValue).formula()))) {
530530
WarningLocToVal.try_emplace(LHSVar->getBeginLoc(), LHSValue);
531-
NullVarLocations.push_back(LHSVar->getBeginLoc());
531+
NullVarLocations.push_back({LHSVar->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull});
532532
}
533533

534534
if (Value *RHSValue = Env.getValue(*RHSVar);
535535
RHSValue->getProperty(kIsNonnull) &&
536536
Env.proves(A.makeNot(getVal(kIsNonnull, *RHSValue).formula()))) {
537537
WarningLocToVal.try_emplace(RHSVar->getBeginLoc(), RHSValue);
538-
NullVarLocations.push_back(RHSVar->getBeginLoc());
538+
NullVarLocations.push_back({RHSVar->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull});
539539
}
540540

541-
return {NullVarLocations, {}};
541+
return NullVarLocations;
542542
}
543543

544544
auto buildTransferMatchSwitch() {
@@ -556,8 +556,7 @@ auto buildTransferMatchSwitch() {
556556
}
557557

558558
auto buildDiagnoseMatchSwitch() {
559-
return CFGMatchSwitchBuilder<NullCheckAfterDereferenceDiagnoser::DiagnoseArgs,
560-
NullCheckAfterDereferenceDiagnoser::ResultType>()
559+
return CFGMatchSwitchBuilder<Diagnoser::DiagnoseArgs, Diagnoser::ResultType>()
561560
.CaseOfCFGStmt<Expr>(derefMatcher(), diagnoseDerefLocation)
562561
.CaseOfCFGStmt<Expr>(arrowMatcher(), diagnoseDerefLocation)
563562
.CaseOfCFGStmt<Expr>(assignMatcher(), diagnoseAssignLocation)
@@ -761,11 +760,11 @@ NullCheckAfterDereferenceDiagnoser::NullCheckAfterDereferenceDiagnoser()
761760
: DiagnoseMatchSwitch(buildDiagnoseMatchSwitch()) {}
762761

763762
NullCheckAfterDereferenceDiagnoser::ResultType
764-
NullCheckAfterDereferenceDiagnoser::diagnose(ASTContext &Ctx,
765-
const CFGElement *Elt,
766-
const Environment &Env) {
767-
DiagnoseArgs Args = {ValToDerefLoc, WarningLocToVal, Env};
768-
return DiagnoseMatchSwitch(*Elt, Ctx, Args);
763+
NullCheckAfterDereferenceDiagnoser::operator()(
764+
const CFGElement &Elt, ASTContext &Ctx,
765+
const TransferStateForDiagnostics<NoopLattice> &State) {
766+
DiagnoseArgs Args = {ValToDerefLoc, WarningLocToVal, State.Env};
767+
return DiagnoseMatchSwitch(Elt, Ctx, Args);
769768
}
770769

771770
} // namespace clang::dataflow

0 commit comments

Comments
 (0)