njames93 updated this revision to Diff 412772.
njames93 added a comment.
Herald added a project: All.
Moved tests into ASTMatchersInternal, makes sense in here and removes the dirty
clang-tidy plugin hack.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120185/new/
https://reviews.llvm.org/D120185
Files:
clang-tools-extra/docs/ReleaseNotes.rst
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ 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,55 @@
EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
}, "");
}
+
+#if ENABLE_BACKTRACES
+TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
+ 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(
+ forStmt(hasLoopInit(declStmt(hasSingleDecl(
+ varDecl(hasType(qualType().bind("QT")),
+ hasType(type().bind("T")),
+ hasInitializer(
+ integerLiteral().bind("IL")))
+ .bind("VD")))
+ .bind("DS")))
+ .bind("FS"),
+ &Callback);
+
+ auto Matcher = testing::AllOf(
+ testing::ContainsRegex("ASTMatcher: Processing 'CrashTester'"),
+ testing::ContainsRegex("--- Bound Nodes Begin ---"),
+ testing::ContainsRegex("FS - \\{ ForStmt : <input.cc:3:5, line:4:5> \\}"),
+ testing::ContainsRegex("DS - \\{ DeclStmt : <input.cc:3:10, col:19> \\}"),
+ testing::ContainsRegex("IL - \\{ IntegerLiteral : <input.cc:3:18> \\}"),
+ testing::ContainsRegex("QT - \\{ QualType : int \\}"),
+ testing::ContainsRegex("T - \\{ BuiltinType : int \\}"),
+ testing::ContainsRegex(
+ "VD - \\{ VarDecl I : <input.cc:3:10, col:18> \\}"),
+ testing::ContainsRegex("--- Bound Nodes End ---"));
+
+ ASSERT_DEATH(
+ tooling::runToolOnCode(newFrontendActionFactory(&Finder)->create(), R"cpp(
+ void foo() {
+ for (int I = 0; I < 5; ++I) {
+ }
+ }
+ )cpp"),
+ Matcher);
+}
+#endif // ENABLE_BACKTRACES
#endif
TEST(ConstructVariadic, MismatchedTypes_Regression) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ 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,66 @@
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))
+ OS << ND->getDeclName() << " : ";
+ else
+ OS << "<anonymous> ";
+ 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 +887,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 +919,7 @@
}
if (MP.first.matches(DynNode, this, &Builder)) {
- MatchVisitor Visitor(ActiveASTContext, MP.second);
+ MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
Builder.visitMatches(&Visitor);
}
}
@@ -1049,18 +1105,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 +1544,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);
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
