Skip to content

Commit cf75a85

Browse files
DiscookieDiscookie
committed
[clang][dataflow] Add null-check after dereference checker
1 parent 7af4e8b commit cf75a85

File tree

12 files changed

+1785
-1
lines changed

12 files changed

+1785
-1
lines changed

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

+3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include "NoEscapeCheck.h"
4949
#include "NonZeroEnumToBoolConversionCheck.h"
5050
#include "NotNullTerminatedResultCheck.h"
51+
#include "NullCheckAfterDereferenceCheck.h"
5152
#include "OptionalValueConversionCheck.h"
5253
#include "ParentVirtualCallCheck.h"
5354
#include "PosixReturnCheck.h"
@@ -180,6 +181,8 @@ class BugproneModule : public ClangTidyModule {
180181
CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
181182
CheckFactories.registerCheck<ReservedIdentifierCheck>(
182183
"bugprone-reserved-identifier");
184+
CheckFactories.registerCheck<NullCheckAfterDereferenceCheck>(
185+
"bugprone-null-check-after-dereference");
183186
CheckFactories.registerCheck<SharedPtrArrayMismatchCheck>(
184187
"bugprone-shared-ptr-array-mismatch");
185188
CheckFactories.registerCheck<SignalHandlerCheck>("bugprone-signal-handler");

‎clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ add_clang_library(clangTidyBugproneModule
4444
NoEscapeCheck.cpp
4545
NonZeroEnumToBoolConversionCheck.cpp
4646
NotNullTerminatedResultCheck.cpp
47+
NullCheckAfterDereferenceCheck.cpp
4748
OptionalValueConversionCheck.cpp
4849
ParentVirtualCallCheck.cpp
4950
PosixReturnCheck.cpp

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

+175
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
//===--- NullCheckAfterDereferenceCheck.cpp - clang-tidy-------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "NullCheckAfterDereferenceCheck.h"
10+
#include "clang/AST/ASTContext.h"
11+
#include "clang/AST/DeclCXX.h"
12+
#include "clang/AST/DeclTemplate.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "clang/ASTMatchers/ASTMatchers.h"
15+
#include "clang/Analysis/CFG.h"
16+
#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
17+
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
18+
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
19+
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
20+
#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
21+
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
22+
#include "clang/Basic/SourceLocation.h"
23+
#include "llvm/ADT/Any.h"
24+
#include "llvm/ADT/STLExtras.h"
25+
#include "llvm/Support/Error.h"
26+
#include <memory>
27+
#include <vector>
28+
29+
namespace clang::tidy::bugprone {
30+
31+
using ast_matchers::MatchFinder;
32+
using dataflow::NullPointerAnalysisModel;
33+
using dataflow::NullCheckAfterDereferenceDiagnoser;
34+
35+
static constexpr llvm::StringLiteral FuncID("fun");
36+
37+
struct ExpandedResult {
38+
SourceLocation WarningLoc;
39+
std::optional<SourceLocation> DerefLoc;
40+
};
41+
42+
using ExpandedResultType = std::pair<std::vector<ExpandedResult>,
43+
std::vector<ExpandedResult>>;
44+
45+
static std::optional<ExpandedResultType> analyzeFunction(
46+
const FunctionDecl &FuncDecl) {
47+
using dataflow::ControlFlowContext;
48+
using dataflow::DataflowAnalysisState;
49+
using llvm::Expected;
50+
51+
ASTContext &ASTCtx = FuncDecl.getASTContext();
52+
53+
if (FuncDecl.getBody() == nullptr) {
54+
return std::nullopt;
55+
}
56+
57+
Expected<ControlFlowContext> Context =
58+
ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
59+
if (!Context)
60+
return std::nullopt;
61+
62+
dataflow::DataflowAnalysisContext AnalysisContext(
63+
std::make_unique<dataflow::WatchedLiteralsSolver>());
64+
dataflow::Environment Env(AnalysisContext, FuncDecl);
65+
NullPointerAnalysisModel Analysis(ASTCtx);
66+
NullCheckAfterDereferenceDiagnoser Diagnoser;
67+
NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
68+
69+
using LatticeState = DataflowAnalysisState<NullPointerAnalysisModel::Lattice>;
70+
using DetailMaybeLatticeStates = std::vector<std::optional<LatticeState>>;
71+
72+
auto DiagnoserImpl = [&ASTCtx, &Diagnoser, &Diagnostics](
73+
const CFGElement &Elt,
74+
const LatticeState &S) mutable -> void {
75+
auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, S.Env);
76+
llvm::move(EltDiagnostics.first,
77+
std::back_inserter(Diagnostics.first));
78+
llvm::move(EltDiagnostics.second,
79+
std::back_inserter(Diagnostics.second));
80+
};
81+
82+
Expected<DetailMaybeLatticeStates>
83+
BlockToOutputState = dataflow::runDataflowAnalysis(
84+
*Context, Analysis, Env, DiagnoserImpl);
85+
86+
if (llvm::Error E = BlockToOutputState.takeError()) {
87+
llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
88+
<< ".\n";
89+
return std::nullopt;
90+
}
91+
92+
ExpandedResultType ExpandedDiagnostics;
93+
94+
llvm::transform(Diagnostics.first,
95+
std::back_inserter(ExpandedDiagnostics.first),
96+
[&](SourceLocation WarningLoc) -> ExpandedResult {
97+
if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
98+
auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
99+
return {WarningLoc, DerefExpr->getBeginLoc()};
100+
}
101+
102+
return {WarningLoc, std::nullopt};
103+
});
104+
105+
llvm::transform(Diagnostics.second,
106+
std::back_inserter(ExpandedDiagnostics.second),
107+
[&](SourceLocation WarningLoc) -> ExpandedResult {
108+
if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
109+
auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
110+
return {WarningLoc, DerefExpr->getBeginLoc()};
111+
}
112+
113+
return {WarningLoc, std::nullopt};
114+
});
115+
116+
return ExpandedDiagnostics;
117+
}
118+
119+
void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
120+
using namespace ast_matchers;
121+
122+
auto hasPointerValue =
123+
hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
124+
Finder->addMatcher(
125+
decl(anyOf(functionDecl(unless(isExpansionInSystemHeader()),
126+
// FIXME: Remove the filter below when lambdas are
127+
// well supported by the check.
128+
unless(hasDeclContext(cxxRecordDecl(isLambda()))),
129+
hasBody(hasPointerValue)),
130+
cxxConstructorDecl(hasAnyConstructorInitializer(
131+
withInitializer(hasPointerValue)))))
132+
.bind(FuncID),
133+
this);
134+
}
135+
136+
void NullCheckAfterDereferenceCheck::check(
137+
const MatchFinder::MatchResult &Result) {
138+
if (Result.SourceManager->getDiagnostics().hasUncompilableErrorOccurred())
139+
return;
140+
141+
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(FuncID);
142+
assert(FuncDecl && "invalid FuncDecl matcher");
143+
if (FuncDecl->isTemplated())
144+
return;
145+
146+
if (const auto Diagnostics = analyzeFunction(*FuncDecl)) {
147+
const auto& [CheckWhenNullLocations, CheckAfterDereferenceLocations] =
148+
*Diagnostics;
149+
150+
for (const auto [WarningLoc, DerefLoc] : CheckAfterDereferenceLocations) {
151+
diag(WarningLoc,
152+
"pointer value is checked even though "
153+
"it cannot be null at this point");
154+
155+
if (DerefLoc) {
156+
diag(*DerefLoc,
157+
"one of the locations where the pointer's value cannot be null",
158+
DiagnosticIDs::Note);
159+
}
160+
}
161+
162+
for (const auto [WarningLoc, DerefLoc] : CheckWhenNullLocations) {
163+
diag(WarningLoc,
164+
"pointer value is checked but it can only be null at this point");
165+
166+
if (DerefLoc) {
167+
diag(*DerefLoc,
168+
"one of the locations where the pointer's value can only be null",
169+
DiagnosticIDs::Note);
170+
}
171+
}
172+
}
173+
}
174+
175+
} // namespace clang::tidy::bugprone

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

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//===--- NullCheckAfterDereferenceCheck.h - clang-tidy ----------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
15+
namespace clang::tidy::bugprone {
16+
17+
/// Finds checks for pointer nullability after a pointer has already been
18+
/// dereferenced, using the data-flow framework.
19+
///
20+
/// For the user-facing documentation see:
21+
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/null-check-after-dereference.html
22+
class NullCheckAfterDereferenceCheck : public ClangTidyCheck {
23+
public:
24+
NullCheckAfterDereferenceCheck(StringRef Name, ClangTidyContext *Context)
25+
: ClangTidyCheck(Name, Context) {}
26+
27+
// The data-flow framework does not support C because of AST differences.
28+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
29+
return LangOpts.CPlusPlus;
30+
}
31+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
32+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
33+
};
34+
35+
} // namespace clang::tidy::bugprone
36+
37+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H

‎clang-tools-extra/clangd/TidyProvider.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,10 @@ TidyProvider disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks) {
219219
"-bugprone-use-after-move",
220220
// Alias for bugprone-use-after-move.
221221
"-hicpp-invalid-access-moved",
222-
// Check uses dataflow analysis, which might hang/crash unexpectedly on
222+
// Checks use dataflow analysis, which might hang/crash unexpectedly on
223223
// incomplete code.
224224
"-bugprone-unchecked-optional-access",
225+
"-bugprone-null-check-after-dereference",
225226

226227
// ----- Performance problems -----
227228

0 commit comments

Comments
 (0)