Skip to content

Commit b4ad3c3

Browse files
njames93njames93
committed
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 61d67c8. This relands commit 6e33e45. Fixing the build issue on 32bit machines due to not enough free bits in the PointerUnion.
1 parent f2ea125 commit b4ad3c3

File tree

2 files changed

+207
-59
lines changed

2 files changed

+207
-59
lines changed

‎clang/lib/ASTMatchers/ASTMatchFinder.cpp

Lines changed: 181 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -761,67 +761,196 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
761761
D);
762762
}
763763

764+
private:
765+
bool TraversingASTNodeNotSpelledInSource = false;
766+
bool TraversingASTNodeNotAsIs = false;
767+
bool TraversingASTChildrenNotSpelledInSource = false;
768+
769+
class CurMatchData {
770+
// We don't have enough free low bits in 32bit builds to discriminate 8 pointer
771+
// types in PointerUnion. so split the union in 2 using a free bit from the
772+
// callback pointer.
773+
#define CMD_TYPES_0 \
774+
const QualType *, const TypeLoc *, const NestedNameSpecifier *, \
775+
const NestedNameSpecifierLoc *
776+
#define CMD_TYPES_1 \
777+
const CXXCtorInitializer *, const TemplateArgumentLoc *, const Attr *, \
778+
const DynTypedNode *
779+
780+
#define IMPL(Index) \
781+
template <typename NodeType> \
782+
typename std::enable_if_t< \
783+
llvm::is_one_of<const NodeType *, CMD_TYPES_##Index>::value> \
784+
SetCallbackAndRawNode(const MatchCallback *CB, const NodeType &N) { \
785+
assertEmpty(); \
786+
Callback.setPointerAndInt(CB, Index); \
787+
Node##Index = &N; \
788+
} \
789+
\
790+
template <typename T> \
791+
typename std::enable_if_t< \
792+
llvm::is_one_of<const T *, CMD_TYPES_##Index>::value, const T *> \
793+
getNode() const { \
794+
assertHoldsState(); \
795+
return Callback.getInt() == (Index) ? Node##Index.dyn_cast<const T *>() \
796+
: nullptr; \
797+
}
798+
799+
public:
800+
CurMatchData() : Node0(nullptr) {}
801+
802+
IMPL(0)
803+
IMPL(1)
804+
805+
const MatchCallback *getCallback() const { return Callback.getPointer(); }
806+
807+
void SetBoundNodes(const BoundNodes &BN) {
808+
assertHoldsState();
809+
BNodes = &BN;
810+
}
811+
812+
void clearBoundNodes() {
813+
assertHoldsState();
814+
BNodes = nullptr;
815+
}
816+
817+
const BoundNodes *getBoundNodes() const {
818+
assertHoldsState();
819+
return BNodes;
820+
}
821+
822+
void reset() {
823+
assertHoldsState();
824+
Callback.setPointerAndInt(nullptr, 0);
825+
Node0 = nullptr;
826+
}
827+
828+
private:
829+
void assertHoldsState() const {
830+
assert(Callback.getPointer() != nullptr && !Node0.isNull());
831+
}
832+
833+
void assertEmpty() const {
834+
assert(Callback.getPointer() == nullptr && Node0.isNull() &&
835+
BNodes == nullptr);
836+
}
837+
838+
llvm::PointerIntPair<const MatchCallback *, 1> Callback;
839+
union {
840+
llvm::PointerUnion<CMD_TYPES_0> Node0;
841+
llvm::PointerUnion<CMD_TYPES_1> Node1;
842+
};
843+
const BoundNodes *BNodes = nullptr;
844+
845+
#undef CMD_TYPES_0
846+
#undef CMD_TYPES_1
847+
#undef IMPL
848+
} CurMatchState;
849+
850+
struct CurMatchRAII {
851+
template <typename NodeType>
852+
CurMatchRAII(MatchASTVisitor &MV, const MatchCallback *CB,
853+
const NodeType &NT)
854+
: MV(MV) {
855+
MV.CurMatchState.SetCallbackAndRawNode(CB, NT);
856+
}
857+
858+
~CurMatchRAII() { MV.CurMatchState.reset(); }
859+
860+
private:
861+
MatchASTVisitor &MV;
862+
};
863+
864+
public:
764865
class TraceReporter : llvm::PrettyStackTraceEntry {
866+
static void dumpNode(const ASTContext &Ctx, const DynTypedNode &Node,
867+
raw_ostream &OS) {
868+
if (const auto *D = Node.get<Decl>()) {
869+
OS << D->getDeclKindName() << "Decl ";
870+
if (const auto *ND = dyn_cast<NamedDecl>(D)) {
871+
ND->printQualifiedName(OS);
872+
OS << " : ";
873+
} else
874+
OS << ": ";
875+
D->getSourceRange().print(OS, Ctx.getSourceManager());
876+
} else if (const auto *S = Node.get<Stmt>()) {
877+
OS << S->getStmtClassName() << " : ";
878+
S->getSourceRange().print(OS, Ctx.getSourceManager());
879+
} else if (const auto *T = Node.get<Type>()) {
880+
OS << T->getTypeClassName() << "Type : ";
881+
QualType(T, 0).print(OS, Ctx.getPrintingPolicy());
882+
} else if (const auto *QT = Node.get<QualType>()) {
883+
OS << "QualType : ";
884+
QT->print(OS, Ctx.getPrintingPolicy());
885+
} else {
886+
OS << Node.getNodeKind().asStringRef() << " : ";
887+
Node.getSourceRange().print(OS, Ctx.getSourceManager());
888+
}
889+
}
890+
891+
static void dumpNodeFromState(const ASTContext &Ctx,
892+
const CurMatchData &State, raw_ostream &OS) {
893+
if (const DynTypedNode *MatchNode = State.getNode<DynTypedNode>()) {
894+
dumpNode(Ctx, *MatchNode, OS);
895+
} else if (const auto *QT = State.getNode<QualType>()) {
896+
dumpNode(Ctx, DynTypedNode::create(*QT), OS);
897+
} else if (const auto *TL = State.getNode<TypeLoc>()) {
898+
dumpNode(Ctx, DynTypedNode::create(*TL), OS);
899+
} else if (const auto *NNS = State.getNode<NestedNameSpecifier>()) {
900+
dumpNode(Ctx, DynTypedNode::create(*NNS), OS);
901+
} else if (const auto *NNSL = State.getNode<NestedNameSpecifierLoc>()) {
902+
dumpNode(Ctx, DynTypedNode::create(*NNSL), OS);
903+
} else if (const auto *CtorInit = State.getNode<CXXCtorInitializer>()) {
904+
dumpNode(Ctx, DynTypedNode::create(*CtorInit), OS);
905+
} else if (const auto *TAL = State.getNode<TemplateArgumentLoc>()) {
906+
dumpNode(Ctx, DynTypedNode::create(*TAL), OS);
907+
} else if (const auto *At = State.getNode<Attr>()) {
908+
dumpNode(Ctx, DynTypedNode::create(*At), OS);
909+
}
910+
}
911+
765912
public:
766913
TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
767914
void print(raw_ostream &OS) const override {
768-
if (!MV.CurMatched) {
915+
const CurMatchData &State = MV.CurMatchState;
916+
const MatchCallback *CB = State.getCallback();
917+
if (!CB) {
769918
OS << "ASTMatcher: Not currently matching\n";
770919
return;
771920
}
921+
772922
assert(MV.ActiveASTContext &&
773923
"ActiveASTContext should be set if there is a matched callback");
774924

775-
OS << "ASTMatcher: Processing '" << MV.CurMatched->getID() << "'\n";
776-
const BoundNodes::IDToNodeMap &Map = MV.CurBoundNodes->getMap();
777-
if (Map.empty()) {
778-
OS << "No bound nodes\n";
779-
return;
780-
}
781-
OS << "--- Bound Nodes Begin ---\n";
782-
for (const auto &Item : Map) {
783-
OS << " " << Item.first << " - { ";
784-
if (const auto *D = Item.second.get<Decl>()) {
785-
OS << D->getDeclKindName() << "Decl ";
786-
if (const auto *ND = dyn_cast<NamedDecl>(D)) {
787-
ND->printQualifiedName(OS);
788-
OS << " : ";
789-
} else
790-
OS << ": ";
791-
D->getSourceRange().print(OS,
792-
MV.ActiveASTContext->getSourceManager());
793-
} else if (const auto *S = Item.second.get<Stmt>()) {
794-
OS << S->getStmtClassName() << " : ";
795-
S->getSourceRange().print(OS,
796-
MV.ActiveASTContext->getSourceManager());
797-
} else if (const auto *T = Item.second.get<Type>()) {
798-
OS << T->getTypeClassName() << "Type : ";
799-
QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
800-
} else if (const auto *QT = Item.second.get<QualType>()) {
801-
OS << "QualType : ";
802-
QT->print(OS, MV.ActiveASTContext->getPrintingPolicy());
803-
} else {
804-
OS << Item.second.getNodeKind().asStringRef() << " : ";
805-
Item.second.getSourceRange().print(
806-
OS, MV.ActiveASTContext->getSourceManager());
925+
ASTContext &Ctx = MV.getASTContext();
926+
927+
if (const BoundNodes *Nodes = State.getBoundNodes()) {
928+
OS << "ASTMatcher: Processing '" << CB->getID() << "' against:\n\t";
929+
dumpNodeFromState(Ctx, State, OS);
930+
const BoundNodes::IDToNodeMap &Map = Nodes->getMap();
931+
if (Map.empty()) {
932+
OS << "\nNo bound nodes\n";
933+
return;
807934
}
808-
OS << " }\n";
935+
OS << "\n--- Bound Nodes Begin ---\n";
936+
for (const auto &Item : Map) {
937+
OS << " " << Item.first << " - { ";
938+
dumpNode(Ctx, Item.second, OS);
939+
OS << " }\n";
940+
}
941+
OS << "--- Bound Nodes End ---\n";
942+
} else {
943+
OS << "ASTMatcher: Matching '" << CB->getID() << "' against:\n\t";
944+
dumpNodeFromState(Ctx, State, OS);
945+
OS << '\n';
809946
}
810-
OS << "--- Bound Nodes End ---\n";
811947
}
812948

813949
private:
814950
const MatchASTVisitor &MV;
815951
};
816952

817953
private:
818-
bool TraversingASTNodeNotSpelledInSource = false;
819-
bool TraversingASTNodeNotAsIs = false;
820-
bool TraversingASTChildrenNotSpelledInSource = false;
821-
822-
const MatchCallback *CurMatched = nullptr;
823-
const BoundNodes *CurBoundNodes = nullptr;
824-
825954
struct ASTNodeNotSpelledInSourceScope {
826955
ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
827956
: MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
@@ -887,6 +1016,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
8871016
if (EnableCheckProfiling)
8881017
Timer.setBucket(&TimeByBucket[MP.second->getID()]);
8891018
BoundNodesTreeBuilder Builder;
1019+
CurMatchRAII RAII(*this, MP.second, Node);
8901020
if (MP.first.matches(Node, this, &Builder)) {
8911021
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
8921022
Builder.visitMatches(&Visitor);
@@ -919,6 +1049,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
9191049
continue;
9201050
}
9211051

1052+
CurMatchRAII RAII(*this, MP.second, DynNode);
9221053
if (MP.first.matches(DynNode, this, &Builder)) {
9231054
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
9241055
Builder.visitMatches(&Visitor);
@@ -1107,35 +1238,30 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
11071238
// the aggregated bound nodes for each match.
11081239
class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
11091240
struct CurBoundScope {
1110-
CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
1111-
assert(MV.CurMatched && !MV.CurBoundNodes);
1112-
MV.CurBoundNodes = &BN;
1241+
CurBoundScope(MatchASTVisitor::CurMatchData &State, const BoundNodes &BN)
1242+
: State(State) {
1243+
State.SetBoundNodes(BN);
11131244
}
11141245

1115-
~CurBoundScope() { MV.CurBoundNodes = nullptr; }
1246+
~CurBoundScope() { State.clearBoundNodes(); }
11161247

11171248
private:
1118-
MatchASTVisitor &MV;
1249+
MatchASTVisitor::CurMatchData &State;
11191250
};
11201251

11211252
public:
11221253
MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
11231254
MatchFinder::MatchCallback *Callback)
1124-
: MV(MV), Context(Context), Callback(Callback) {
1125-
assert(!MV.CurMatched && !MV.CurBoundNodes);
1126-
MV.CurMatched = Callback;
1127-
}
1128-
1129-
~MatchVisitor() { MV.CurMatched = nullptr; }
1255+
: State(MV.CurMatchState), Context(Context), Callback(Callback) {}
11301256

11311257
void visitMatch(const BoundNodes& BoundNodesView) override {
11321258
TraversalKindScope RAII(*Context, Callback->getCheckTraversalKind());
1133-
CurBoundScope RAII2(MV, BoundNodesView);
1259+
CurBoundScope RAII2(State, BoundNodesView);
11341260
Callback->run(MatchFinder::MatchResult(BoundNodesView, Context));
11351261
}
11361262

11371263
private:
1138-
MatchASTVisitor &MV;
1264+
MatchASTVisitor::CurMatchData &State;
11391265
ASTContext* Context;
11401266
MatchFinder::MatchCallback* Callback;
11411267
};

‎clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,24 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) {
3939
// FIXME: Figure out why back traces aren't being generated on clang builds on
4040
// windows.
4141
#if ENABLE_BACKTRACES && (!defined(_MSC_VER) || !defined(__clang__))
42+
43+
AST_MATCHER(Decl, causeCrash) {
44+
abort();
45+
return true;
46+
}
47+
48+
TEST(MatcherCrashDeathTest, CrashOnMatcherDump) {
49+
llvm::EnablePrettyStackTrace();
50+
auto Matcher = testing::HasSubstr(
51+
"ASTMatcher: Matching '<unknown>' against:\n\tFunctionDecl foo : "
52+
"<input.cc:1:1, col:10>");
53+
ASSERT_DEATH(matches("void foo();", functionDecl(causeCrash())), Matcher);
54+
}
55+
4256
template <typename MatcherT>
4357
static void crashTestNodeDump(MatcherT Matcher,
4458
ArrayRef<StringRef> MatchedNodes,
45-
StringRef Code) {
59+
StringRef Against, StringRef Code) {
4660
llvm::EnablePrettyStackTrace();
4761
MatchFinder Finder;
4862

@@ -58,7 +72,9 @@ static void crashTestNodeDump(MatcherT Matcher,
5872
ASSERT_DEATH(tooling::runToolOnCode(
5973
newFrontendActionFactory(&Finder)->create(), Code),
6074
testing::HasSubstr(
61-
"ASTMatcher: Processing 'CrashTester'\nNo bound nodes"));
75+
("ASTMatcher: Processing 'CrashTester' against:\n\t" +
76+
Against + "\nNo bound nodes")
77+
.str()));
6278
} else {
6379
std::vector<testing::PolymorphicMatcher<
6480
testing::internal::HasSubstrMatcher<std::string>>>
@@ -69,7 +85,9 @@ static void crashTestNodeDump(MatcherT Matcher,
6985
}
7086
auto CrashMatcher = testing::AllOf(
7187
testing::HasSubstr(
72-
"ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
88+
("ASTMatcher: Processing 'CrashTester' against:\n\t" + Against +
89+
"\n--- Bound Nodes Begin ---")
90+
.str()),
7391
testing::HasSubstr("--- Bound Nodes End ---"),
7492
testing::AllOfArray(Matchers));
7593

@@ -79,7 +97,8 @@ static void crashTestNodeDump(MatcherT Matcher,
7997
}
8098
}
8199
TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
82-
crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }");
100+
crashTestNodeDump(forStmt(), {}, "ForStmt : <input.cc:1:14, col:21>",
101+
"void foo() { for(;;); }");
83102
crashTestNodeDump(
84103
forStmt(hasLoopInit(declStmt(hasSingleDecl(
85104
varDecl(hasType(qualType().bind("QT")),
@@ -94,6 +113,7 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
94113
"IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }",
95114
"T - { BuiltinType : int }",
96115
"VD - { VarDecl I : <input.cc:3:10, col:18> }"},
116+
"ForStmt : <input.cc:3:5, line:4:5>",
97117
R"cpp(
98118
void foo() {
99119
for (int I = 0; I < 5; ++I) {
@@ -106,12 +126,14 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
106126
{"Unnamed - { CXXRecordDecl (anonymous) : <input.cc:1:1, col:36> }",
107127
"Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, "
108128
"col:29> }"},
129+
"CXXRecordDecl (anonymous) : <input.cc:1:1, col:36>",
109130
"struct { int operator+(int) const; } Unnamed;");
110131
crashTestNodeDump(
111132
cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")),
112133
hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))),
113134
{"Ctor - { CXXConstructorDecl Foo::Foo : <input.cc:1:14, col:28> }",
114135
"Dtor - { CXXDestructorDecl Foo::~Foo : <input.cc:1:31, col:46> }"},
136+
"CXXRecordDecl Foo : <input.cc:1:1, col:49>",
115137
"struct Foo { Foo() = default; ~Foo() = default; };");
116138
}
117139
#endif // ENABLE_BACKTRACES

0 commit comments

Comments
 (0)