[clang] a36c2dd - [clang][dataflow] Add modeling of Chromium's CHECK functionality

Yitzhak Mandelbaum via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 18 07:40:17 PDT 2022


Author: Yitzhak Mandelbaum
Date: 2022-03-18T14:39:23Z
New Revision: a36c2dd6d54c6ff854cb4872cd2831ed995e9275

URL: https://github.com/llvm/llvm-project/commit/a36c2dd6d54c6ff854cb4872cd2831ed995e9275
DIFF: https://github.com/llvm/llvm-project/commit/a36c2dd6d54c6ff854cb4872cd2831ed995e9275.diff

LOG: [clang][dataflow] Add modeling of Chromium's CHECK functionality

Chromium's implementation of assertions (`CHECK`, `DCHECK`, etc.) are not
annotated with "noreturn", by default. This patch adds a model of the logical
implications of successfully executing one of these assertions.

Differential Revision: https://reviews.llvm.org/D121797

Added: 
    clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
    clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
    clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp

Modified: 
    clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
    clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
new file mode 100644
index 0000000000000..93c427bd1ddc6
--- /dev/null
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
@@ -0,0 +1,39 @@
+//===-- ChromiumCheckModel.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 dataflow model for Chromium's family of CHECK functions.
+//
+//===----------------------------------------------------------------------===//
+#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_CHROMIUMCHECKMODEL_H
+#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_CHROMIUMCHECKMODEL_H
+
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "llvm/ADT/DenseSet.h"
+
+namespace clang {
+namespace dataflow {
+
+/// Models the behavior of Chromium's CHECK, DCHECK, etc. macros, so that code
+/// after a call to `*CHECK` can rely on the condition being true.
+class ChromiumCheckModel : public DataflowModel {
+public:
+  ChromiumCheckModel() = default;
+  bool transfer(const Stmt *Stmt, Environment &Env) override;
+
+private:
+  /// Declarations for `::logging::CheckError::.*Check`, lazily initialized.
+  llvm::SmallDenseSet<const CXXMethodDecl *> CheckDecls;
+};
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_CHROMIUMCHECKMODEL_H

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
index 5bed00c4bfdc6..89bbe8791eb2c 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
+++ b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_clang_library(clangAnalysisFlowSensitiveModels
+  ChromiumCheckModel.cpp
   UncheckedOptionalAccessModel.cpp
 
   LINK_LIBS

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
new file mode 100644
index 0000000000000..3910847316a59
--- /dev/null
+++ b/clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
@@ -0,0 +1,67 @@
+//===-- ChromiumCheckModel.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
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/ADT/DenseSet.h"
+
+namespace clang {
+namespace dataflow {
+
+/// Determines whether `D` is one of the methods used to implement Chromium's
+/// `CHECK` macros. Populates `CheckDecls`, if empty.
+bool isCheckLikeMethod(llvm::SmallDenseSet<const CXXMethodDecl *> &CheckDecls,
+                       const CXXMethodDecl &D) {
+  // All of the methods of interest are static, so avoid any lookup for
+  // non-static methods (the common case).
+  if (!D.isStatic())
+    return false;
+
+  if (CheckDecls.empty()) {
+    // Attempt to initialize `CheckDecls` with the methods in class
+    // `CheckError`.
+    const CXXRecordDecl *ParentClass = D.getParent();
+    if (ParentClass == nullptr || !ParentClass->getDeclName().isIdentifier() ||
+        ParentClass->getName() != "CheckError")
+      return false;
+
+    // Check whether namespace is "logging".
+    const auto *N =
+        dyn_cast_or_null<NamespaceDecl>(ParentClass->getDeclContext());
+    if (N == nullptr || !N->getDeclName().isIdentifier() ||
+        N->getName() != "logging")
+      return false;
+
+    // Check whether "logging" is a top-level namespace.
+    if (N->getParent() == nullptr || !N->getParent()->isTranslationUnit())
+      return false;
+
+    for (const CXXMethodDecl *M : ParentClass->methods())
+      if (M->getDeclName().isIdentifier() && M->getName().endswith("Check"))
+        CheckDecls.insert(M);
+  }
+
+  return CheckDecls.contains(&D);
+}
+
+bool ChromiumCheckModel::transfer(const Stmt *Stmt, Environment &Env) {
+  if (const auto *Call = dyn_cast<CallExpr>(Stmt)) {
+    if (const auto *M = dyn_cast<CXXMethodDecl>(Call->getDirectCallee())) {
+      if (isCheckLikeMethod(CheckDecls, *M)) {
+        // Mark this branch as unreachable.
+        Env.addToFlowCondition(Env.getBoolLiteralValue(false));
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+} // namespace dataflow
+} // namespace clang

diff  --git a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
index c2bae4d0e4882..c299e039ff822 100644
--- a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
   MapLatticeTest.cpp

diff  --git a/clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
new file mode 100644
index 0000000000000..a61d6dc682d87
--- /dev/null
+++ b/clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
@@ -0,0 +1,219 @@
+//===- ChromiumCheckModelTest.cpp -----------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+// FIXME: Move this to clang/unittests/Analysis/FlowSensitive/Models.
+
+#include "clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h"
+#include "NoopAnalysis.h"
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <string>
+
+using namespace clang;
+using namespace dataflow;
+using namespace test;
+
+namespace {
+using ::testing::_;
+using ::testing::ElementsAre;
+using ::testing::NotNull;
+using ::testing::Pair;
+
+static constexpr char ChromiumCheckHeader[] = R"(
+namespace std {
+class ostream;
+} // namespace std
+
+namespace logging {
+class VoidifyStream {
+ public:
+  VoidifyStream() = default;
+  void operator&(std::ostream&) {}
+};
+
+class CheckError {
+ public:
+  static CheckError Check(const char* file, int line, const char* condition);
+  static CheckError DCheck(const char* file, int line, const char* condition);
+  static CheckError PCheck(const char* file, int line, const char* condition);
+  static CheckError PCheck(const char* file, int line);
+  static CheckError DPCheck(const char* file, int line, const char* condition);
+
+  std::ostream& stream();
+
+  ~CheckError();
+
+  CheckError(const CheckError& other) = delete;
+  CheckError& operator=(const CheckError& other) = delete;
+  CheckError(CheckError&& other) = default;
+  CheckError& operator=(CheckError&& other) = default;
+};
+
+} // namespace logging
+
+#define LAZY_CHECK_STREAM(stream, condition) \
+  !(condition) ? (void)0 : ::logging::VoidifyStream() & (stream)
+
+#define CHECK(condition)                                                     \
+  LAZY_CHECK_STREAM(                                                         \
+      ::logging::CheckError::Check(__FILE__, __LINE__, #condition).stream(), \
+      !(condition))
+
+#define PCHECK(condition)                                                     \
+  LAZY_CHECK_STREAM(                                                          \
+      ::logging::CheckError::PCheck(__FILE__, __LINE__, #condition).stream(), \
+      !(condition))
+
+#define DCHECK(condition)                                                     \
+  LAZY_CHECK_STREAM(                                                          \
+      ::logging::CheckError::DCheck(__FILE__, __LINE__, #condition).stream(), \
+      !(condition))
+
+#define DPCHECK(condition)                                                     \
+  LAZY_CHECK_STREAM(                                                           \
+      ::logging::CheckError::DPCheck(__FILE__, __LINE__, #condition).stream(), \
+      !(condition))
+)";
+
+// A definition of the `CheckError` class that looks like the Chromium one, but
+// is actually something else.
+static constexpr char OtherCheckHeader[] = R"(
+namespace other {
+namespace logging {
+class CheckError {
+ public:
+  static CheckError Check(const char* file, int line, const char* condition);
+};
+} // namespace logging
+} // namespace other
+)";
+
+/// Replaces all occurrences of `Pattern` in `S` with `Replacement`.
+std::string ReplacePattern(std::string S, const std::string &Pattern,
+                           const std::string &Replacement) {
+  size_t Pos = 0;
+  Pos = S.find(Pattern, Pos);
+  if (Pos != std::string::npos)
+    S.replace(Pos, Pattern.size(), Replacement);
+  return S;
+}
+
+template <typename Model>
+class ModelAdaptorAnalysis
+    : public DataflowAnalysis<ModelAdaptorAnalysis<Model>, NoopLattice> {
+public:
+  explicit ModelAdaptorAnalysis(ASTContext &Context)
+      : DataflowAnalysis<ModelAdaptorAnalysis, NoopLattice>(
+            Context, /*ApplyBuiltinTransfer=*/true) {}
+
+  static NoopLattice initialElement() { return NoopLattice(); }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+    M.transfer(S, Env);
+  }
+
+private:
+  Model M;
+};
+
+class ChromiumCheckModelTest : public ::testing::TestWithParam<std::string> {
+protected:
+  template <typename Matcher>
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+    const tooling::FileContentMappings FileContents = {
+        {"check.h", ChromiumCheckHeader}, {"othercheck.h", OtherCheckHeader}};
+
+    ASSERT_THAT_ERROR(
+        test::checkDataflow<ModelAdaptorAnalysis<ChromiumCheckModel>>(
+            Code, "target",
+            [](ASTContext &C, Environment &) {
+              return ModelAdaptorAnalysis<ChromiumCheckModel>(C);
+            },
+            [&Match](
+                llvm::ArrayRef<
+                    std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                    Results,
+                ASTContext &ASTCtx) { Match(Results, ASTCtx); },
+            {"-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"},
+            FileContents),
+        llvm::Succeeded());
+  }
+};
+
+TEST_F(ChromiumCheckModelTest, CheckSuccessImpliesConditionHolds) {
+  auto Expectations =
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+             Results,
+         ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+
+        auto *FooVal = cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+
+        EXPECT_TRUE(Env.flowConditionImplies(*FooVal));
+      };
+
+  std::string Code = R"(
+    #include "check.h"
+
+    void target(bool Foo) {
+      $check(Foo);
+      bool X = true;
+      (void)X;
+      // [[p]]
+    }
+  )";
+  runDataflow(ReplacePattern(Code, "$check", "CHECK"), Expectations);
+  runDataflow(ReplacePattern(Code, "$check", "DCHECK"), Expectations);
+  runDataflow(ReplacePattern(Code, "$check", "PCHECK"), Expectations);
+  runDataflow(ReplacePattern(Code, "$check", "DPCHECK"), Expectations);
+}
+
+TEST_F(ChromiumCheckModelTest, UnrelatedCheckIgnored) {
+  auto Expectations =
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+             Results,
+         ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+
+        auto *FooVal = cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+
+        EXPECT_FALSE(Env.flowConditionImplies(*FooVal));
+      };
+
+  std::string Code = R"(
+    #include "othercheck.h"
+
+    void target(bool Foo) {
+      if (!Foo) {
+        (void)other::logging::CheckError::Check(__FILE__, __LINE__, "Foo");
+      }
+      bool X = true;
+      (void)X;
+      // [[p]]
+    }
+  )";
+  runDataflow(Code, Expectations);
+}
+} // namespace


        


More information about the cfe-commits mailing list