[clang][dataflow] Add bugprone-dataflow-dead-code check

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

@@ -23,6 +23,7 @@
 #include "CopyConstructorInitCheck.h"
 #include "CrtpConstructorAccessibilityCheck.h"
 #include "DanglingHandleCheck.h"
+#include "DataflowDeadCodeCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "EasilySwappableParametersCheck.h"
 #include "EmptyCatchCheck.h"
@@ -131,6 +132,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-copy-constructor-init");
     CheckFactories.registerCheck<DanglingHandleCheck>(
         "bugprone-dangling-handle");
+    CheckFactories.registerCheck<DataflowDeadCodeCheck>(
+        "bugprone-dataflow-dead-code");
     CheckFactories.registerCheck<DynamicStaticInitializersCheck>(
         "bugprone-dynamic-static-initializers");
     CheckFactories.registerCheck<EasilySwappableParametersCheck>(

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

@@ -19,6 +19,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   CopyConstructorInitCheck.cpp
   CrtpConstructorAccessibilityCheck.cpp
   DanglingHandleCheck.cpp
+  DataflowDeadCodeCheck.cpp
   DynamicStaticInitializersCheck.cpp
   EasilySwappableParametersCheck.cpp
   EmptyCatchCheck.cpp

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

@@ -0,0 +1,122 @@
+//===--- DataflowDeadCodeCheck.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 "DataflowDeadCodeCheck.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/DeadCodeModel.h"
+#include "clang/Analysis/FlowSensitive/NoopAnalysis.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::DeadCodeDiagnoser;
+using dataflow::DeadCodeModel;
+using dataflow::NoopAnalysis;
+using Diagnoser = DeadCodeDiagnoser;
+
+static constexpr llvm::StringLiteral FuncID("fun");
+
+struct ExpandedResult {
+  Diagnoser::DiagnosticEntry Entry;
+  std::optional<SourceLocation> DerefLoc;
+};
+
+using ResultType = Diagnoser::ResultType;
+
+static std::optional<ResultType> analyzeFunction(const FunctionDecl &FuncDecl) {
+  using dataflow::AdornedCFG;
+  using dataflow::DataflowAnalysisState;
+  using llvm::Expected;
+
+  ASTContext &ASTCtx = FuncDecl.getASTContext();
+
+  if (!FuncDecl.doesThisDeclarationHaveABody()) {
+    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);
+  DeadCodeModel Analysis(ASTCtx);
+  Diagnoser Diagnoser;
+
+  ResultType Diagnostics;
+
+  if (llvm::Error E =
+          dataflow::diagnoseFunction<DeadCodeModel, Diagnoser::DiagnosticEntry>(
+              FuncDecl, ASTCtx, Diagnoser)
+              .moveInto(Diagnostics)) {
+    llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
+                 << ".\n";
+    return std::nullopt;
+  }
+
+  return Diagnostics;
+}
+
+void DataflowDeadCodeCheck::registerMatchers(MatchFinder *Finder) {
+  using namespace ast_matchers;
+  Finder->addMatcher(
+      decl(
+          anyOf(functionDecl(unless(isExpansionInSystemHeader()),
+                             // FIXME: Remove the filter below when lambdas are
+                             // well supported by the check.
+                             unless(hasDeclContext(cxxRecordDecl(isLambda())))),
+                cxxConstructorDecl(
+                    unless(hasDeclContext(cxxRecordDecl(isLambda()))))))
+          .bind(FuncID),
+      this);
+}
+
+void DataflowDeadCodeCheck::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 [Loc, Type] : *Diagnostics) {
+
+      switch (Type) {
+      case Diagnoser::DiagnosticType::AlwaysTrue:
+        diag(Loc, "dead code - branching condition is always true");
+        break;
+
+      case Diagnoser::DiagnosticType::AlwaysFalse:
+        diag(Loc, "dead code - branching condition is always false");
+        break;
+      }
+    }
+  }
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,37 @@
+//===--- DataflowDeadCodeCheck.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_DEADCODECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DEADCODECHECK_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 DataflowDeadCodeCheck : public ClangTidyCheck {
+public:
+  DataflowDeadCodeCheck(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_DEADCODECHECK_H

clang-tools-extra/clangd/TidyProvider.cpp

@@ -222,7 +222,7 @@ TidyProvider disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks) {
       "-hicpp-invalid-access-moved",
       // Check uses dataflow analysis, which might hang/crash unexpectedly on
       // incomplete code.
-      "-bugprone-unchecked-optional-access");
+      "-bugprone-dataflow-dead-code", "-bugprone-unchecked-optional-access");
 
   size_t Size = BadChecks.size();
   for (const std::string &Str : ExtraBadChecks) {

clang-tools-extra/docs/clang-tidy/checks/bugprone/dataflow-dead-code.rst

@@ -0,0 +1,79 @@
+.. title:: clang-tidy - bugprone-dataflow-dead-code
+
+bugprone-dataflow-dead-code
+===========================
+
+*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.
+
+Finds instances of always-true and always-false conditions in branch statements.
+
+.. code-block:: c++
+
+   void f(bool a, bool b) {
+     if (a) {
+       return;
+     } else if (a == b) {
+       if (b) { // warning: dead code - branching condition is always false
+         return;
+       }
+     }
+   }
+
+Notes
+-----
+
+True and false literals
+-----------------------
+
+Since macro and template code commonly uses always-true and always-false loops,
+the literals ``true`` and ``false`` are excluded from being matched outright.
+Assertion statements are a common example.
+
+.. code-block:: c++
+
+   // common way to define asserts in libraries
+   #define assert(x) do {} while(false)
+
+   void f(int *param) {
+     assert(param); // no-warning, even though while(false) is always false
+   }
+
+C++ class support
+-----------------
+
+Support for C++ datastructures is limited due to framework limitations.
+Calling non-const member functions of a class do not invalidate member variable
+values.
+
+.. code-block:: c++
+
+   struct S {
+     bool a;
+     void change_a() { a = random_bool(); }
+   };
+
+   void f(S s) {
+     if (s.a) {
+       return;
+     }
+     s.change_a();
+     if (s.a) {} // false-positive: condition is always false
+   }
+
+Marking of unexpected values
+----------------------------
+
+Due to framework limitations, the check currently utilizes a mark-and-check
+approach. First it marks all loop condition values, then it checks whether the
+value is always true or not. This can lead to the same value showing as
+always-true in an unexpected place, or in an unexpected expression.
+
+.. code-block:: c++
+
+   void f(int a, int b) {
+     if (a == b) {
+       f(a == b); // unexpected-warning: condition is always true
+     }
+   }

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -91,6 +91,7 @@ Clang-Tidy Checks
    :doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes"
    :doc:`bugprone-crtp-constructor-accessibility <bugprone/crtp-constructor-accessibility>`, "Yes"
    :doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,
+   :doc:`bugprone-dataflow-dead-code <bugprone/dataflow-dead-code>`,
    :doc:`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers>`,
    :doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`,
    :doc:`bugprone-empty-catch <bugprone/empty-catch>`,

clang-tools-extra/test/clang-tidy/checkers/bugprone/dataflow-dead-code.cpp

@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s bugprone-dataflow-dead-code %t
+
+void simple_cases(bool a) {
+  if (a) {
+    if (a) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dead code - branching condition is always true
+    while (!a) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: dead code - branching condition is always false
+    // fixme: false positive
+    // CHECK-MESSAGES: :[[@LINE-3]]:13: warning: dead code - branching condition is always true
+  }
+}
+
+void transitive(bool a, bool b) {
+  if (a) {
+    return;
+  } else if (a == b) {
+    // fixme: false positive
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: dead code - branching condition is always false
+    if (b) { 
+      // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dead code - branching condition is always false
+      return;
+    }
+  }
+}
+
+#define assert(x) do {} while(false)
+
+void assert_excluded(int *x) {
+  assert(x); // no-warning
+}
+
+extern bool random_bool();
+
+void empty_forloop_excluded() {
+  for (;;) { // no-warning
+    if (random_bool()) { // no-warning
+      return;
+    }
+  }
+}
+
+struct S {
+  bool a;
+  void change_a() { a = random_bool(); }
+};
+
+void false_positive_classes(S s) {
+  if (s.a) {
+    return;
+  }
+  s.change_a();
+  if (s.a) {} // fixme: false positive
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dead code - branching condition is always false
+}