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/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 @@ -760,11 +761,67 @@ D); } + class TraceReporter : llvm::PrettyStackTraceEntry { + public: + TraceReporter(const MatchASTVisitor &MV) : MV(MV) {} + void print(raw_ostream &OS) const override { + if (!MV.CurMatched) { + OS << "ASTMatcher: Not currently matching\n"; + return; + } + assert(MV.ActiveASTContext && + "ActiveASTContext should be set if there is a matched callback"); + + OS << "ASTMatcher: 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)) { + ND->printQualifiedName(OS); + OS << " : "; + } 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; + }; + 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 +888,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 +920,7 @@ } if (MP.first.matches(DynNode, this, &Builder)) { - MatchVisitor Visitor(ActiveASTContext, MP.second); + MatchVisitor Visitor(*this, ActiveASTContext, MP.second); Builder.visitMatches(&Visitor); } } @@ -1049,18 +1106,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 +1545,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); diff --git a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp --- a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -12,8 +12,10 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/Triple.h" +#include "llvm/Config/config.h" #include "llvm/Support/Host.h" #include "llvm/Testing/Support/SupportHelpers.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" namespace clang { @@ -34,6 +36,87 @@ EXPECT_TRUE(notMatches("class X {};", HasEmptyName)); }, ""); } + +#if ENABLE_BACKTRACES + +template +static void crashTestNodeDump(MatcherT Matcher, + ArrayRef MatchedNodes, + StringRef Code) { + llvm::EnablePrettyStackTrace(); + MatchFinder Finder; + + struct CrashCallback : public MatchFinder::MatchCallback { + void run(const MatchFinder::MatchResult &Result) override { + LLVM_BUILTIN_TRAP; + } + llvm::Optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + StringRef getID() const override { return "CrashTester"; } + } Callback; + Finder.addMatcher(std::move(Matcher), &Callback); + if (MatchedNodes.empty()) { + ASSERT_DEATH(tooling::runToolOnCode( + newFrontendActionFactory(&Finder)->create(), Code), + testing::HasSubstr( + "ASTMatcher: Processing 'CrashTester'\nNo bound nodes")); + } else { + std::vector>> + Matchers; + Matchers.reserve(MatchedNodes.size()); + for (auto Node : MatchedNodes) { + Matchers.push_back(testing::HasSubstr(Node.str())); + } + auto CrashMatcher = testing::AllOf( + testing::HasSubstr( + "ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"), + testing::HasSubstr("--- Bound Nodes End ---"), + testing::AllOfArray(Matchers)); + + ASSERT_DEATH(tooling::runToolOnCode( + newFrontendActionFactory(&Finder)->create(), Code), + CrashMatcher); + } +} +TEST(MatcherCrashDeathTest, CrashOnCallbackDump) { + crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }"); + crashTestNodeDump( + forStmt(hasLoopInit(declStmt(hasSingleDecl( + varDecl(hasType(qualType().bind("QT")), + hasType(type().bind("T")), + hasInitializer( + integerLiteral().bind("IL"))) + .bind("VD"))) + .bind("DS"))) + .bind("FS"), + {"FS - { ForStmt : }", + "DS - { DeclStmt : }", + "IL - { IntegerLiteral : }", "QT - { QualType : int }", + "T - { BuiltinType : int }", + "VD - { VarDecl I : }"}, + R"cpp( + void foo() { + for (int I = 0; I < 5; ++I) { + } + } + )cpp"); + crashTestNodeDump( + cxxRecordDecl(hasMethod(cxxMethodDecl(hasName("operator+")).bind("Op+"))) + .bind("Unnamed"), + {"Unnamed - { CXXRecordDecl (anonymous) : }", + "Op+ - { CXXMethodDecl (anonymous struct)::operator+ : }"}, + "struct { int operator+(int) const; } Unnamed;"); + crashTestNodeDump( + cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")), + hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))), + {"Ctor - { CXXConstructorDecl Foo::Foo : }", + "Dtor - { CXXDestructorDecl Foo::~Foo : }"}, + "struct Foo { Foo() = default; ~Foo() = default; };"); +} +#endif // ENABLE_BACKTRACES #endif TEST(ConstructVariadic, MismatchedTypes_Regression) {