Author: Nathan James Date: 2022-03-25T17:53:58Z New Revision: b97f26083bd04ccbdd63b0c726e047496e5b847a
URL: https://github.com/llvm/llvm-project/commit/b97f26083bd04ccbdd63b0c726e047496e5b847a DIFF: https://github.com/llvm/llvm-project/commit/b97f26083bd04ccbdd63b0c726e047496e5b847a.diff LOG: Reland "[ASTMatchers] Output currently processing match and nodes on crash" This reverts commit cff34ccb605aa78030cd51cfe44362ed1c1fb80b. This relands commit d89f9e963e4979466193dc6a15fe091bf7ca5c47 Added: Modified: clang-tools-extra/docs/ReleaseNotes.rst clang/lib/ASTMatchers/ASTMatchFinder.cpp clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b54fca707eda8..282cf1d0a4cc1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -96,6 +96,9 @@ The improvements are... 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 index b19a7fe3be04c..70598460151ae 100644 --- 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 <deque> #include <memory> @@ -760,11 +761,67 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, 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<Decl>()) { + OS << D->getDeclKindName() << "Decl "; + if (const auto *ND = dyn_cast<NamedDecl>(D)) { + ND->printQualifiedName(OS); + OS << " : "; + } else + OS << ": "; + 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 +888,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, 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 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, } 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 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, // 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::match(const clang::DynTypedNode &Node, ASTContext &Context) { 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 index 2766065f9e5d1..b86917c36dcb1 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -12,6 +12,7 @@ #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 "gtest/gtest.h" @@ -34,6 +35,86 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) { EXPECT_TRUE(notMatches("class X {};", HasEmptyName)); }, ""); } + +// FIXME: Figure out why back traces aren't being generated on clang builds on +// windows. +#if ENABLE_BACKTRACES && (!defined(_MSC_VER) || !defined(__clang__)) +template <typename MatcherT> +static void crashTestNodeDump(MatcherT Matcher, + ArrayRef<StringRef> MatchedNodes, + StringRef Code) { + llvm::EnablePrettyStackTrace(); + MatchFinder Finder; + + struct CrashCallback : public MatchFinder::MatchCallback { + void run(const MatchFinder::MatchResult &Result) override { abort(); } + llvm::Optional<TraversalKind> 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<testing::PolymorphicMatcher< + testing::internal::HasSubstrMatcher<std::string>>> + 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 : <input.cc:3:5, line:4:5> }", + "DS - { DeclStmt : <input.cc:3:10, col:19> }", + "IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }", + "T - { BuiltinType : int }", + "VD - { VarDecl I : <input.cc:3:10, col:18> }"}, + 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) : <input.cc:1:1, col:36> }", + "Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, " + "col:29> }"}, + "struct { int operator+(int) const; } Unnamed;"); + crashTestNodeDump( + cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")), + hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))), + {"Ctor - { CXXConstructorDecl Foo::Foo : <input.cc:1:14, col:28> }", + "Dtor - { CXXDestructorDecl Foo::~Foo : <input.cc:1:31, col:46> }"}, + "struct Foo { Foo() = default; ~Foo() = default; };"); +} +#endif // ENABLE_BACKTRACES #endif TEST(ConstructVariadic, MismatchedTypes_Regression) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
