[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
github-actions[bot] wrote: # :penguin: Linux x64 Test Results * 87830 tests passed * 1537 tests skipped * 1 test failed ## Failed Tests (click on a test name to see its output) ### Clang Tools Clang Tools.clang-tidy/infrastructure/update-checks-list.test ``` Exit Code: 1 Command Output (stdout): -- # RUN: at line 1 /usr/bin/python3 /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/test/clang-tidy/infrastructure/../../../clang-tidy/tool/check_update_docs.py -o /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/infrastructure/Output/update-checks-list.test.tmp.clang-tidy-checks-list.rst # executed command: /usr/bin/python3 /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/test/clang-tidy/infrastructure/../../../clang-tidy/tool/check_update_docs.py -o /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/infrastructure/Output/update-checks-list.test.tmp.clang-tidy-checks-list.rst # .---command stderr # | # | 'clang-tools-extra/docs/clang-tidy/checks/list.rst' is out of date. # | Fix it by running 'clang-tools-extra/clang-tidy/add_new_check.py --update-docs'. # | # `- # RUN: at line 2 diff --strip-trailing-cr /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/test/clang-tidy/infrastructure/../../../docs/clang-tidy/checks/list.rst /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/infrastructure/Output/update-checks-list.test.tmp.clang-tidy-checks-list.rst # executed command: diff --strip-trailing-cr /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/test/clang-tidy/infrastructure/../../../docs/clang-tidy/checks/list.rst /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/infrastructure/Output/update-checks-list.test.tmp.clang-tidy-checks-list.rst # .---command stdout # | *** /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/test/clang-tidy/infrastructure/../../../docs/clang-tidy/checks/list.rst # | --- /home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/infrastructure/Output/update-checks-list.test.tmp.clang-tidy-checks-list.rst # | *** # | *** 131,136 # | --- 131,137 # | :doc:`bugprone-non-zero-enum-to-bool-conversion `, # | :doc:`bugprone-nondeterministic-pointer-iteration-order `, # | :doc:`bugprone-not-null-terminated-result `, "Yes" # | +:doc:`bugprone-null-check-after-dereference `, # | :doc:`bugprone-optional-value-conversion `, "Yes" # | :doc:`bugprone-parent-virtual-call `, "Yes" # | :doc:`bugprone-pointer-arithmetic-on-polymorphic-object `, # `- # error: command failed with exit status: 1 -- ``` If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the `infrastructure` label. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
github-actions[bot] wrote: # :window: Windows x64 Test Results * 53860 tests passed * 1186 tests skipped * 1 test failed ## Failed Tests (click on a test name to see its output) ### Clang Tools Clang Tools.clang-tidy/infrastructure/update-checks-list.test ``` Exit Code: 1 Command Output (stdout): -- # RUN: at line 1 C:/Python312/python.exe C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-tidy\infrastructure/../../../clang-tidy/tool/check_update_docs.py -o C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra\test\clang-tidy\infrastructure\Output\update-checks-list.test.tmp.clang-tidy-checks-list.rst # executed command: C:/Python312/python.exe 'C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-tidy\infrastructure/../../../clang-tidy/tool/check_update_docs.py' -o 'C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra\test\clang-tidy\infrastructure\Output\update-checks-list.test.tmp.clang-tidy-checks-list.rst' # .---command stderr # | # | 'clang-tools-extra/docs/clang-tidy/checks/list.rst' is out of date. # | Fix it by running 'clang-tools-extra/clang-tidy/add_new_check.py --update-docs'. # | # `- # RUN: at line 2 diff --strip-trailing-cr C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-tidy\infrastructure/../../../docs/clang-tidy/checks/list.rst C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra\test\clang-tidy\infrastructure\Output\update-checks-list.test.tmp.clang-tidy-checks-list.rst # executed command: diff --strip-trailing-cr 'C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-tidy\infrastructure/../../../docs/clang-tidy/checks/list.rst' 'C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra\test\clang-tidy\infrastructure\Output\update-checks-list.test.tmp.clang-tidy-checks-list.rst' # .---command stdout # | *** C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-tidy\infrastructure/../../../docs/clang-tidy/checks/list.rst # | --- C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra\test\clang-tidy\infrastructure\Output\update-checks-list.test.tmp.clang-tidy-checks-list.rst # | *** # | *** 131,136 # | --- 131,137 # | :doc:`bugprone-non-zero-enum-to-bool-conversion `, # | :doc:`bugprone-nondeterministic-pointer-iteration-order `, # | :doc:`bugprone-not-null-terminated-result `, "Yes" # | +:doc:`bugprone-null-check-after-dereference `, # | :doc:`bugprone-optional-value-conversion `, "Yes" # | :doc:`bugprone-parent-virtual-call `, "Yes" # | :doc:`bugprone-pointer-arithmetic-on-polymorphic-object `, # `- # error: command failed with exit status: 1 -- ``` If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the `infrastructure` label. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
github-actions[bot] wrote:
:warning: C/C++ code linter, clang-tidy found issues in your code. :warning:
You can test this locally with the following command:
```bash
git diff -U0 origin/main...HEAD --
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp |
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py -path build -p1
-quiet
```
View the output from clang-tidy here.
```
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:16:1:
warning: #includes are not sorted properly [llvm-include-order]
note: this fix will not be applied because it overlaps with another fix
16 | #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
| ^
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:18:1:
warning: included header DataflowLattice.h is not used directly
[misc-include-cleaner]
18 | #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
| ^
19 | #include
"clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:22:1:
warning: included header Any.h is not used directly [misc-include-cleaner]
22 | #include "llvm/ADT/Any.h"
| ^
23 | #include "llvm/ADT/STLExtras.h"
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:27:1:
warning: included header vector is not used directly [misc-include-cleaner]
27 | #include
| ^
28 |
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:38:8:
warning: struct 'ExpandedResult' can be moved into an anonymous namespace to
enforce internal linkage [misc-use-internal-linkage]
38 | struct ExpandedResult {
|^
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:64:3:
warning: variable 'Env' of type 'dataflow::Environment' can be declared 'const'
[misc-const-correctness]
64 | dataflow::Environment Env(AnalysisContext, FuncDecl);
| ^
| const
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:83:25:
warning: 'auto Val' can be declared as 'const auto *Val' [llvm-qualified-auto]
83 | if (auto Val =
Diagnoser.WarningLocToVal[Entry.Location];
| ^~~~
| const auto *
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:84:25:
warning: 'auto DerefExpr' can be declared as 'const auto *DerefExpr'
[llvm-qualified-auto]
84 | auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
| ^~~~
| const auto *
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp:97:8:
warning: invalid case style for variable 'containsPointerValue'
[readability-identifier-naming]
97 | auto containsPointerValue =
|^~~~
|ContainsPointerValue
98 | hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
99 | Finder->addMatcher(
100 | decl(anyOf(functionDecl(unless(isExpansionInSystemHeader()),
101 | // FIXME: Remove the filter below when
lambdas are
102 | // well supported by the check.
103 |
unless(hasDeclContext(cxxRecordDecl(isLambda(,
104 | hasBody(containsPointerValue)),
|
| ContainsPointerValue
105 | cxxConstructorDecl(
106 | unless(hasDeclContext(cxxRecordDecl(isLambda(,
107 | hasAnyConstructorInitializer(
108 | withInitializer(containsPointerValue)
|
| ContainsPointerValue
```
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
github-actions[bot] wrote: :warning: RST documentation linter, doc8 found issues in your code. :warning: You can test this locally with the following command: ```bash doc8 -q clang-tools-extra/docs/clang-tidy/checks/bugprone/null-check-after-dereference.rst --config clang-tools-extra/clang-tidy/doc8.ini ``` View the output from doc8 here. ``` clang-tools-extra/docs/clang-tidy/checks/bugprone/null-check-after-dereference.rst:81: D002 Trailing whitespace clang-tools-extra/docs/clang-tidy/checks/bugprone/null-check-after-dereference.rst:119: D001 Line too long ``` Note: documentation lines should be less than 80 characters wide. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
+bool Branch;
+Environment &Env;
+ };
+
+private:
+ CFGMatchSwitch TransferMatchSwitch;
+ ASTMatchSwitch BranchTransferMatchSwitch;
+
+public:
+ explicit NullPointerAnalysisModel(ASTContext &Context);
+
+ static NoopLattice initialElement() { return {}; }
+
+ static ast_matchers::StatementMatcher ptrValueMatcher();
+
+ // Used to initialize the storage locations of function arguments.
+ // Required to merge these values within the environment.
+ void initializeFunctionParameters(const ControlFlowContext &CFCtx,
+Environment &Env);
+
+ void transfer(const CFGElement &E, NoopLattice &State, Environment &Env);
+
+ void transferBranch(bool Branch, const Stmt *E, NoopLattice &State,
+ Environment &Env);
+
+ void join(QualType Type, const Value &Val1, const Environment &Env1,
+const Value &Val2, const Environment &Env2, Value &MergedVal,
+Environment &MergedEnv) override;
+
+ ComparisonResult compare(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ Value *widen(QualType Type, Value &Prev, const Environment &PrevEnv,
+ Value &Current, Environment &CurrentEnv) override;
+};
+
+class NullCheckAfterDereferenceDiagnoser {
+public:
+ struct DiagnoseArgs {
+llvm::DenseMap &ValToDerefLoc;
+llvm::DenseMap &WarningLocToVal;
+const Environment &Env;
+ };
+
+ using ResultType =
+ std::pair, std::vector>;
+
+ // Maps a pointer's Value to a dereference, null-assignment, etc.
+ // This is used later to construct the Note tag.
+ llvm::DenseMap ValToDerefLoc;
+ // Maps Maps a warning's SourceLocation to its relevant Value.
+ llvm::DenseMap WarningLocToVal;
+
Discookie wrote:
Indeed, I'll put them behind getters for now.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
Discookie wrote:
I think having `SatisfiabilityResult` is better, if for none else than for
readability's sake.
Instead of matching against a true/false value (which doesn't obviously compare
against a different true/false value).
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie commented: I think this PR should do with another review - there's been significant refactors since the last one. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,163 @@
+.. title:: clang-tidy - bugprone-null-check-after-dereference
+
+bugprone-null-check-after-dereference
+=
+
+.. note::
+
+ This check uses a flow-sensitive static analysis to produce its
+ results. Therefore, it may be more resource intensive (RAM, CPU) than the
+ average Clang-Tidy check.
+
+Identifies redundant pointer null-checks, by finding cases where the pointer
+cannot be null at the location of the null-check.
+
+Redundant null-checks can signal faulty assumptions about the current value of
+a pointer at different points in the program. Either the null-check is
+redundant, or there could be a null-pointer dereference earlier in the program.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ *ptr = 20; // note: one of the locations where the pointer's value cannot
be null
+ // ...
+ if (ptr) { // bugprone: pointer is checked even though it cannot be null
at this point
+ return *ptr;
+ }
+ return 0;
+ }
+
+Supported pointer operations
+
+
+Pointer null-checks
+---
+
+The check currently supports null-checks on pointers that use
+``operator bool``, such as when being used as the condition
+for an ``if`` statement. It also supports comparisons such as ``!= nullptr``,
and
EugeneZelenko wrote:
Please follow 80 characters limit.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,163 @@ +.. title:: clang-tidy - bugprone-null-check-after-dereference + +bugprone-null-check-after-dereference += + +.. note:: + + This check uses a flow-sensitive static analysis to produce its + results. Therefore, it may be more resource intensive (RAM, CPU) than the + average Clang-tidy check. EugeneZelenko wrote: ```suggestion average Clang-Tidy check. ``` https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -103,6 +103,12 @@ New checks Finds lambda captures that capture the ``this`` pointer and store it as class members without handle the copy and move constructors and the assignments. +- New :doc:`bugprone-null-check-after-dereference + ` check. + + Identifies redundant pointer null-checks, by finding cases where EugeneZelenko wrote: Please use as much of characters as possible to follow 80 characters limit. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,163 @@
+.. title:: clang-tidy - bugprone-null-check-after-dereference
+
+bugprone-null-check-after-dereference
+=
+
+.. note::
+
+ This check uses a flow-sensitive static analysis to produce its
+ results. Therefore, it may be more resource intensive (RAM, CPU) than the
+ average Clang-tidy check.
+
+Identifies redundant pointer null-checks, by finding cases where the pointer
+cannot be null at the location of the null-check.
+
+Redundant null-checks can signal faulty assumptions about the current value of
+a pointer at different points in the program. Either the null-check is
+redundant, or there could be a null-pointer dereference earlier in the program.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ *ptr = 20; // note: one of the locations where the pointer's value cannot
be null
+ // ...
+ if (ptr) { // bugprone: pointer is checked even though it cannot be null
at this point
+ return *ptr;
+ }
+ return 0;
+ }
+
+Supported pointer operations
+
+
+Pointer null-checks
+---
+
+The check currently supports null-checks on pointers that use
+``operator bool``, such as when being used as the condition
+for an ``if`` statement. It also supports comparisons such as ``!= nullptr``,
and
+``== other_ptr``.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ if (ptr) {
+ if (ptr) { // bugprone: pointer is re-checked after its null-ness is
already checked.
+ return *ptr;
+ }
+
+ return ptr ? *ptr : 0; // bugprone: pointer is re-checked after its
null-ness is already checked.
+ }
+ return 0;
+ }
+
+Pointer dereferences
+
+
+Pointer star- and arrow-dereferences are supported.
+
+.. code-block:: c++
+
+ struct S {
+ int val;
+ };
+
+ void f(int *ptr, S *wrapper) {
+ *ptr = 20;
+ wrapper->val = 15;
+ }
+
+Null-pointer and other value assignments
+
+
+The check supports assigning various values to pointers, making them *null*
+or *non-null*. The check also supports passing pointers of a pointer to
+external functions.
+
+.. code-block:: c++
+
+ extern int *external();
+ extern void refresh(int **ptr_ptr);
+
+ int f() {
+ int *ptr_null = nullptr;
+ if (ptr_null) { // bugprone: pointer is checked where it cannot be
non-null.
+ return *ptr_null;
+ }
+
+ int *ptr = external();
+ if (ptr) { // safe: external() could return either nullable or nonnull
pointers.
+ return *ptr;
+ }
+
+ int *ptr2 = external();
+ *ptr2 = 20;
+ refresh(&ptr2);
+ if (ptr2) { // safe: pointer could be changed by refresh().
+ return *ptr2;
+ }
+ return 0;
+ }
+
+Limitations
+~~~
+
+The check only supports C++ due to limitations in the data-flow framework.
+
+The annotations ``_Nullable`` and ``_Nonnull`` are not supported.
+
+.. code-block:: c++
+
+ extern int *_nonnull external_nonnull();
+
+ int annotations() {
+ int *ptr = external_nonnull();
+
+ return ptr ? *ptr : 0; // false-negative: pointer is known to be non-null.
+ }
+
+Function calls taking a pointer value as a reference or a pointer-to-pointer
are
+not supported.
+
+.. code-block:: c++
+
+ extern int *external();
+ extern void refresh_ref(int *&ptr);
+ extern void refresh_ptr(int **ptr);
+
+ int extern_ref() {
+ int *ptr = external();
+ *ptr = 20;
+
+ refresh_ref(ptr);
+ refresh_ptr(&ptr);
+
+ return ptr ? *ptr : 0; // false-positive: pointer could be changed by
refresh_ref().
+ }
+
+Note tags are currently appended to a single location, even if all paths ensure
+a pointer is not null.
+
+.. code-block:: c++
+
+ int branches(int *p, bool b) {
+ if (b) {
+ *p = 42; // true-positive: note-tag appended here
+ } else {
+ *p = 20; // false-positive: note tag not appended here
+ }
+
+ return ptr ? *ptr : 0;
denzor200 wrote:
`return p ? *p : 0;` ?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,786 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+enum class CompareResult {
+ Same,
+ Different,
+ Top,
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+using CR = CompareResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrWithBinding(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ expr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return unaryOperator(hasOperatorName("*"),
hasUnaryOperand(ptrWithBinding()));
+}
+
+auto arrowMatcher() {
+ return memberExpr(allOf(isArrow(), hasObjectExpression(ptrWithBinding(;
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrWithBinding()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher(llvm::StringRef VarName = kVar) {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(VarName);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(
+ callee(functionDecl(hasAnyParameter(anyOf(hasType(pointerType()),
+hasType(referenceType()));
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrWithBinding()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto nullCheckExprMatcher() {
+ return binaryOperator(hasAnyOperatorName("==", "!="),
+hasOperands(ptrWithBinding(), nullptrMatcher(kValue)));
+}
+
+// FIXME: When TK_IgnoreUnlessSpelledInSource is removed from ptrWithBinding(),
+// this matcher should be merged with nullCheckExprMatcher().
+auto equalExprMatcher() {
+ return binaryOperator(hasAnyOperatorName("==", "!="),
+hasOperands(ptrWithBinding(kVar),
+ptrWithBinding(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+S
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
+bool Branch;
+Environment &Env;
+ };
+
+private:
+ CFGMatchSwitch TransferMatchSwitch;
+ ASTMatchSwitch BranchTransferMatchSwitch;
+
+public:
+ explicit NullPointerAnalysisModel(ASTContext &Context);
+
+ static NoopLattice initialElement() { return {}; }
+
+ static ast_matchers::StatementMatcher ptrValueMatcher();
+
+ // Used to initialize the storage locations of function arguments.
+ // Required to merge these values within the environment.
+ void initializeFunctionParameters(const ControlFlowContext &CFCtx,
+Environment &Env);
+
+ void transfer(const CFGElement &E, NoopLattice &State, Environment &Env);
+
+ void transferBranch(bool Branch, const Stmt *E, NoopLattice &State,
+ Environment &Env);
+
+ void join(QualType Type, const Value &Val1, const Environment &Env1,
+const Value &Val2, const Environment &Env2, Value &MergedVal,
+Environment &MergedEnv) override;
+
+ ComparisonResult compare(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ Value *widen(QualType Type, Value &Prev, const Environment &PrevEnv,
+ Value &Current, Environment &CurrentEnv) override;
+};
+
+class NullCheckAfterDereferenceDiagnoser {
+public:
+ struct DiagnoseArgs {
+llvm::DenseMap &ValToDerefLoc;
+llvm::DenseMap &WarningLocToVal;
+const Environment &Env;
+ };
+
+ using ResultType =
+ std::pair, std::vector>;
+
+ // Maps a pointer's Value to a dereference, null-assignment, etc.
+ // This is used later to construct the Note tag.
+ llvm::DenseMap ValToDerefLoc;
+ // Maps Maps a warning's SourceLocation to its relevant Value.
+ llvm::DenseMap WarningLocToVal;
+
martinboehme wrote:
It looks, though, as if these maps are passed to the diagnose-match-switch
through a `DiagnoseArgs` parameter, rather than being accessed directly. So I
think these could (and should) still be private?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
martinboehme wrote:
The transfer function used in `transferBranch()` doesn't currently appear to be
doing anything (see also my comment there) -- so can you just delete this
struct entirely?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
martinboehme wrote:
> I'm not sure if there's a better way to bind to the "root" Expr that I
> haven't found yet, but the current solution has too many dragons. (Maybe this
> is also a case of X/Y problem, where if I assign atoms as IsNull/IsNonnull
> instead of formulas, I wouldn't need to re-assign Values...)
I think this is the underlying issue. When you glean new information about an
existing `Value`, that information should be reflected in the flow condition,
rather than by replacing the existing value.
As you've discovered, replacing the value of a prvalue expression with a
different value does nothing to change the value of a glvalue expression that
the prvalue may have originated from (using an lvalue-to-rvalue cast). And I
don't think you should be trying to change the value of this original glvalue
(I think this is what you mean by the "root" expression?):
* I don't think there's a reliable way of finding this original glvalue.
`TK_IgnoreUnlessSpelledInSource` looks like an attempt to do this that works in
some cases, but I'm sure there are other cases where this doesn't work.
* Indeed, there may not even be any glvalue that the prvalue originated from.
More generally, attempting to do this goes against the grain of value
categories in C++. The whole idea of a prvalue is that it is a "pure value"
with no storage location associated with it. Operations on prvalues are pure
expression evaluation in the sense that this works in functional languages --
they have no way of changing anything that is stored in memory, i.e. they are
free of side-effects.
So a check shouldn't be trying to go out and find the glvalue that a prvalue
might have been cast from, and then changing the value associated with that
glvalue's storage location -- because that would be doing something that isn't
possible in C++. If we want to model the fact that we have learned new
information about a value, that should be done by changing the flow condition,
rather than changing the value itself.
Feel free to ask back if you're unsure how you would do this in the context of
this check.
As an aside, I think making this change would also eliminate the restriction
that the check currently doesn't deal correctly with references to pointers (as
you note in the PR description).
https://github.com/llvm/ll
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/martinboehme requested changes to this pull request. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/martinboehme edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
martinboehme wrote: > gentle ping @martinboehme @gribozavr Sorry, I didn't notice your original comment that this is ready for review again. I've added a few individual comments, but I haven't reviewed the whole PR in depth because I see that there are still quite a number of old comments that haven't been addressed. Please could you address these comments before the next review round? Also: > I'm still yet to refactor handling `Top`, currently it's used in early > returns where possible, but I still need to think of a clear way to make > central functions for it. Can you address this too before the next review round? It's hard to review a PR that the author themself acknowledges isn't in a final state yet. If you're not sure what the best way is to do certain things, happy to help with that of course -- but then we should address those specific topics first before continuing to review the whole PR. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
+ if (!b) {
+*p = 42;
+ }
+
+ if (b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+
+int regular_assignment(int *p, int *q) {
+ *p = 42;
+ q = p;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-5]]:3: note: one of the locations where the
pointer's value cannot be null
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int nullptr_assignment(int *nullptr_param, bool b) {
+ *nullptr_param = 42;
+ int *nullptr_assigned;
+
+ if (b) {
+nullptr_assigned = nullptr;
+ } else {
+nullptr_assigned = nullptr_param;
+ }
+
+ if (nullptr_assigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*nullptr_assigned = 20;
+return *nullptr_assigned;
+ } else {
+return 0;
+ }
+}
+
+extern int *fncall();
+extern void refresh_ref(int *&ptr);
+extern void refresh_ptr(int **ptr);
+
+int fncall_reassignment(int *fncall_reassigned) {
+ *fncall_reassigned = 42;
+
+ fncall_reassigned = fncall();
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ fncall_reassigned = fncall();
+
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer'
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
martinboehme wrote:
I've found the implementation now.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
Discookie wrote: gentle ping @martinboehme @gribozavr https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
Discookie wrote: Should be ready for another round of review. Simplified a lot of the logic, implemented most of the changes requested, added support for `p != nullptr`, and `void f(int **ptr)` invalidating the value of `*ptr`. I've been experimenting with trying to remove `TK_IgnoreUnlessSpelledInSource`, without much luck. I need to be able to set a new `Value` to all of my pointers, and currently the framework doesn't let me set it in a persistent manner. I'm still yet to refactor handling `Top`, currently it's used in early returns where possible, but I still need to think of a clear way to make central functions for it. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -217,34 +231,72 @@ void matchDereferenceExpr(const Stmt *stmt,
Env.assume(Env.arena().makeNot(getVal(kIsNull, *RootValue).formula()));
}
-void matchCastExpr(const CastExpr *cond, const MatchFinder::MatchResult
&Result,
- NullPointerAnalysisModel::TransferArgs &Data) {
- auto [IsNonnullBranch, Env] = Data;
-
+void matchNullCheckExpr(const Expr *NullCheck,
+const MatchFinder::MatchResult &Result,
+Environment &Env) {
const auto *Var = Result.Nodes.getNodeAs(kVar);
assert(Var != nullptr);
- Value *RootValue = getValue(*Var, Env);
-
- Value *NewRootValue = Env.createValue(Var->getType());
+ // (bool)p or (p != nullptr)
+ bool IsNonnullOp = true;
+ if (auto *BinOp = dyn_cast(NullCheck);
EugeneZelenko wrote:
`
```suggestion
if (const auto *BinOp = dyn_cast(NullCheck);
```
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -110,6 +110,12 @@ New checks Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP can be constructed outside itself and the derived class. +- New :doc:`bugprone-null-check-after-dereference + ` check. + + This check identifies redundant pointer null-checks, by finding cases where EugeneZelenko wrote: Please omit `This check`. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
Discookie wrote:
Apparently `TK_IgnoreUnlessSpelledInSource` is still required here. While
getting `Value`s out of the framework is propagated correctly, *setting* values
is not.
When setting a new `Value`, only the outermost `Expr` will have that value set,
which is not guaranteed to be a glvalue even if the underlying value is, and
could be overwritten by the propagation done by `Transfer.cpp`. To set a value
directly, we still need to bind to the root `Expr` in this case.
This traversal has weird behavior with `nullptr` and some other casts - on
`nullptr` we need to match the `NullToPointer` cast, and not the nullptr
directly, and explicit casts also have the chance to not bind correctly to the
root pointer.
I'm not sure if there's a better way to bind to the "root" `Expr` that I
haven't found yet, but the current solution has too many dragons. (Maybe this
is also a case of X/Y problem, where if I assign atoms as `IsNull/IsNonnull`
instead of formulas, I wouldn't need to re-assign `Value`s...)
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
Discookie wrote:
Indeed they are glvalues/prvalues.
The issue I encountered earlier was that there are a couple unintuitive
operators, such as prefix `++` as glvalue, that can appear in unexpected
places, where there should only be a prvalue otherwise. Which leaves the model
to decide which `setValue` to call.
Looks like it's not an issue for now as I haven't found any new crashes.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
martinboehme wrote:
> I was afraid that a more complex argument would run into issues with being
> rvalue vs lvalue, and the differences between the two had me crash the
> framework a couple times before.
If this is causing issues for you and you're not clear on why, let me know --
happy to go over the details of how this works.
Nit: The relevant distinction is actually between:
* prvalue -- a "pure rvalue" that is "just a value" (it has no storage
location / address, i.e. it's not permissible to apply the address-of operator
`&` to it). This is represented in the framework as a `Value` (or some subclass)
* glvalue -- a "generalized lvalue", i.e. something that has a storage
location / address, which you can obtain using the `&` operator. This is
represented in the framework as a `StorageLocation` (or
`RecordStorageLocation`).
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
martinboehme wrote: PS * Tests appear to be failing. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
martinboehme wrote: Some general comments: * Is this ready for the next round of review? I see you've pushed a change, but you remark in other comments that there are still changes left to do. Can you let us know when this is ready for review again? * When updating the PR, can you [use fixups instead of force-pushing](https://llvm.org/docs/GitHub.html#updating-pull-requests)? This makes it easier to see what you've changed. * This PR has a _lot_ of reviewers on it. I'd suggest keeping the ones who have made comments so far and removing the others. (They can always re-add themselves if they want to, but a PR shouldn't generally need this many reviewers.) https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
martinboehme wrote:
That link to the implementation doesn't work for me, and I still can't find the
implementation by searching the diff. (When I asked where it's "defined", I
specifically meant the implementation; the mention of the function in the
header file is a declaration.)
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -116,6 +116,12 @@ New checks Replaces certain conditional statements with equivalent calls to ``std::min`` or ``std::max``. +- New :doc:`bugprone-null-check-after-dereference EugeneZelenko wrote: Please keep alphabetical order (by check name) in this section. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,162 @@ +.. title:: clang-tidy - bugprone-null-check-after-dereference + +bugprone-null-check-after-dereference += + +.. note:: + + This check uses a flow-sensitive static analysis to produce its + results. Therefore, it may be more resource intensive (RAM, CPU) than the + average Clang-tidy check. + +This check identifies redundant pointer null-checks, by finding cases where the EugeneZelenko wrote: Please synchronize with statement in Release Notes. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
Discookie wrote:
Returning a `BoolValue &` sounds useful. I wouldn't be able to differentiate
cases such as `nullptr` vs `unknown`, but maybe I don't even need to, since I'm
gonna initialize the variable later anyways.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ // Early return with the fast compute values.
+ if (SatisfiabilityResult ShallowResult =
+ shallowComputeSatisfiability(Val, Env);
+ ShallowResult != SR::Unknown) {
+return ShallowResult;
+ }
+
+ if (Env.proves(Val->formula()))
+return SR::True;
+
+ if (Env.proves(Env.arena().makeNot(Val->formula(
+return SR::False;
+
+ return SR::Unknown;
+}
+
+inline BoolValue &getVal(llvm::StringRef Name, Value &RootValue) {
+ return *cast(RootValue.getProperty(Name));
+}
+
+// Assign
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
+ if (!b) {
+*p = 42;
+ }
+
+ if (b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+
+int regular_assignment(int *p, int *q) {
+ *p = 42;
+ q = p;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-5]]:3: note: one of the locations where the
pointer's value cannot be null
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int nullptr_assignment(int *nullptr_param, bool b) {
+ *nullptr_param = 42;
+ int *nullptr_assigned;
+
+ if (b) {
+nullptr_assigned = nullptr;
+ } else {
+nullptr_assigned = nullptr_param;
+ }
+
+ if (nullptr_assigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*nullptr_assigned = 20;
+return *nullptr_assigned;
+ } else {
+return 0;
+ }
+}
+
+extern int *fncall();
+extern void refresh_ref(int *&ptr);
+extern void refresh_ptr(int **ptr);
+
+int fncall_reassignment(int *fncall_reassigned) {
+ *fncall_reassigned = 42;
+
+ fncall_reassigned = fncall();
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ fncall_reassigned = fncall();
+
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer'
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
+bool Branch;
+Environment &Env;
+ };
+
+private:
+ CFGMatchSwitch TransferMatchSwitch;
+ ASTMatchSwitch BranchTransferMatchSwitch;
+
+public:
+ explicit NullPointerAnalysisModel(ASTContext &Context);
+
+ static NoopLattice initialElement() { return {}; }
+
+ static ast_matchers::StatementMatcher ptrValueMatcher();
+
+ // Used to initialize the storage locations of function arguments.
+ // Required to merge these values within the environment.
+ void initializeFunctionParameters(const ControlFlowContext &CFCtx,
+Environment &Env);
+
+ void transfer(const CFGElement &E, NoopLattice &State, Environment &Env);
+
+ void transferBranch(bool Branch, const Stmt *E, NoopLattice &State,
+ Environment &Env);
+
+ void join(QualType Type, const Value &Val1, const Environment &Env1,
+const Value &Val2, const Environment &Env2, Value &MergedVal,
+Environment &MergedEnv) override;
+
+ ComparisonResult compare(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ Value *widen(QualType Type, Value &Prev, const Environment &PrevEnv,
+ Value &Current, Environment &CurrentEnv) override;
+};
+
+class NullCheckAfterDereferenceDiagnoser {
+public:
+ struct DiagnoseArgs {
+llvm::DenseMap &ValToDerefLoc;
+llvm::DenseMap &WarningLocToVal;
+const Environment &Env;
+ };
+
+ using ResultType =
+ std::pair, std::vector>;
+
+ // Maps a pointer's Value to a dereference, null-assignment, etc.
+ // This is used later to construct the Note tag.
+ llvm::DenseMap ValToDerefLoc;
+ // Maps Maps a warning's SourceLocation to its relevant Value.
+ llvm::DenseMap WarningLocToVal;
+
Discookie wrote:
These maps are used to make the note-tags within
`NullCheckAfterDereferenceCheck`. Their naming scheme is a bit misleading in
that sense, and note-tags could probably also be created directly by the
Diagnoser.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ // Early return with the fast compute values.
+ if (SatisfiabilityResult ShallowResult =
+ shallowComputeSatisfiability(Val, Env);
+ ShallowResult != SR::Unknown) {
+return ShallowResult;
+ }
+
+ if (Env.proves(Val->formula()))
+return SR::True;
+
+ if (Env.proves(Env.arena().makeNot(Val->formula(
+return SR::False;
+
+ return SR::Unknown;
+}
+
+inline BoolValue &getVal(llvm::StringRef Name, Value &RootValue) {
+ return *cast(RootValue.getProperty(Name));
+}
+
+// Assign
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
Discookie wrote:
Removed a couple of the redundant lines, but I do wanna go over the tests once
again, and rewrite them to be more concise.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
Discookie wrote:
Indeed - it was testing the check's behavior from back when variables' `Value`s
weren't initialized by default.
Merging a variable that only had a `Value` on one of the branches caused weird
behavior - same reason the leftover ``initializeFunctionParameters()`` was
still in the code.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
+ Finder->addMatcher(
+ decl(anyOf(functionDecl(unless(isExpansionInSystemHeader()),
+ // FIXME: Remove the filter below when lambdas
are
+ // well supported by the check.
+
unless(hasDeclContext(cxxRecordDecl(isLambda(,
+ hasBody(hasPointerValue)),
+ cxxConstructorDecl(hasAnyConstructorInitializer(
+ withInitializer(hasPointerValue)
+ .bind(Fu
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
Discookie wrote:
They are, I just wanted to show that this is the place where a false-positive
would come up. Would a ``// no-warning`` or similar be better, or is removing
the line just fine?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
Discookie wrote:
The traversal here was only necessary because I was binding the name to the
underlying variable directly.
Now that arbitrary pointer values are supported, it's not needed, the framework
is indeed good at passing values through other implicit casts.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
Discookie wrote:
It's in the model header file,
[here](https://github.com/llvm/llvm-project/pull/84166/files#diff-bbce48612d4066ae8a050f787028e52d9f7d957a2f8b834a06f69dfefb155b6eR59),
implemented
[here](https://github.com/llvm/llvm-project/pull/84166/files#diff-2c9f636b2ee4dd92e7a2119fcddcc68ddaefe218a1a39f0d1492ac4f94d05f87R417).
In hindsight, it seems like some meaningless code deduplication that isn't
really worth exporting to a separate function though.
https://github.com/llvm/llvm-project
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,162 @@
+.. title:: clang-tidy - bugprone-null-check-after-dereference
+
+bugprone-null-check-after-dereference
+=
+
+.. note::
+
+ This check uses a flow-sensitive static analysis to produce its
+ results. Therefore, it may be more resource intensive (RAM, CPU) than the
+ average clang-tidy check.
+
+This check identifies redundant pointer null-checks, by finding cases where the
+pointer cannot be null at the location of the null-check.
+
+Redundant null-checks can signal faulty assumptions about the current value of
+a pointer at different points in the program. Either the null-check is
+redundant, or there could be a null-pointer dereference earlier in the program.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ *ptr = 20; // note: one of the locations where the pointer's value cannot
be null
+ // ...
+ if (ptr) { // bugprone: pointer is checked even though it cannot be null
at this point
+ return *ptr;
+ }
+ return 0;
+ }
+
+Supported pointer operations
+
+
+Pointer null-checks
+---
+
+The checker currently supports null-checks on pointers that use
+``operator bool``, such as when being used as the condition
+for an `if` statement.
Discookie wrote:
I thought I implemented it, but apparently not, good catch!
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
Discookie wrote:
It indeed shouldn't, I was afraid that a more complex argument would run into
issues with being rvalue vs lvalue, and the differences between the two had me
crash the framework a couple times before. Seems to have no issues anymore, so
changed and renamed `ptrToVar()` to take any pointer value.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
Discookie wrote:
`State` works, yep.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie commented: Thanks for the reviews! Fixed a lot of the comments, and I have a few more things to test for the rest. @gribozavr Thank you for the thorough introduction to your approaches, and the links to related works as well! Templated types are something I didn't take a look at before, will need to experiment with those as well. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie updated
https://github.com/llvm/llvm-project/pull/84166
>From 704d175fde121edaf962614d8c8d626bf8dbf156 Mon Sep 17 00:00:00 2001
From: Viktor
Date: Wed, 6 Mar 2024 14:10:44 +
Subject: [PATCH 1/3] [clang][dataflow] Add null-check after dereference
checker
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt| 1 +
.../NullCheckAfterDereferenceCheck.cpp| 171 +
.../bugprone/NullCheckAfterDereferenceCheck.h | 37 ++
clang-tools-extra/clangd/TidyProvider.cpp | 3 +-
.../bugprone/null-check-after-dereference.rst | 162 +
.../bugprone/null-check-after-dereference.cpp | 330 +
.../Models/NullPointerAnalysisModel.h | 112
.../FlowSensitive/Models/CMakeLists.txt | 1 +
.../Models/NullPointerAnalysisModel.cpp | 625 ++
.../Analysis/FlowSensitive/CMakeLists.txt | 1 +
.../NullPointerAnalysisModelTest.cpp | 332 ++
12 files changed, 1777 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/null-check-after-dereference.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/null-check-after-dereference.cpp
create mode 100644
clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h
create mode 100644
clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp
create mode 100644
clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a8a23b045f80bb..ddd708dd513355 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -48,6 +48,7 @@
#include "NoEscapeCheck.h"
#include "NonZeroEnumToBoolConversionCheck.h"
#include "NotNullTerminatedResultCheck.h"
+#include "NullCheckAfterDereferenceCheck.h"
#include "OptionalValueConversionCheck.h"
#include "ParentVirtualCallCheck.h"
#include "PosixReturnCheck.h"
@@ -180,6 +181,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck("bugprone-posix-return");
CheckFactories.registerCheck(
"bugprone-reserved-identifier");
+CheckFactories.registerCheck(
+"bugprone-null-check-after-dereference");
CheckFactories.registerCheck(
"bugprone-shared-ptr-array-mismatch");
CheckFactories.registerCheck("bugprone-signal-handler");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 1cd6fb207d7625..5dbe761cb810e7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -44,6 +44,7 @@ add_clang_library(clangTidyBugproneModule
NoEscapeCheck.cpp
NonZeroEnumToBoolConversionCheck.cpp
NotNullTerminatedResultCheck.cpp
+ NullCheckAfterDereferenceCheck.cpp
OptionalValueConversionCheck.cpp
ParentVirtualCallCheck.cpp
PosixReturnCheck.cpp
diff --git
a/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
new file mode 100644
index 00..7ef3169cc63863
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDia
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
whisperity wrote: > Is there no way to mark new/experimental checks so that they are off by > default? @ymand At some point in the past, albeit several years ago, I had worked on a check (not with the data-flow framework!) which was originally on track to be introduced as such an *experimental* check. See https://reviews.llvm.org/D76545. The idea was that similarly to `alpha.` checkers in **CSA** we could add an `experimental-`. The biggest counter-argument is that there is no good policy for when something can be put in as experimental, and we don't have a consensus as to how long something can stay experimental, how experimental things are dropped, or promoted into non-experimental checkers, etc. **CSA** has several alpha checkers (some have been there for almost a decade now!), and some of my colleagues are working hard on improving them. As of some 18.0 version, Clang-Tidy by default enables **no** checkers apart from those "inherited" from **CSA**. At least calling a raw `clang-tidy --list-checks` only gives back `clang-analyzer-` ones... @PiotrZSL For interactive users, we have added the new check to the list of forbidden checks that are auto-enabled through clangd. Just like how the optional checker is on that list. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
ymand wrote: > > I think those issues are stale. > > Last time I tested it it were on Clang-tidy 17. I'm doing migration to > Clang-tidy 18 now. I will check this and bugprone-unchecked-optional-access > check on my code-base, on which it were unstable previously. If nothing will > hang/crash, then I will be more than happy. Thanks! Note that one open issue is being fixed in https://github.com/llvm/llvm-project/pull/84138 (which I just realized has not landed yet as a commit). So, if you still see crashes, please let us know if that PR fixes it. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
PiotrZSL wrote: > I think those issues are stale. Last time I tested it it were on Clang-tidy 17. I'm doing migration to Clang-tidy 18 now. I will check this and bugprone-unchecked-optional-access check on my code-base, on which it were unstable previously. If nothing will hang/crash, then I will be more than happy. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
ymand wrote: > My main problem is that dataflow framework is slow and unstable, there are 20 > issues open for an bugprone-unchecked-optional-access check that uses this > framework and 19 issues for a framework alone. It crashes, it hangs and only > cause problems. I think those issues are stale. We've done a huge amount of stability engineering in the past 6 months and are running multiple checks based on this framework ~daily over our entire (> 500 MLoc) codebase with no known crashes. The same goes for speed - it's definitely more expensive than typical, local, clang tidies, but we've made significant improvements in this area as well, and track performance of our nullability check with benchmark tests. More generally, I think that if we push folks away from experimental clang tidy checks because of stability, we'll shut down experimentation for clang tidy. Is there no way to mark new/experimental checks so that they are off by default? https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/PiotrZSL commented: I like idea behind this check, and I think that there should be version of this check not only for raw pointers but also for optionals, smart pointers and iterators. My main problem is that dataflow framework is slow and unstable, there are 20 issues open for an bugprone-unchecked-optional-access check that uses this framework and 19 issues for a framework alone. It crashes, it hangs and only cause problems. Personally I would prefer check like this to be written in simpler way by using same method as bugprone-use-after-move is using. Even if it would find less issue, but at-least wouldn't force half of the projects to disable it due to crashes or long execution time. https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
martinboehme wrote:
```suggestion
auto containsPointerValue =
```
"has" in matchers tends to imply a direct child; "contains" reads more
naturally to me.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
martinboehme wrote:
Naming nit: This isn't really computing satisfiability; it's computing whether
or not the environment proves that a `BoolValue` is necessarily true or false.
I think you could rename this `tryToProve()`, and you could probably do without
the `SatisfiabilityResult` type entirely (which isn't really an accurate name
either) and instead return a `BoolValue &` (since that's what the caller ends
up needing anyway). This would be either a literal true or false (if the
environment can prove the value true or false) or `*Val` if the environme
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
+ if (!b) {
+*p = 42;
+ }
+
+ if (b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+
+int regular_assignment(int *p, int *q) {
+ *p = 42;
+ q = p;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-5]]:3: note: one of the locations where the
pointer's value cannot be null
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int nullptr_assignment(int *nullptr_param, bool b) {
+ *nullptr_param = 42;
+ int *nullptr_assigned;
+
+ if (b) {
+nullptr_assigned = nullptr;
+ } else {
+nullptr_assigned = nullptr_param;
+ }
+
+ if (nullptr_assigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*nullptr_assigned = 20;
+return *nullptr_assigned;
+ } else {
+return 0;
+ }
+}
+
+extern int *fncall();
+extern void refresh_ref(int *&ptr);
+extern void refresh_ptr(int **ptr);
+
+int fncall_reassignment(int *fncall_reassigned) {
+ *fncall_reassigned = 42;
+
+ fncall_reassigned = fncall();
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ fncall_reassigned = fncall();
+
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*fncall_reassigned = 42;
+ }
+
+ refresh_ptr(&fncall_reassigned);
+ *fncall_reassigned = 42;
+
+ if (fncall_reassigned) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer'
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
+bool Branch;
+Environment &Env;
+ };
+
+private:
+ CFGMatchSwitch TransferMatchSwitch;
+ ASTMatchSwitch BranchTransferMatchSwitch;
+
+public:
+ explicit NullPointerAnalysisModel(ASTContext &Context);
+
+ static NoopLattice initialElement() { return {}; }
+
+ static ast_matchers::StatementMatcher ptrValueMatcher();
+
+ // Used to initialize the storage locations of function arguments.
+ // Required to merge these values within the environment.
+ void initializeFunctionParameters(const ControlFlowContext &CFCtx,
+Environment &Env);
+
+ void transfer(const CFGElement &E, NoopLattice &State, Environment &Env);
+
+ void transferBranch(bool Branch, const Stmt *E, NoopLattice &State,
+ Environment &Env);
+
+ void join(QualType Type, const Value &Val1, const Environment &Env1,
+const Value &Val2, const Environment &Env2, Value &MergedVal,
+Environment &MergedEnv) override;
+
+ ComparisonResult compare(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ Value *widen(QualType Type, Value &Prev, const Environment &PrevEnv,
+ Value &Current, Environment &CurrentEnv) override;
+};
+
+class NullCheckAfterDereferenceDiagnoser {
+public:
+ struct DiagnoseArgs {
+llvm::DenseMap &ValToDerefLoc;
+llvm::DenseMap &WarningLocToVal;
+const Environment &Env;
+ };
+
+ using ResultType =
+ std::pair, std::vector>;
+
+ // Maps a pointer's Value to a dereference, null-assignment, etc.
+ // This is used later to construct the Note tag.
+ llvm::DenseMap ValToDerefLoc;
+ // Maps Maps a warning's SourceLocation to its relevant Value.
+ llvm::DenseMap WarningLocToVal;
+
martinboehme wrote:
Do these need to be public? They look like internal implementation details of
the class.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
martinboehme wrote:
This test seems borderline redundant -- it's really testing the framework
itself rather than your check.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
martinboehme wrote:
I think this is redundant. This style of test will fail if there are any
unexpected diagnostics.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int warning_member(S *q) {
+ q->a = 42;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+q->a += 20;
+return q->a;
+ } else {
+return 0;
+ }
+}
+
+int negative_warning(int *p) {
+ *p = 42;
+
+ if (!p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int no_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (p) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int else_branch_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ } else {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-7]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_warning(int *p, bool b) {
+ if (b) {
+*p = 42;
+ }
+
+ if (!b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+int two_branches_reversed(int *p, bool b) {
+ if (!b) {
+*p = 42;
+ }
+
+ if (b) {
+*p = 20;
+ }
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-9]]:5: note: one of the locations where the
pointer's value cannot be null
+return 0;
+ } else {
+*p += 20;
+return *p;
+ }
+}
+
+
+int regular_assignment(int *p, int *q) {
+ *p = 42;
+ q = p;
+
+ if (q) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point
+// CHECK-MESSAGES: :[[@LINE-5]]:3: note: one of the locations where the
pointer's value cannot be null
+*p += 20;
+return *p;
+ } else {
+return 0;
+ }
+}
+
+int nullptr_assignment(int *nullptr_param, bool b) {
+ *nullptr_param = 42;
+ int *nullptr_assigned;
+
+ if (b) {
+nullptr_assigned = nullptr;
+ } else {
+nullptr_assigned = nullptr_param;
+ }
+
+ if (nullptr_assigned) {
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: pointer value is checked
even though it cannot be null at this point
martinboehme wrote:
Redundant
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s bugprone-null-check-after-dereference %t
+
+struct S {
+ int a;
+};
+
+int warning_deref(int *p) {
+ *p = 42;
+
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: pointer value is checked even
though it cannot be null at this point [bugprone-null-check-after-dereference]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: one of the locations where the
pointer's value cannot be null
+ // FIXME: If there's a direct path, make the error message more precise, ie.
remove `one of the locations`
+*p += 20;
+return *p;
martinboehme wrote:
Nit: This line seems to be redundant -- we already have a `*p` in the same
block. Also, why does this function need to return anything -- just make it
`void`?
(Generally, try to make tests contain only the minimal amount of code you need
to test the behavior the test targets. Also applies similarly to other tests
below.)
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
martinboehme wrote:
Nit: This isn't just the "lattice state" -- it's the entire state, comprising
the `Environment` and the lattice. Maybe simply `State`?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
martinboehme wrote:
Where is `NullPointerAnalysisModel::ptrValueMatcher()` defined? (Can't seem to
find it -- I'm obviously not looking hard enough.)
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
martinboehme wrote:
Why does this need to be public? It doesn't seem to be used outside
`NullPointerAnalysisModel`?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
martinboehme wrote:
Consider using `Expected::moveInto()`.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,162 @@
+.. title:: clang-tidy - bugprone-null-check-after-dereference
+
+bugprone-null-check-after-dereference
+=
+
+.. note::
+
+ This check uses a flow-sensitive static analysis to produce its
+ results. Therefore, it may be more resource intensive (RAM, CPU) than the
+ average clang-tidy check.
+
+This check identifies redundant pointer null-checks, by finding cases where the
+pointer cannot be null at the location of the null-check.
+
+Redundant null-checks can signal faulty assumptions about the current value of
+a pointer at different points in the program. Either the null-check is
+redundant, or there could be a null-pointer dereference earlier in the program.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ *ptr = 20; // note: one of the locations where the pointer's value cannot
be null
+ // ...
+ if (ptr) { // bugprone: pointer is checked even though it cannot be null
at this point
+ return *ptr;
+ }
+ return 0;
+ }
+
+Supported pointer operations
+
+
+Pointer null-checks
+---
+
+The checker currently supports null-checks on pointers that use
+``operator bool``, such as when being used as the condition
+for an `if` statement.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ if (ptr) {
+ if (ptr) { // bugprone: pointer is re-checked after its null-ness is
already checked.
+ return *ptr;
+ }
+
+ return ptr ? *ptr : 0; // bugprone: pointer is re-checked after its
null-ness is already checked.
+ }
+ return 0;
+ }
+
+Pointer dereferences
+
+
+Pointer star- and arrow-dereferences are supported.
+
+.. code-block:: c++
+
+ struct S {
+ int val;
+ };
+
+ void f(int *ptr, S *wrapper) {
+ *ptr = 20;
+ wrapper->val = 15;
+ }
+
+Null-pointer and other value assignments
+
+
+The checker supports assigning various values to pointers, making them *null*
+or *non-null*. The checker also supports passing pointers of a pointer to
+external functions.
+
+.. code-block:: c++
+
+ extern int *external();
+ extern void refresh(int **ptr_ptr);
+
+ int f() {
+ int *ptr_null = nullptr;
+ if (ptr_null) { // bugprone: pointer is checked where it cannot be
non-null.
+ return *ptr_null;
+ }
+
+ int *ptr = external();
+ if (ptr) { // safe: external() could return either nullable or nonnull
pointers.
+ return *ptr;
+ }
+
+ int *ptr2 = external();
+ *ptr2 = 20;
+ refresh(&ptr2);
+ if (ptr2) { // safe: pointer could be changed by refresh().
+ return *ptr2;
+ }
+ return 0;
+ }
+
+Limitations
+~~~
+
+The check only supports C++ due to limitations in the data-flow framework.
+
+The annotations ``_nullable`` and ``_nonnull`` are not supported.
martinboehme wrote:
These annotations should be upper-case (`_Nullable` and `_Nonnull`). The
lower-case spellings are not supported (they produce a compiler error).
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+ SourceLocation WarningLoc;
+ std::optional DerefLoc;
+};
+
+using ExpandedResultType =
+std::pair, std::vector>;
+
+static std::optional
+analyzeFunction(const FunctionDecl &FuncDecl) {
+ using dataflow::ControlFlowContext;
+ using dataflow::DataflowAnalysisState;
+ using llvm::Expected;
+
+ ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+ if (FuncDecl.getBody() == nullptr) {
+return std::nullopt;
+ }
+
+ Expected Context =
+ ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+ if (!Context)
+return std::nullopt;
+
+ dataflow::DataflowAnalysisContext AnalysisContext(
+ std::make_unique());
+ dataflow::Environment Env(AnalysisContext, FuncDecl);
+ NullPointerAnalysisModel Analysis(ASTCtx);
+ NullCheckAfterDereferenceDiagnoser Diagnoser;
+ NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+
+ using LatticeState =
DataflowAnalysisState;
+ using DetailMaybeLatticeStates = std::vector>;
+
+ 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 BlockToOutputState =
+ dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
+
+ if (llvm::Error E = BlockToOutputState.takeError()) {
+llvm::dbgs() << "Dataflow analysis failed: " <<
llvm::toString(std::move(E))
+ << ".\n";
+return std::nullopt;
+ }
+
+ ExpandedResultType ExpandedDiagnostics;
+
+ llvm::transform(Diagnostics.first,
+ std::back_inserter(ExpandedDiagnostics.first),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ llvm::transform(Diagnostics.second,
+ std::back_inserter(ExpandedDiagnostics.second),
+ [&](SourceLocation WarningLoc) -> ExpandedResult {
+if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+ return {WarningLoc, DerefExpr->getBeginLoc()};
+}
+
+return {WarningLoc, std::nullopt};
+ });
+
+ return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+ using namespace ast_matchers;
+
+ auto hasPointerValue =
+ hasDescendant(NullPointerAnalysisModel::ptrValueMatcher());
+ Finder->addMatcher(
+ decl(anyOf(functionDecl(unless(isExpansionInSystemHeader()),
+ // FIXME: Remove the filter below when lambdas
are
+ // well supported by the check.
+
unless(hasDeclContext(cxxRecordDecl(isLambda(,
+ hasBody(hasPointerValue)),
+ cxxConstructorDecl(hasAnyConstructorInitializer(
+ withInitializer(hasPointerValue)
+ .bind(Fu
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,162 @@
+.. title:: clang-tidy - bugprone-null-check-after-dereference
+
+bugprone-null-check-after-dereference
+=
+
+.. note::
+
+ This check uses a flow-sensitive static analysis to produce its
+ results. Therefore, it may be more resource intensive (RAM, CPU) than the
+ average clang-tidy check.
+
+This check identifies redundant pointer null-checks, by finding cases where the
+pointer cannot be null at the location of the null-check.
+
+Redundant null-checks can signal faulty assumptions about the current value of
+a pointer at different points in the program. Either the null-check is
+redundant, or there could be a null-pointer dereference earlier in the program.
+
+.. code-block:: c++
+
+ int f(int *ptr) {
+ *ptr = 20; // note: one of the locations where the pointer's value cannot
be null
+ // ...
+ if (ptr) { // bugprone: pointer is checked even though it cannot be null
at this point
+ return *ptr;
+ }
+ return 0;
+ }
+
+Supported pointer operations
+
+
+Pointer null-checks
+---
+
+The checker currently supports null-checks on pointers that use
+``operator bool``, such as when being used as the condition
+for an `if` statement.
martinboehme wrote:
Is `p != nullptr` supported too? This is a common construct (and is mandated
over `operator bool` by some style guides)l
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ // Early return with the fast compute values.
+ if (SatisfiabilityResult ShallowResult =
+ shallowComputeSatisfiability(Val, Env);
+ ShallowResult != SR::Unknown) {
+return ShallowResult;
+ }
+
+ if (Env.proves(Val->formula()))
+return SR::True;
+
+ if (Env.proves(Env.arena().makeNot(Val->formula(
+return SR::False;
+
+ return SR::Unknown;
+}
+
+inline BoolValue &getVal(llvm::StringRef Name, Value &RootValue) {
+ return *cast(RootValue.getProperty(Name));
+}
+
+// Assign
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/gribozavr edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/gribozavr edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/gribozavr edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/gribozavr edited https://github.com/llvm/llvm-project/pull/84166 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ // Early return with the fast compute values.
+ if (SatisfiabilityResult ShallowResult =
+ shallowComputeSatisfiability(Val, Env);
+ ShallowResult != SR::Unknown) {
+return ShallowResult;
+ }
+
+ if (Env.proves(Val->formula()))
+return SR::True;
+
+ if (Env.proves(Env.arena().makeNot(Val->formula(
+return SR::False;
+
+ return SR::Unknown;
+}
+
+inline BoolValue &getVal(llvm::StringRef Name, Value &RootValue) {
+ return *cast(RootValue.getProperty(Name));
+}
+
+// Assign
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
gribozavr wrote:
Why limit the cast argument to a var? It shouldn't matter if the argument is a
complex expression. As long as the framework models the value somehow (for
example, one of the implicit casts that the core framework already models, like
the lvalue to rvalue cast), the transfer function should not need to worry
about how that ended up happening.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
+}
+
+auto assignMatcher() {
+ return binaryOperation(isAssignmentOperator(), hasLHS(ptrToVar()),
+ hasRHS(expr().bind(kValue)));
+}
+
+auto anyPointerMatcher() { return expr(hasType(isAnyPointer())).bind(kVar); }
+
+// Only computes simple comparisons against the literals True and False in the
+// environment. Faster, but produces many Unknown values.
+SatisfiabilityResult shallowComputeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ if (!Val)
+return SR::Nullptr;
+
+ if (isa(Val))
+return SR::Top;
+
+ if (Val == &Env.getBoolLiteralValue(true))
+return SR::True;
+
+ if (Val == &Env.getBoolLiteralValue(false))
+return SR::False;
+
+ return SR::Unknown;
+}
+
+// Computes satisfiability by using the flow condition. Slower, but more
+// precise.
+SatisfiabilityResult computeSatisfiability(BoolValue *Val,
+ const Environment &Env) {
+ // Early return with the fast compute values.
+ if (SatisfiabilityResult ShallowResult =
+ shallowComputeSatisfiability(Val, Env);
+ ShallowResult != SR::Unknown) {
+return ShallowResult;
+ }
+
+ if (Env.proves(Val->formula()))
+return SR::True;
+
+ if (Env.proves(Env.arena().makeNot(Val->formula(
+return SR::False;
+
+ return SR::Unknown;
+}
+
+inline BoolValue &getVal(llvm::StringRef Name, Value &RootValue) {
+ return *cast(RootValue.getProperty(Name));
+}
+
+// Assign
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
+ declRefExpr(hasType(isAnyPointer())).bind(VarName));
+}
+
+auto derefMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ptrToVar(;
+}
+
+auto arrowMatcher() {
+ return traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ memberExpr(allOf(isArrow(), hasObjectExpression(ptrToVar();
+}
+
+auto castExprMatcher() {
+ return castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ptrToVar()))
+ .bind(kCond);
+}
+
+auto nullptrMatcher() {
+ return castExpr(hasCastKind(CK_NullToPointer)).bind(kVar);
+}
+
+auto addressofMatcher() {
+ return unaryOperator(hasOperatorName("&")).bind(kVar);
+}
+
+auto functionCallMatcher() {
+ return callExpr(hasDeclaration(functionDecl(returns(isAnyPointer()
+ .bind(kVar);
gribozavr wrote:
Please see my other comment about following the `transferValue_Pointer` pattern
from our nullability checker - if you do that you wouldn't need to define
multiple matchers to cover all C++ syntax that could produce a pointer. Instead
it would be only one matcher that covers all pointer-valued expressions, which
should be simpler.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,112 @@
+//===-- NullPointerAnalysisModel.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_NULLPOINTERANALYSISMODEL_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+class NullPointerAnalysisModel
+: public DataflowAnalysis {
+public:
+ /// A transparent wrapper around the function arguments of transferBranch().
+ /// Does not outlive the call to transferBranch().
+ struct TransferArgs {
+bool Branch;
+Environment &Env;
+ };
+
+private:
+ CFGMatchSwitch TransferMatchSwitch;
+ ASTMatchSwitch BranchTransferMatchSwitch;
+
+public:
+ explicit NullPointerAnalysisModel(ASTContext &Context);
+
+ static NoopLattice initialElement() { return {}; }
+
+ static ast_matchers::StatementMatcher ptrValueMatcher();
+
+ // Used to initialize the storage locations of function arguments.
+ // Required to merge these values within the environment.
+ void initializeFunctionParameters(const ControlFlowContext &CFCtx,
+Environment &Env);
+
+ void transfer(const CFGElement &E, NoopLattice &State, Environment &Env);
+
+ void transferBranch(bool Branch, const Stmt *E, NoopLattice &State,
+ Environment &Env);
+
+ void join(QualType Type, const Value &Val1, const Environment &Env1,
+const Value &Val2, const Environment &Env2, Value &MergedVal,
+Environment &MergedEnv) override;
+
+ ComparisonResult compare(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ Value *widen(QualType Type, Value &Prev, const Environment &PrevEnv,
+ Value &Current, Environment &CurrentEnv) override;
+};
+
+class NullCheckAfterDereferenceDiagnoser {
+public:
+ struct DiagnoseArgs {
+llvm::DenseMap &ValToDerefLoc;
+llvm::DenseMap &WarningLocToVal;
+const Environment &Env;
+ };
+
+ using ResultType =
+ std::pair, std::vector>;
gribozavr wrote:
Could you add a doc comment that explains what these vectors hold?
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
@@ -0,0 +1,625 @@
+//===-- NullPointerAnalysisModel.cpp *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a generic null-pointer analysis model, used for finding
+// pointer null-checks after the pointer has already been dereferenced.
+//
+// Only a limited set of operations are currently recognized. Notably, pointer
+// arithmetic, null-pointer assignments and _nullable/_nonnull attributes are
+// missing as of yet.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+namespace clang::dataflow {
+
+namespace {
+using namespace ast_matchers;
+
+constexpr char kCond[] = "condition";
+constexpr char kVar[] = "var";
+constexpr char kValue[] = "value";
+constexpr char kIsNonnull[] = "is-nonnull";
+constexpr char kIsNull[] = "is-null";
+
+enum class SatisfiabilityResult {
+ // Returned when the value was not initialized yet.
+ Nullptr,
+ // Special value that signals that the boolean value can be anything.
+ // It signals that the underlying formulas are too complex to be calculated
+ // efficiently.
+ Top,
+ // Equivalent to the literal True in the current environment.
+ True,
+ // Equivalent to the literal False in the current environment.
+ False,
+ // Both True and False values could be produced with an appropriate set of
+ // conditions.
+ Unknown
+};
+
+using SR = SatisfiabilityResult;
+
+// FIXME: These AST matchers should also be exported via the
+// NullPointerAnalysisModel class, for tests
+auto ptrToVar(llvm::StringRef VarName = kVar) {
+ return traverse(TK_IgnoreUnlessSpelledInSource,
gribozavr wrote:
Could you check if `TK_IgnoreUnlessSpelledInSource` is necessary here?
Generally it shouldn't be. The dataflow framework applies the transfer function
to each CFG element. So it will visit both the implicit wrapping AST nodes and
the nested explicitly spelled node.
Furthermore, applying the transfer function twice to effectively the same
construct could lead to bad effects and incorrect state updates (if the author
of the analysis does not carefully consider that this is a possibility).
This is why we think that the best practice for writing AST matchers for the
transfer function is to only match the immediate node being visited by the
dataflow framework right now, don't traverse into the expression tree.
If there is an issue related to implicit AST nodes not propagating the state,
we should look at it more carefully, and probably either enhance the framework
core to do that, or add transfer functions for the specific AST nodes to
propagate the necessary state.
PTAL at `VisitImplicitCastExpr` in
llvm-project/clang/lib/Analysis/FlowSensitive/Transfer.cpp - the core framework
tries to forward all of the already-modeled values through implicit casts that
can be modeled generically.
https://github.com/llvm/llvm-project/pull/84166
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang-tidy][dataflow] Add `bugprone-null-check-after-dereference` check (PR #84166)
https://github.com/Discookie updated
https://github.com/llvm/llvm-project/pull/84166
>From 704d175fde121edaf962614d8c8d626bf8dbf156 Mon Sep 17 00:00:00 2001
From: Viktor
Date: Wed, 6 Mar 2024 14:10:44 +
Subject: [PATCH] [clang][dataflow] Add null-check after dereference checker
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt| 1 +
.../NullCheckAfterDereferenceCheck.cpp| 171 +
.../bugprone/NullCheckAfterDereferenceCheck.h | 37 ++
clang-tools-extra/clangd/TidyProvider.cpp | 3 +-
.../bugprone/null-check-after-dereference.rst | 162 +
.../bugprone/null-check-after-dereference.cpp | 330 +
.../Models/NullPointerAnalysisModel.h | 112
.../FlowSensitive/Models/CMakeLists.txt | 1 +
.../Models/NullPointerAnalysisModel.cpp | 625 ++
.../Analysis/FlowSensitive/CMakeLists.txt | 1 +
.../NullPointerAnalysisModelTest.cpp | 332 ++
12 files changed, 1777 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/null-check-after-dereference.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/null-check-after-dereference.cpp
create mode 100644
clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h
create mode 100644
clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp
create mode 100644
clang/unittests/Analysis/FlowSensitive/NullPointerAnalysisModelTest.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a8a23b045f80bb..ddd708dd513355 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -48,6 +48,7 @@
#include "NoEscapeCheck.h"
#include "NonZeroEnumToBoolConversionCheck.h"
#include "NotNullTerminatedResultCheck.h"
+#include "NullCheckAfterDereferenceCheck.h"
#include "OptionalValueConversionCheck.h"
#include "ParentVirtualCallCheck.h"
#include "PosixReturnCheck.h"
@@ -180,6 +181,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck("bugprone-posix-return");
CheckFactories.registerCheck(
"bugprone-reserved-identifier");
+CheckFactories.registerCheck(
+"bugprone-null-check-after-dereference");
CheckFactories.registerCheck(
"bugprone-shared-ptr-array-mismatch");
CheckFactories.registerCheck("bugprone-signal-handler");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 1cd6fb207d7625..5dbe761cb810e7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -44,6 +44,7 @@ add_clang_library(clangTidyBugproneModule
NoEscapeCheck.cpp
NonZeroEnumToBoolConversionCheck.cpp
NotNullTerminatedResultCheck.cpp
+ NullCheckAfterDereferenceCheck.cpp
OptionalValueConversionCheck.cpp
ParentVirtualCallCheck.cpp
PosixReturnCheck.cpp
diff --git
a/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
new file mode 100644
index 00..7ef3169cc63863
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp
@@ -0,0 +1,171 @@
+//===--- NullCheckAfterDereferenceCheck.cpp -
clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NullCheckAfterDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include
+#include
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnose
