diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 ^^^^^^^^^^ diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt --- a/clang-tools-extra/test/CMakeLists.txt +++ b/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" diff --git a/clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp b/clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp new file mode 100644 --- /dev/null +++ b/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 getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +class CTCrashTestTraceModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck("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; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c b/clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c new file mode 100644 --- /dev/null +++ b/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 --- diff --git a/clang-tools-extra/test/lit.cfg.py b/clang-tools-extra/test/lit.cfg.py --- a/clang-tools-extra/test/lit.cfg.py +++ b/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__) diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/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 #include @@ -416,6 +417,8 @@ // matchers. class MatchASTVisitor : public RecursiveASTVisitor, 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()) { + OS << D->getDeclKindName() << "Decl "; + if (const auto *ND = dyn_cast(D)) + OS << ND->getDeclName() << " : "; + else + OS << " "; + D->getSourceRange().print(OS, MV.ActiveASTContext->getSourceManager()); + } else if (const auto *S = Item.second.get()) { + OS << S->getStmtClassName() << " : "; + S->getSourceRange().print(OS, MV.ActiveASTContext->getSourceManager()); + } else if (const auto *T = Item.second.get()) { + OS << T->getTypeClassName() << "Type : "; + QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy()); + } else if (const auto *QT = Item.second.get()) { + 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);