[clang-tidy][dataflow] Add bugprone-null-check-after-dereference check

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

@@ -49,6 +49,7 @@
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NotNullTerminatedResultCheck.h"
+#include "NullCheckAfterDereferenceCheck.h"
 #include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PointerArithmeticOnPolymorphicObjectCheck.h"
@@ -188,6 +189,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
     CheckFactories.registerCheck<ReservedIdentifierCheck>(
         "bugprone-reserved-identifier");
+    CheckFactories.registerCheck<NullCheckAfterDereferenceCheck>(
+        "bugprone-null-check-after-dereference");
     CheckFactories.registerCheck<SharedPtrArrayMismatchCheck>(
         "bugprone-shared-ptr-array-mismatch");
     CheckFactories.registerCheck<SignalHandlerCheck>("bugprone-signal-handler");

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

@@ -46,6 +46,7 @@ add_clang_library(clangTidyBugproneModule
   NoEscapeCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
+  NullCheckAfterDereferenceCheck.cpp
   OptionalValueConversionCheck.cpp
   ParentVirtualCallCheck.cpp
   PointerArithmeticOnPolymorphicObjectCheck.cpp

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

@@ -0,0 +1,154 @@
+//===--- 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/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 <clang/Analysis/FlowSensitive/AdornedCFG.h>
+#include <memory>
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+using ast_matchers::MatchFinder;
+using dataflow::NullCheckAfterDereferenceDiagnoser;
+using dataflow::NullPointerAnalysisModel;
+using Diagnoser = NullCheckAfterDereferenceDiagnoser;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+  Diagnoser::DiagnosticEntry Entry;
+  std::optional<SourceLocation> DerefLoc;
+};
+
+using ExpandedResultType = llvm::SmallVector<ExpandedResult>;
+
+static std::optional<ExpandedResultType>
+analyzeFunction(const FunctionDecl &FuncDecl) {
+  using dataflow::AdornedCFG;
+  using dataflow::DataflowAnalysisState;
+  using llvm::Expected;
+
+  ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+  if (FuncDecl.getBody() == nullptr) {
+    return std::nullopt;
+  }
+
+  Expected<AdornedCFG> Context =
+      AdornedCFG::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
+  if (!Context)
+    return std::nullopt;
+
+  dataflow::DataflowAnalysisContext AnalysisContext(
+      std::make_unique<dataflow::WatchedLiteralsSolver>());
+  dataflow::Environment Env(AnalysisContext, FuncDecl);
+  NullPointerAnalysisModel Analysis(ASTCtx);
+  Diagnoser Diagnoser;
+
+  Expected<Diagnoser::ResultType> Diagnostics =
+      dataflow::diagnoseFunction<NullPointerAnalysisModel, Diagnoser::DiagnosticEntry>(
+      FuncDecl, ASTCtx, Diagnoser);
+
+
+  if (llvm::Error E = Diagnostics.takeError()) {
+    llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
+                 << ".\n";
+    return std::nullopt;
+  }
+
+  ExpandedResultType ExpandedDiagnostics;
+
+  llvm::transform(*Diagnostics,
+                  std::back_inserter(ExpandedDiagnostics),
+                  [&](Diagnoser::DiagnosticEntry Entry) -> ExpandedResult {
+                    if (auto Val = Diagnoser.WarningLocToVal[Entry.Location];
+                        auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
+                      return {Entry, DerefExpr->getBeginLoc()};
+                    }
+
+                    return {Entry, std::nullopt};
+                  });
+
+  return ExpandedDiagnostics;
+}
+
+void NullCheckAfterDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+  using namespace ast_matchers;
+
+  auto containsPointerValue =
+      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(containsPointerValue)),
+                 cxxConstructorDecl(
+                     unless(hasDeclContext(cxxRecordDecl(isLambda()))),
+                     hasAnyConstructorInitializer(
+                         withInitializer(containsPointerValue)))))
+          .bind(FuncID),
+      this);
+}
+
+void NullCheckAfterDereferenceCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (Result.SourceManager->getDiagnostics().hasUncompilableErrorOccurred())
+    return;
+
+  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(FuncID);
+  assert(FuncDecl && "invalid FuncDecl matcher");
+  if (FuncDecl->isTemplated())
+    return;
+
+  if (const auto Diagnostics = analyzeFunction(*FuncDecl)) {
+    for (const auto [Entry, DerefLoc] : *Diagnostics) {
+      const auto [WarningLoc, Type] = Entry;
+
+      switch (Type) {
+      case Diagnoser::DiagnosticType::CheckAfterDeref:
+        diag(WarningLoc, "pointer value is checked even though "
+                         "it cannot be null at this point");
+
+        if (DerefLoc) {
+          diag(*DerefLoc,
+               "one of the locations where the pointer's value cannot be null",
+               DiagnosticIDs::Note);
+        }
+        break;
+      case Diagnoser::DiagnosticType::CheckWhenNull:
+        diag(WarningLoc,
+             "pointer value is checked but it can only be null at this point");
+
+        if (DerefLoc) {
+          diag(*DerefLoc,
+               "one of the locations where the pointer's value can only be null",
+               DiagnosticIDs::Note);
+        }
+        break;
+      }
+    }
+  }
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,37 @@
+//===--- NullCheckAfterDereferenceCheck.h - clang-tidy ----------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds checks for pointer nullability after a pointer has already been
+/// dereferenced, using the data-flow framework.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/null-check-after-dereference.html
+class NullCheckAfterDereferenceCheck : public ClangTidyCheck {
+public:
+  NullCheckAfterDereferenceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  // The data-flow framework does not support C because of AST differences.
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NULLCHECKAFTERDEREFERENCECHECK_H

clang-tools-extra/clangd/TidyProvider.cpp

@@ -219,9 +219,10 @@ TidyProvider disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks) {
       "-bugprone-use-after-move",
       // Alias for bugprone-use-after-move.
       "-hicpp-invalid-access-moved",
-      // Check uses dataflow analysis, which might hang/crash unexpectedly on
+      // Checks use dataflow analysis, which might hang/crash unexpectedly on
       // incomplete code.
-      "-bugprone-unchecked-optional-access");
+      "-bugprone-unchecked-optional-access",
+      "-bugprone-null-check-after-dereference");
 
   size_t Size = BadChecks.size();
   for (const std::string &Str : ExtraBadChecks) {

clang-tools-extra/docs/ReleaseNotes.rst

@@ -101,6 +101,77 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-crtp-constructor-accessibility
+  <clang-tidy/checks/bugprone/crtp-constructor-accessibility>` check.
+
+  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
+  <clang-tidy/checks/bugprone/null-check-after-dereference>` check.
+
+  This check identifies redundant pointer null-checks, by finding cases where
+  the pointer cannot be null at the location of the null-check.
+
+- New :doc:`bugprone-return-const-ref-from-parameter
+  <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check.
+
+  Detects return statements that return a constant reference parameter as constant
+  reference. This may cause use-after-free errors if the caller uses xvalues as
+  arguments.
+
+- New :doc:`bugprone-suspicious-stringview-data-usage
+  <clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check.
+
+  Identifies suspicious usages of ``std::string_view::data()`` that could lead
+  to reading out-of-bounds data due to inadequate or incorrect string null
+  termination.
+
+- New :doc:`misc-use-internal-linkage
+  <clang-tidy/checks/misc/use-internal-linkage>` check.
+
+  Detects variables and functions that can be marked as static or moved into
+  an anonymous namespace to enforce internal linkage.
+
+- New :doc:`modernize-min-max-use-initializer-list
+  <clang-tidy/checks/modernize/min-max-use-initializer-list>` check.
+
+  Replaces nested ``std::min`` and ``std::max`` calls with an initializer list
+  where applicable.
+
+- New :doc:`modernize-use-designated-initializers
+  <clang-tidy/checks/modernize/use-designated-initializers>` check.
+
+  Finds initializer lists for aggregate types that could be
+  written as designated initializers instead.
+
+- New :doc:`modernize-use-std-format
+  <clang-tidy/checks/modernize/use-std-format>` check.
+
+  Converts calls to ``absl::StrFormat``, or other functions via
+  configuration options, to C++20's ``std::format``, or another function
+  via a configuration option, modifying the format string appropriately and
+  removing now-unnecessary calls to ``std::string::c_str()`` and
+  ``std::string::data()``.
+
+- New :doc:`readability-enum-initial-value
+  <clang-tidy/checks/readability/enum-initial-value>` check.
+
+  Enforces consistent style for enumerators' initialization, covering three
+  styles: none, first only, or all initialized explicitly.
+
+- New :doc:`readability-math-missing-parentheses
+  <clang-tidy/checks/readability/math-missing-parentheses>` check.
+
+  Check for missing parentheses in mathematical expressions that involve
+  operators of different priorities.
+
+- New :doc:`readability-use-std-min-max
+  <clang-tidy/checks/readability/use-std-min-max>` check.
+
+  Replaces certain conditional statements with equivalent calls to
+  ``std::min`` or ``std::max``.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^