[clang] [clang-tools-extra] [clang][dataflow] Add null-check after dereference checker (PR #84166)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 6 05:23:51 PST 2024
github-actions[bot] wrote:
<!--LLVM CODE FORMAT COMMENT: {clang-format}-->
:warning: C/C++ code formatter, clang-format found issues in your code. :warning:
<details>
<summary>
You can test this locally with the following command:
</summary>
``````````bash
git-clang-format --diff 7af4e8bcc354d2bd7e46ecf547172b1f19ddde3e cf75a856dd75a87c7d2e9f307c206f4283e8b8f7 -- clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.h clang-tools-extra/test/clang-tidy/checkers/bugprone/null-check-after-dereference.cpp clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clangd/TidyProvider.cpp
``````````
</details>
<details>
<summary>
View the diff from clang-format here.
</summary>
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
index 1ba4edc1ea..7ef3169cc6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
@@ -29,8 +29,8 @@
namespace clang::tidy::bugprone {
using ast_matchers::MatchFinder;
-using dataflow::NullPointerAnalysisModel;
using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
static constexpr llvm::StringLiteral FuncID("fun");
@@ -39,11 +39,11 @@ struct ExpandedResult {
std::optional<SourceLocation> DerefLoc;
};
-using ExpandedResultType = std::pair<std::vector<ExpandedResult>,
- std::vector<ExpandedResult>>;
+using ExpandedResultType =
+ std::pair<std::vector<ExpandedResult>, std::vector<ExpandedResult>>;
-static std::optional<ExpandedResultType> analyzeFunction(
- const FunctionDecl &FuncDecl) {
+static std::optional<ExpandedResultType>
+analyzeFunction(const FunctionDecl &FuncDecl) {
using dataflow::ControlFlowContext;
using dataflow::DataflowAnalysisState;
using llvm::Expected;
@@ -69,19 +69,16 @@ static std::optional<ExpandedResultType> analyzeFunction(
using LatticeState = DataflowAnalysisState<NullPointerAnalysisModel::Lattice>;
using DetailMaybeLatticeStates = std::vector<std::optional<LatticeState>>;
- auto DiagnoserImpl = [&ASTCtx, &Diagnoser, &Diagnostics](
- const CFGElement &Elt,
- const LatticeState &S) mutable -> void {
- auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, S.Env);
- llvm::move(EltDiagnostics.first,
- std::back_inserter(Diagnostics.first));
- llvm::move(EltDiagnostics.second,
- std::back_inserter(Diagnostics.second));
- };
+ auto DiagnoserImpl = [&ASTCtx, &Diagnoser,
+ &Diagnostics](const CFGElement &Elt,
+ const LatticeState &S) mutable -> void {
+ auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, S.Env);
+ llvm::move(EltDiagnostics.first, std::back_inserter(Diagnostics.first));
+ llvm::move(EltDiagnostics.second, std::back_inserter(Diagnostics.second));
+ };
- Expected<DetailMaybeLatticeStates>
- BlockToOutputState = dataflow::runDataflowAnalysis(
- *Context, Analysis, Env, DiagnoserImpl);
+ Expected<DetailMaybeLatticeStates> BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
if (llvm::Error E = BlockToOutputState.takeError()) {
llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
@@ -139,18 +136,17 @@ void NullCheckAfterDereferenceCheck::check(
return;
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(FuncID);
- assert(FuncDecl && "invalid FuncDecl matcher");
+ assert(FuncDecl && "invalid FuncDecl matcher");
if (FuncDecl->isTemplated())
return;
if (const auto Diagnostics = analyzeFunction(*FuncDecl)) {
- const auto& [CheckWhenNullLocations, CheckAfterDereferenceLocations] =
+ const auto &[CheckWhenNullLocations, CheckAfterDereferenceLocations] =
*Diagnostics;
for (const auto [WarningLoc, DerefLoc] : CheckAfterDereferenceLocations) {
- diag(WarningLoc,
- "pointer value is checked even though "
- "it cannot be null at this point");
+ diag(WarningLoc, "pointer value is checked even though "
+ "it cannot be null at this point");
if (DerefLoc) {
diag(*DerefLoc,
@@ -162,7 +158,7 @@ void NullCheckAfterDereferenceCheck::check(
for (const auto [WarningLoc, DerefLoc] : CheckWhenNullLocations) {
diag(WarningLoc,
"pointer value is checked but it can only be null at this point");
-
+
if (DerefLoc) {
diag(*DerefLoc,
"one of the locations where the pointer's value can only be null",
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h
index ae94c06206..16b28ecc9e 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h
@@ -69,8 +69,8 @@ public:
Environment &Env);
void join(QualType Type, const Value &Val1, const Environment &Env1,
- const Value &Val2, const Environment &Env2, Value &MergedVal,
- Environment &MergedEnv) override;
+ const Value &Val2, const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) override;
ComparisonResult compare(QualType Type, const Value &Val1,
const Environment &Env1, const Value &Val2,
diff --git a/clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp
index 36c2e79880..49750406a3 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp
@@ -156,7 +156,7 @@ void initializeRootValue(Value &RootValue, Environment &Env) {
BoolValue *IsNull = cast_or_null<BoolValue>(RootValue.getProperty(kIsNull));
BoolValue *IsNonnull =
cast_or_null<BoolValue>(RootValue.getProperty(kIsNonnull));
-
+
if (!IsNull) {
IsNull = &A.makeAtomValue();
RootValue.setProperty(kIsNull, *IsNull);
@@ -343,13 +343,13 @@ diagnoseDerefLocation(const Expr *Deref, const MatchFinder::MatchResult &Result,
}
NullCheckAfterDereferenceDiagnoser::ResultType
-diagnoseAssignLocation(const Expr *Assign,
+diagnoseAssignLocation(const Expr *Assign,
const MatchFinder::MatchResult &Result,
NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
const auto *RHSVar = Result.Nodes.getNodeAs<Expr>(kValue);
- assert (RHSVar != nullptr);
+ assert(RHSVar != nullptr);
auto *RHSValue = Env.getValue(*RHSVar);
if (!RHSValue)
@@ -418,8 +418,7 @@ auto buildTransferMatchSwitch() {
}
auto buildBranchTransferMatchSwitch() {
- return ASTMatchSwitchBuilder<Stmt,
- NullPointerAnalysisModel::TransferArgs>()
+ return ASTMatchSwitchBuilder<Stmt, NullPointerAnalysisModel::TransferArgs>()
.CaseOf<CastExpr>(castExprMatcher(), matchCastExpr)
.Build();
}
@@ -436,26 +435,23 @@ auto buildDiagnoseMatchSwitch() {
} // namespace
-NullPointerAnalysisModel::NullPointerAnalysisModel(
- ASTContext &Context)
+NullPointerAnalysisModel::NullPointerAnalysisModel(ASTContext &Context)
: DataflowAnalysis<NullPointerAnalysisModel, NoopLattice>(Context),
TransferMatchSwitch(buildTransferMatchSwitch()),
BranchTransferMatchSwitch(buildBranchTransferMatchSwitch()) {}
-ast_matchers::StatementMatcher
-NullPointerAnalysisModel::ptrValueMatcher() {
+ast_matchers::StatementMatcher NullPointerAnalysisModel::ptrValueMatcher() {
return ptrToVar();
}
-void NullPointerAnalysisModel::transfer(const CFGElement &E,
- NoopLattice &State,
- Environment &Env) {
+void NullPointerAnalysisModel::transfer(const CFGElement &E, NoopLattice &State,
+ Environment &Env) {
TransferMatchSwitch(E, getASTContext(), Env);
}
void NullPointerAnalysisModel::transferBranch(bool Branch, const Stmt *E,
- NoopLattice &State,
- Environment &Env) {
+ NoopLattice &State,
+ Environment &Env) {
if (!E)
return;
@@ -464,11 +460,9 @@ void NullPointerAnalysisModel::transferBranch(bool Branch, const Stmt *E,
}
void NullPointerAnalysisModel::join(QualType Type, const Value &Val1,
- const Environment &Env1,
- const Value &Val2,
- const Environment &Env2,
- Value &MergedVal,
- Environment &MergedEnv) {
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) {
if (!Type->isAnyPointerType())
return;
@@ -513,9 +507,11 @@ void NullPointerAnalysisModel::join(QualType Type, const Value &Val1,
MergedEnv.assume(MergedEnv.makeOr(NonnullValue, NullValue).formula());
}
-ComparisonResult NullPointerAnalysisModel::compare(
- QualType Type, const Value &Val1, const Environment &Env1,
- const Value &Val2, const Environment &Env2) {
+ComparisonResult NullPointerAnalysisModel::compare(QualType Type,
+ const Value &Val1,
+ const Environment &Env1,
+ const Value &Val2,
+ const Environment &Env2) {
if (!Type->isAnyPointerType())
return ComparisonResult::Unknown;
@@ -599,9 +595,9 @@ ComparisonResult compareAndReplace(QualType Type, Value &Val1,
}
Value *NullPointerAnalysisModel::widen(QualType Type, Value &Prev,
- const Environment &PrevEnv,
- Value &Current,
- Environment &CurrentEnv) {
+ const Environment &PrevEnv,
+ Value &Current,
+ Environment &CurrentEnv) {
if (!Type->isAnyPointerType())
return nullptr;
diff --git a/clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp
index 18ab99e8c2..d63430de2a 100644
--- a/clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp
@@ -17,6 +17,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@@ -35,7 +36,6 @@
#include "llvm/Support/Error.h"
#include "llvm/Testing/ADT/StringMapEntry.h"
#include "llvm/Testing/Support/Error.h"
-#include "TestingSupport.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <cstdint>
@@ -91,8 +91,7 @@ std::string checkNullabilityState(BoolValue *value, const Environment &Env) {
// address than the one stored in the framework.
auto nameToVar(llvm::StringRef name) {
return declRefExpr(hasType(isAnyPointer()),
- hasDeclaration(namedDecl(hasName(name)).bind(kVar)))
- ;
+ hasDeclaration(namedDecl(hasName(name)).bind(kVar)));
}
using ::clang::dataflow::test::AnalysisInputs;
@@ -147,10 +146,10 @@ void ExpectDataflowResult(llvm::StringRef Code, MatcherFactory Expectations) {
return NullPointerAnalysisModel(C);
})
.withASTBuildArgs({"-fsyntax-only", "-std=c++17"}),
- /*VerifyResults=*/[&Expectations](
- const llvm::StringMap<DataflowAnalysisState<
- NullPointerAnalysisModel::Lattice>> &Results,
- const AnalysisOutputs &Output) {
+ /*VerifyResults=*/
+ [&Expectations](const llvm::StringMap<DataflowAnalysisState<
+ NullPointerAnalysisModel::Lattice>> &Results,
+ const AnalysisOutputs &Output) {
EXPECT_THAT(Results, Expectations(Output));
}),
llvm::Succeeded());
@@ -243,8 +242,7 @@ TEST(NullCheckAfterDereferenceTest, UnrelatedCondition) {
kBoolFalse, kBoolTrue))),
IsStringMapEntry("b_p_true", HoldsVariable("p", Output,
HasNullabilityState(
- kBoolFalse,
- kBoolTrue))),
+ kBoolFalse, kBoolTrue))),
// FIXME: Flow condition is false in this last entry,
// should test that instead of an invalid state
IsStringMapEntry(
@@ -281,17 +279,15 @@ TEST(NullCheckAfterDereferenceTest, AssignmentOfCommonValues) {
return UnorderedElementsAre(
// FIXME: Recognize that malloc (and other functions) are nullable
IsStringMapEntry(
- "p_malloc",
+ "p_malloc",
HoldsVariable("p", Output,
HasNullabilityState(kBoolUnknown, kBoolUnknown))),
- IsStringMapEntry(
- "p_true",
- HoldsVariable("p", Output,
- HasNullabilityState(kBoolFalse, kBoolTrue))),
- IsStringMapEntry(
- "p_false",
- HoldsVariable("p", Output,
- HasNullabilityState(kBoolTrue, kBoolFalse))),
+ IsStringMapEntry("p_true", HoldsVariable("p", Output,
+ HasNullabilityState(
+ kBoolFalse, kBoolTrue))),
+ IsStringMapEntry("p_false", HoldsVariable("p", Output,
+ HasNullabilityState(
+ kBoolTrue, kBoolFalse))),
IsStringMapEntry(
"p_merge",
HoldsVariable("p", Output,
``````````
</details>
https://github.com/llvm/llvm-project/pull/84166
More information about the cfe-commits
mailing list