njames93 updated this revision to Diff 405581.
njames93 added a comment.
Herald added a subscriber: mgorny.

Add testing infrastructure to verify stack trace output


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118520

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  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.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.cpp
@@ -0,0 +1,17 @@
+// REQUIRES: plugins
+// 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 check crash-test
+// CHECK-NEXT: --- Bound Nodes Begin ---
+// CHECK-DAG: FS - { ForStmt : <{{.*}}/crash-trace.cpp:6:3, line:7:3> }
+// CHECK-DAG: DS - { DeclStmt : <{{.*}}/crash-trace.cpp:6:8, col:17> }
+// CHECK-DAG: VD - { VarDecl I : <{{.*}}/crash-trace.cpp:6:8, col:16> }
+// CHECK-DAG: IL - { IntegerLiteral : <{{.*}}/crash-trace.cpp: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
@@ -96,6 +96,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
 ^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -21,6 +21,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/PluginLoader.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
@@ -385,6 +386,12 @@
   return FS;
 }
 
+class ClangTidyCrashTraceReporter : public llvm::PrettyStackTraceEntry,
+                                    public CurProcessingCheckState {
+public:
+  void print(raw_ostream &OS) const override { return dump(OS); }
+};
+
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
@@ -494,8 +501,11 @@
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
+  ClangTidyCrashTraceReporter Trace;
+
   ClangTidyContext Context(std::move(OwningOptionsProvider),
                            AllowEnablingAnalyzerAlphaCheckers);
+  Context.setCrashTraceEngine(&Trace);
   std::vector<ClangTidyError> Errors =
       runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
                    FixNotes, EnableCheckProfile, ProfilePrefix);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -22,6 +22,10 @@
 class ASTContext;
 class SourceManager;
 
+namespace ast_matchers {
+class BoundNodes;
+} // namespace ast_matchers
+
 namespace tidy {
 class CachedGlobList;
 
@@ -54,6 +58,31 @@
   }
 };
 
+class CurProcessingCheckState {
+public:
+  void dump(raw_ostream &OS) const;
+  void onProcessingCheckStart(StringRef CheckName,
+                              const ast_matchers::BoundNodes &BoundNodes) {
+    assert(CurContext && "ASTContext not set");
+    CurrentCheckName = CheckName;
+    CurNodes = &BoundNodes;
+  }
+
+  void onProcessingCheckEnd() {
+    CurrentCheckName = "";
+    CurNodes = nullptr;
+  }
+
+  void setContext(const ASTContext &Ctx) { CurContext = &Ctx; }
+
+  void clearContext() { CurContext = nullptr; }
+
+private:
+  mutable StringRef CurrentCheckName;
+  const ast_matchers::BoundNodes *CurNodes;
+  const ASTContext *CurContext;
+};
+
 /// Every \c ClangTidyCheck reports errors through a \c DiagnosticsEngine
 /// provided by this context.
 ///
@@ -75,6 +104,10 @@
     this->DiagEngine = DiagEngine;
   }
 
+  void setCrashTraceEngine(CurProcessingCheckState *State) {
+    this->CurrentlyProcessing = State;
+  }
+
   ~ClangTidyContext();
 
   /// Report any errors detected using this method.
@@ -197,12 +230,24 @@
             DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
   }
 
+  void onProcessingCheckStart(StringRef CheckName,
+                              const ast_matchers::BoundNodes &Result) const {
+    if (CurrentlyProcessing)
+      CurrentlyProcessing->onProcessingCheckStart(CheckName, Result);
+  }
+
+  void onProcessingCheckEnd() const {
+    if (CurrentlyProcessing)
+      CurrentlyProcessing->onProcessingCheckEnd();
+  }
+
 private:
   // Writes to Stats.
   friend class ClangTidyDiagnosticConsumer;
 
   DiagnosticsEngine *DiagEngine;
   std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
+  CurProcessingCheckState *CurrentlyProcessing;
 
   std::string CurrentFile;
   ClangTidyOptions CurrentOptions;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Attr.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -161,7 +162,7 @@
     std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
     bool AllowEnablingAnalyzerAlphaCheckers)
     : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
-      Profile(false),
+      CurrentlyProcessing(nullptr), Profile(false),
       AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
   // Before the first translation unit we can get errors related to command-line
   // parsing, use empty string for the file name in this case.
@@ -231,6 +232,8 @@
 void ClangTidyContext::setASTContext(ASTContext *Context) {
   DiagEngine->SetArgToStringFn(&FormatASTNodeDiagnosticArgument, Context);
   LangOpts = Context->getLangOpts();
+  if (CurrentlyProcessing)
+    CurrentlyProcessing->setContext(*Context);
 }
 
 const ClangTidyGlobalOptions &ClangTidyContext::getGlobalOptions() const {
@@ -337,10 +340,51 @@
   }
   return Result;
 }
-
 } // namespace tidy
 } // namespace clang
 
+void CurProcessingCheckState::dump(raw_ostream &OS) const {
+  if (CurrentCheckName.empty())
+    return;
+  // This should be an assert, but asserts shouldn't be used in signal
+  // handlers
+  if (!CurContext)
+    return;
+  OS << "Processing check " << CurrentCheckName << '\n';
+  CurrentCheckName = "";
+  const clang::ast_matchers::BoundNodes::IDToNodeMap &Map = CurNodes->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, CurContext->getSourceManager());
+    } else if (const auto *S = Item.second.get<Stmt>()) {
+      OS << S->getStmtClassName() << " : ";
+      S->getSourceRange().print(OS, CurContext->getSourceManager());
+    } else if (const auto *T = Item.second.get<Type>()) {
+      OS << T->getTypeClassName() << "Type : ";
+      QualType(T, 0).print(OS, CurContext->getPrintingPolicy());
+    } else if (const auto *QT = Item.second.get<QualType>()) {
+      OS << "QualType : ";
+      QT->print(OS, CurContext->getPrintingPolicy());
+    } else {
+      OS << Item.second.getNodeKind().asStringRef() << " : ";
+      Item.second.getSourceRange().print(OS, CurContext->getSourceManager());
+    }
+    OS << " }\n";
+  }
+  OS << "--- Bound Nodes End ---\n";
+}
+
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -39,10 +39,12 @@
 }
 
 void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) {
+  Context->onProcessingCheckStart(CheckName, Result.Nodes);
   // For historical reasons, checks don't implement the MatchFinder run()
   // callback directly. We keep the run()/check() distinction to avoid interface
   // churn, and to allow us to add cross-cutting logic in the future.
   check(Result);
+  Context->onProcessingCheckEnd();
 }
 
 ClangTidyCheck::OptionsView::OptionsView(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to