[clang] d89f9e9 - [ASTMatchers] Output currently processing match and nodes on crash
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 21 12:13:48 PDT 2022
Author: Nathan James
Date: 2022-03-21T19:13:36Z
New Revision: d89f9e963e4979466193dc6a15fe091bf7ca5c47
URL: https://github.com/llvm/llvm-project/commit/d89f9e963e4979466193dc6a15fe091bf7ca5c47
DIFF: https://github.com/llvm/llvm-project/commit/d89f9e963e4979466193dc6a15fe091bf7ca5c47.diff
LOG: [ASTMatchers] Output currently processing match and nodes on crash
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 which has a nicer integration and should be useful to clients other than clang-tidy.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D120185
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 551e36dea0937..ca4aea676eda3 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..728ba77077f8a 100644
--- 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 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) {
EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
}, "");
}
+
+#if ENABLE_BACKTRACES
+
+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 {
+ LLVM_BUILTIN_TRAP;
+ }
+ 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) {
More information about the cfe-commits
mailing list