njames93 created this revision.
njames93 added reviewers: klimek, aaron.ballman, LegalizeAdulthood.
Herald added a subscriber: mgorny.
njames93 requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
Create a PrettyStackTraceEvent that will dump the current `MatchCallback` id as
well as the `BoundNodes` if the 'run' method of a `MatchCallback` results in a
crash.
The purpose of this is sometimes clang-tidy checks can crash in the `check`
method. And in a large codebase with alot of checks enabled and in a release
build, it can be near impossible to figure out which check as well as the
source code that caused the crash. Without that information a reproducer is
very hard to create.
This is a more generalised version of D118520
<https://reviews.llvm.org/D118520> which has a nicer integration and should be
useful to clients other than clang-tidy.
Repository:
rG LLVM Github Monorepo
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>
@@ -416,6 +417,8 @@
// matchers.
class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
public ASTMatchFinder {
+ friend class BoundNodeRunStackTrace;
+
public:
MatchASTVisitor(const MatchFinder::MatchersByType *Matchers,
const MatchFinder::MatchFinderOptions &Options)
@@ -765,6 +768,9 @@
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 +837,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 +869,7 @@
}
if (MP.first.matches(DynNode, this, &Builder)) {
- MatchVisitor Visitor(ActiveASTContext, MP.second);
+ MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
Builder.visitMatches(&Visitor);
}
}
@@ -1049,18 +1055,34 @@
// Implements a BoundNodesTree::Visitor that calls a MatchCallback with
// the aggregated bound nodes for each match.
class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
- public:
- MatchVisitor(ASTContext* Context,
- MatchFinder::MatchCallback* Callback)
- : Context(Context),
- Callback(Callback) {}
+ 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(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;
};
@@ -1132,6 +1154,54 @@
MemoizationMap ResultCache;
};
+class BoundNodeRunStackTrace : llvm::PrettyStackTraceEntry {
+public:
+ BoundNodeRunStackTrace(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;
+};
+
static CXXRecordDecl *
getAsCXXRecordDeclOrPrimaryTemplate(const Type *TypeNode) {
if (auto *RD = TypeNode->getAsCXXRecordDecl())
@@ -1470,6 +1540,7 @@
void MatchFinder::matchAST(ASTContext &Context) {
internal::MatchASTVisitor Visitor(&Matchers, Options);
+ internal::BoundNodeRunStackTrace 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
@@ -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
^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
