Author: Nathan James Date: 2022-04-05T21:47:16+01:00 New Revision: b4ad3c3891e550b04db3aca73e1f34007e94eaa8
URL: https://github.com/llvm/llvm-project/commit/b4ad3c3891e550b04db3aca73e1f34007e94eaa8 DIFF: https://github.com/llvm/llvm-project/commit/b4ad3c3891e550b04db3aca73e1f34007e94eaa8.diff LOG: Reland "[ASTMatchers] Output currently matching node on crash" Extend D120185 to also log the node being matched on in case of a crash. This can help if a matcher is causing a crash or there are not enough interesting nodes bound. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D122529 This reverts commit 61d67c8eecd2547bc690f9a6bba61b9ba814c71b. This relands commit 6e33e45b943061f79ab6de1b60d04d4c6f32f8f9. Fixing the build issue on 32bit machines due to not enough free bits in the PointerUnion. Added: Modified: clang/lib/ASTMatchers/ASTMatchFinder.cpp clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp Removed: ################################################################################ diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp index 70598460151ae..5d245d3fc9b76 100644 --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -761,53 +761,189 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, D); } +private: + bool TraversingASTNodeNotSpelledInSource = false; + bool TraversingASTNodeNotAsIs = false; + bool TraversingASTChildrenNotSpelledInSource = false; + + class CurMatchData { +// We don't have enough free low bits in 32bit builds to discriminate 8 pointer +// types in PointerUnion. so split the union in 2 using a free bit from the +// callback pointer. +#define CMD_TYPES_0 \ + const QualType *, const TypeLoc *, const NestedNameSpecifier *, \ + const NestedNameSpecifierLoc * +#define CMD_TYPES_1 \ + const CXXCtorInitializer *, const TemplateArgumentLoc *, const Attr *, \ + const DynTypedNode * + +#define IMPL(Index) \ + template <typename NodeType> \ + typename std::enable_if_t< \ + llvm::is_one_of<const NodeType *, CMD_TYPES_##Index>::value> \ + SetCallbackAndRawNode(const MatchCallback *CB, const NodeType &N) { \ + assertEmpty(); \ + Callback.setPointerAndInt(CB, Index); \ + Node##Index = &N; \ + } \ + \ + template <typename T> \ + typename std::enable_if_t< \ + llvm::is_one_of<const T *, CMD_TYPES_##Index>::value, const T *> \ + getNode() const { \ + assertHoldsState(); \ + return Callback.getInt() == (Index) ? Node##Index.dyn_cast<const T *>() \ + : nullptr; \ + } + + public: + CurMatchData() : Node0(nullptr) {} + + IMPL(0) + IMPL(1) + + const MatchCallback *getCallback() const { return Callback.getPointer(); } + + void SetBoundNodes(const BoundNodes &BN) { + assertHoldsState(); + BNodes = &BN; + } + + void clearBoundNodes() { + assertHoldsState(); + BNodes = nullptr; + } + + const BoundNodes *getBoundNodes() const { + assertHoldsState(); + return BNodes; + } + + void reset() { + assertHoldsState(); + Callback.setPointerAndInt(nullptr, 0); + Node0 = nullptr; + } + + private: + void assertHoldsState() const { + assert(Callback.getPointer() != nullptr && !Node0.isNull()); + } + + void assertEmpty() const { + assert(Callback.getPointer() == nullptr && Node0.isNull() && + BNodes == nullptr); + } + + llvm::PointerIntPair<const MatchCallback *, 1> Callback; + union { + llvm::PointerUnion<CMD_TYPES_0> Node0; + llvm::PointerUnion<CMD_TYPES_1> Node1; + }; + const BoundNodes *BNodes = nullptr; + +#undef CMD_TYPES_0 +#undef CMD_TYPES_1 +#undef IMPL + } CurMatchState; + + struct CurMatchRAII { + template <typename NodeType> + CurMatchRAII(MatchASTVisitor &MV, const MatchCallback *CB, + const NodeType &NT) + : MV(MV) { + MV.CurMatchState.SetCallbackAndRawNode(CB, NT); + } + + ~CurMatchRAII() { MV.CurMatchState.reset(); } + + private: + MatchASTVisitor &MV; + }; + +public: class TraceReporter : llvm::PrettyStackTraceEntry { + static void dumpNode(const ASTContext &Ctx, const DynTypedNode &Node, + raw_ostream &OS) { + if (const auto *D = Node.get<Decl>()) { + OS << D->getDeclKindName() << "Decl "; + if (const auto *ND = dyn_cast<NamedDecl>(D)) { + ND->printQualifiedName(OS); + OS << " : "; + } else + OS << ": "; + D->getSourceRange().print(OS, Ctx.getSourceManager()); + } else if (const auto *S = Node.get<Stmt>()) { + OS << S->getStmtClassName() << " : "; + S->getSourceRange().print(OS, Ctx.getSourceManager()); + } else if (const auto *T = Node.get<Type>()) { + OS << T->getTypeClassName() << "Type : "; + QualType(T, 0).print(OS, Ctx.getPrintingPolicy()); + } else if (const auto *QT = Node.get<QualType>()) { + OS << "QualType : "; + QT->print(OS, Ctx.getPrintingPolicy()); + } else { + OS << Node.getNodeKind().asStringRef() << " : "; + Node.getSourceRange().print(OS, Ctx.getSourceManager()); + } + } + + static void dumpNodeFromState(const ASTContext &Ctx, + const CurMatchData &State, raw_ostream &OS) { + if (const DynTypedNode *MatchNode = State.getNode<DynTypedNode>()) { + dumpNode(Ctx, *MatchNode, OS); + } else if (const auto *QT = State.getNode<QualType>()) { + dumpNode(Ctx, DynTypedNode::create(*QT), OS); + } else if (const auto *TL = State.getNode<TypeLoc>()) { + dumpNode(Ctx, DynTypedNode::create(*TL), OS); + } else if (const auto *NNS = State.getNode<NestedNameSpecifier>()) { + dumpNode(Ctx, DynTypedNode::create(*NNS), OS); + } else if (const auto *NNSL = State.getNode<NestedNameSpecifierLoc>()) { + dumpNode(Ctx, DynTypedNode::create(*NNSL), OS); + } else if (const auto *CtorInit = State.getNode<CXXCtorInitializer>()) { + dumpNode(Ctx, DynTypedNode::create(*CtorInit), OS); + } else if (const auto *TAL = State.getNode<TemplateArgumentLoc>()) { + dumpNode(Ctx, DynTypedNode::create(*TAL), OS); + } else if (const auto *At = State.getNode<Attr>()) { + dumpNode(Ctx, DynTypedNode::create(*At), OS); + } + } + public: TraceReporter(const MatchASTVisitor &MV) : MV(MV) {} void print(raw_ostream &OS) const override { - if (!MV.CurMatched) { + const CurMatchData &State = MV.CurMatchState; + const MatchCallback *CB = State.getCallback(); + if (!CB) { 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()); + ASTContext &Ctx = MV.getASTContext(); + + if (const BoundNodes *Nodes = State.getBoundNodes()) { + OS << "ASTMatcher: Processing '" << CB->getID() << "' against:\n\t"; + dumpNodeFromState(Ctx, State, OS); + const BoundNodes::IDToNodeMap &Map = Nodes->getMap(); + if (Map.empty()) { + OS << "\nNo bound nodes\n"; + return; } - OS << " }\n"; + OS << "\n--- Bound Nodes Begin ---\n"; + for (const auto &Item : Map) { + OS << " " << Item.first << " - { "; + dumpNode(Ctx, Item.second, OS); + OS << " }\n"; + } + OS << "--- Bound Nodes End ---\n"; + } else { + OS << "ASTMatcher: Matching '" << CB->getID() << "' against:\n\t"; + dumpNodeFromState(Ctx, State, OS); + OS << '\n'; } - OS << "--- Bound Nodes End ---\n"; } private: @@ -815,13 +951,6 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, }; 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) { @@ -887,6 +1016,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, if (EnableCheckProfiling) Timer.setBucket(&TimeByBucket[MP.second->getID()]); BoundNodesTreeBuilder Builder; + CurMatchRAII RAII(*this, MP.second, Node); if (MP.first.matches(Node, this, &Builder)) { MatchVisitor Visitor(*this, ActiveASTContext, MP.second); Builder.visitMatches(&Visitor); @@ -919,6 +1049,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, continue; } + CurMatchRAII RAII(*this, MP.second, DynNode); if (MP.first.matches(DynNode, this, &Builder)) { MatchVisitor Visitor(*this, ActiveASTContext, MP.second); Builder.visitMatches(&Visitor); @@ -1107,35 +1238,30 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, // 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(MatchASTVisitor::CurMatchData &State, const BoundNodes &BN) + : State(State) { + State.SetBoundNodes(BN); } - ~CurBoundScope() { MV.CurBoundNodes = nullptr; } + ~CurBoundScope() { State.clearBoundNodes(); } private: - MatchASTVisitor &MV; + MatchASTVisitor::CurMatchData &State; }; 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; } + : State(MV.CurMatchState), Context(Context), Callback(Callback) {} void visitMatch(const BoundNodes& BoundNodesView) override { TraversalKindScope RAII(*Context, Callback->getCheckTraversalKind()); - CurBoundScope RAII2(MV, BoundNodesView); + CurBoundScope RAII2(State, BoundNodesView); Callback->run(MatchFinder::MatchResult(BoundNodesView, Context)); } private: - MatchASTVisitor &MV; + MatchASTVisitor::CurMatchData &State; ASTContext* Context; MatchFinder::MatchCallback* Callback; }; diff --git a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp index b86917c36dcb1..6573461c9fc1e 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -39,10 +39,24 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) { // FIXME: Figure out why back traces aren't being generated on clang builds on // windows. #if ENABLE_BACKTRACES && (!defined(_MSC_VER) || !defined(__clang__)) + +AST_MATCHER(Decl, causeCrash) { + abort(); + return true; +} + +TEST(MatcherCrashDeathTest, CrashOnMatcherDump) { + llvm::EnablePrettyStackTrace(); + auto Matcher = testing::HasSubstr( + "ASTMatcher: Matching '<unknown>' against:\n\tFunctionDecl foo : " + "<input.cc:1:1, col:10>"); + ASSERT_DEATH(matches("void foo();", functionDecl(causeCrash())), Matcher); +} + template <typename MatcherT> static void crashTestNodeDump(MatcherT Matcher, ArrayRef<StringRef> MatchedNodes, - StringRef Code) { + StringRef Against, StringRef Code) { llvm::EnablePrettyStackTrace(); MatchFinder Finder; @@ -58,7 +72,9 @@ static void crashTestNodeDump(MatcherT Matcher, ASSERT_DEATH(tooling::runToolOnCode( newFrontendActionFactory(&Finder)->create(), Code), testing::HasSubstr( - "ASTMatcher: Processing 'CrashTester'\nNo bound nodes")); + ("ASTMatcher: Processing 'CrashTester' against:\n\t" + + Against + "\nNo bound nodes") + .str())); } else { std::vector<testing::PolymorphicMatcher< testing::internal::HasSubstrMatcher<std::string>>> @@ -69,7 +85,9 @@ static void crashTestNodeDump(MatcherT Matcher, } auto CrashMatcher = testing::AllOf( testing::HasSubstr( - "ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"), + ("ASTMatcher: Processing 'CrashTester' against:\n\t" + Against + + "\n--- Bound Nodes Begin ---") + .str()), testing::HasSubstr("--- Bound Nodes End ---"), testing::AllOfArray(Matchers)); @@ -79,7 +97,8 @@ static void crashTestNodeDump(MatcherT Matcher, } } TEST(MatcherCrashDeathTest, CrashOnCallbackDump) { - crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }"); + crashTestNodeDump(forStmt(), {}, "ForStmt : <input.cc:1:14, col:21>", + "void foo() { for(;;); }"); crashTestNodeDump( forStmt(hasLoopInit(declStmt(hasSingleDecl( varDecl(hasType(qualType().bind("QT")), @@ -94,6 +113,7 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) { "IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }", "T - { BuiltinType : int }", "VD - { VarDecl I : <input.cc:3:10, col:18> }"}, + "ForStmt : <input.cc:3:5, line:4:5>", R"cpp( void foo() { for (int I = 0; I < 5; ++I) { @@ -106,12 +126,14 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) { {"Unnamed - { CXXRecordDecl (anonymous) : <input.cc:1:1, col:36> }", "Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, " "col:29> }"}, + "CXXRecordDecl (anonymous) : <input.cc:1:1, col:36>", "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> }"}, + "CXXRecordDecl Foo : <input.cc:1:1, col:49>", "struct Foo { Foo() = default; ~Foo() = default; };"); } #endif // ENABLE_BACKTRACES _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
