njames93 updated this revision to Diff 410139.
njames93 added a comment.

Moved TraceReporter into the MatchASTVisitor class
Added some missing new lines
Fixed patch not being based off a commit in trunk


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120185/new/

https://reviews.llvm.org/D120185

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
  clang-tools-extra/test/lit.cfg.py
  clang/lib/ASTMatchers/ASTMatchFinder.cpp

Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Timer.h"
 #include <deque>
 #include <memory>
@@ -760,11 +761,64 @@
         D);
   }
 
+  class TraceReporter : llvm::PrettyStackTraceEntry {
+  public:
+    TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
+    void print(raw_ostream &OS) const override {
+      if (!MV.CurMatched)
+        return;
+      assert(MV.ActiveASTContext &&
+             "ActiveASTContext should be set if there is a matched callback");
+
+      OS << "Processing '" << MV.CurMatched->getID() << "'\n";
+      const BoundNodes::IDToNodeMap &Map = MV.CurBoundNodes->getMap();
+      if (Map.empty()) {
+        OS << "No bound nodes\n";
+        return;
+      }
+      OS << "--- Bound Nodes Begin ---\n";
+      for (const auto &Item : Map) {
+        OS << "    " << Item.first << " - { ";
+        if (const auto *D = Item.second.get<Decl>()) {
+          OS << D->getDeclKindName() << "Decl ";
+          if (const auto *ND = dyn_cast<NamedDecl>(D))
+            OS << ND->getDeclName() << " : ";
+          else
+            OS << "<anonymous> ";
+          D->getSourceRange().print(OS,
+                                    MV.ActiveASTContext->getSourceManager());
+        } else if (const auto *S = Item.second.get<Stmt>()) {
+          OS << S->getStmtClassName() << " : ";
+          S->getSourceRange().print(OS,
+                                    MV.ActiveASTContext->getSourceManager());
+        } else if (const auto *T = Item.second.get<Type>()) {
+          OS << T->getTypeClassName() << "Type : ";
+          QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
+        } else if (const auto *QT = Item.second.get<QualType>()) {
+          OS << "QualType : ";
+          QT->print(OS, MV.ActiveASTContext->getPrintingPolicy());
+        } else {
+          OS << Item.second.getNodeKind().asStringRef() << " : ";
+          Item.second.getSourceRange().print(
+              OS, MV.ActiveASTContext->getSourceManager());
+        }
+        OS << " }\n";
+      }
+      OS << "--- Bound Nodes End ---\n";
+    }
+
+  private:
+    const MatchASTVisitor &MV;
+  };
+
 private:
   bool TraversingASTNodeNotSpelledInSource = false;
   bool TraversingASTNodeNotAsIs = false;
   bool TraversingASTChildrenNotSpelledInSource = false;
 
+  const MatchCallback *CurMatched = nullptr;
+  const BoundNodes *CurBoundNodes = nullptr;
+
   struct ASTNodeNotSpelledInSourceScope {
     ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
         : MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
@@ -831,7 +885,7 @@
         Timer.setBucket(&TimeByBucket[MP.second->getID()]);
       BoundNodesTreeBuilder Builder;
       if (MP.first.matches(Node, this, &Builder)) {
-        MatchVisitor Visitor(ActiveASTContext, MP.second);
+        MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
         Builder.visitMatches(&Visitor);
       }
     }
@@ -863,7 +917,7 @@
       }
 
       if (MP.first.matches(DynNode, this, &Builder)) {
-        MatchVisitor Visitor(ActiveASTContext, MP.second);
+        MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
         Builder.visitMatches(&Visitor);
       }
     }
@@ -1049,18 +1103,36 @@
   // Implements a BoundNodesTree::Visitor that calls a MatchCallback with
   // the aggregated bound nodes for each match.
   class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
+    struct CurBoundScope {
+      CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
+        assert(MV.CurMatched && !MV.CurBoundNodes);
+        MV.CurBoundNodes = &BN;
+      }
+
+      ~CurBoundScope() { MV.CurBoundNodes = nullptr; }
+
+    private:
+      MatchASTVisitor &MV;
+    };
+
   public:
-    MatchVisitor(ASTContext* Context,
-                 MatchFinder::MatchCallback* Callback)
-      : Context(Context),
-        Callback(Callback) {}
+    MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
+                 MatchFinder::MatchCallback *Callback)
+        : MV(MV), Context(Context), Callback(Callback) {
+      assert(!MV.CurMatched && !MV.CurBoundNodes);
+      MV.CurMatched = Callback;
+    }
+
+    ~MatchVisitor() { MV.CurMatched = nullptr; }
 
     void visitMatch(const BoundNodes& BoundNodesView) override {
       TraversalKindScope RAII(*Context, Callback->getCheckTraversalKind());
+      CurBoundScope RAII2(MV, BoundNodesView);
       Callback->run(MatchFinder::MatchResult(BoundNodesView, Context));
     }
 
   private:
+    MatchASTVisitor &MV;
     ASTContext* Context;
     MatchFinder::MatchCallback* Callback;
   };
@@ -1470,6 +1542,7 @@
 
 void MatchFinder::matchAST(ASTContext &Context) {
   internal::MatchASTVisitor Visitor(&Matchers, Options);
+  internal::MatchASTVisitor::TraceReporter StackTrace(Visitor);
   Visitor.set_active_ast_context(&Context);
   Visitor.onStartOfTranslationUnit();
   Visitor.TraverseAST(Context);
Index: clang-tools-extra/test/lit.cfg.py
===================================================================
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -48,7 +48,7 @@
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
 # from test suites; there won't be any lit tests within them.
-config.excludes = ['Inputs']
+config.excludes = ['Inputs', 'CTCrashTestTrace.cpp']
 
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
Index: clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
@@ -0,0 +1,17 @@
+// REQUIRES: plugins, crash-recovery
+// RUN: not --crash clang-tidy %s -checks='-*,crash-test' -load               \
+// RUN:   %llvmshlibdir/CTCrashTestTrace%pluginext 2>&1 | FileCheck %s
+
+void foo() {
+  for (int I = 0; I < 5; ++I) {
+  }
+}
+// CHECK: Processing 'crash-test'
+// CHECK-NEXT: --- Bound Nodes Begin ---
+// CHECK-DAG: FS - { ForStmt : <{{.*}}/crash-trace.c:6:3, line:7:3> }
+// CHECK-DAG: DS - { DeclStmt : <{{.*}}/crash-trace.c:6:8, col:17> }
+// CHECK-DAG: VD - { VarDecl I : <{{.*}}/crash-trace.c:6:8, col:16> }
+// CHECK-DAG: IL - { IntegerLiteral : <{{.*}}/crash-trace.c:6:16> }
+// CHECK-DAG: QT - { QualType : int }
+// CHECK-DAG: T - { BuiltinType : int }
+// CHECK-NEXT: --- Bound Nodes End ---
Index: clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
@@ -0,0 +1,69 @@
+//===------------------------ CTCrashTestTrace.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
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file This file implements a clang-tidy plugin that registers one check
+///  designed to crash on any match. This is to test clang-tidys stack trace
+///  output for when a buggy check causes a crash.
+///
+//===----------------------------------------------------------------------===//
+
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class CrashingCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  void registerMatchers(MatchFinder *Finder) override {
+    Finder->addMatcher(
+        forStmt(
+            hasLoopInit(
+                declStmt(hasSingleDecl(varDecl(hasType(qualType().bind("QT")),
+                                               hasType(type().bind("T")),
+                                               hasInitializer(
+                                                   integerLiteral().bind("IL")))
+                                           .bind("VD")))
+                    .bind("DS")))
+            .bind("FS"),
+        this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+    LLVM_BUILTIN_TRAP;
+  }
+
+  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+class CTCrashTestTraceModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<CrashingCheck>("crash-test");
+  }
+};
+} // namespace
+
+namespace tidy {
+// Register the CTCrashTestTraceModule using this statically initialized
+// variable.
+static ClangTidyModuleRegistry::Add<::CTCrashTestTraceModule>
+    X("crash-module", "Add a check which will trap on any match");
+} // namespace tidy
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTCrashTestTraceModule.
+volatile int CTCrashTestTraceAnchor = 0;
Index: clang-tools-extra/test/CMakeLists.txt
===================================================================
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -105,6 +105,22 @@
         )
       endif()
   endif()
+
+  llvm_add_library(
+      CTCrashTestTrace
+      MODULE clang-tidy/CTCrashTestTrace.cpp
+      PLUGIN_TOOL clang-tidy
+      DEPENDS clang-tidy-headers)
+
+  if(TARGET CTCrashTestTrace)
+      list(APPEND CLANG_TOOLS_TEST_DEPS CTCrashTestTrace LLVMHello)
+      target_include_directories(CTCrashTestTrace PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+      if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+        set(LLVM_LINK_COMPONENTS
+          Support
+        )
+      endif()
+  endif()
 endif()
 
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,9 @@
 Improvements to clang-tidy
 --------------------------
 
+- Added trace code to help narrow down any checks and the relevant source code
+  that result in crashes.
+
 New checks
 ^^^^^^^^^^
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to