[clang] [clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
https://github.com/JonathanMarriott updated https://github.com/llvm/llvm-project/pull/146103 >From 646d006e327f9d891dea5d9c334e44174a5560a3 Mon Sep 17 00:00:00 2001 From: Jon Marriott Date: Wed, 25 Jun 2025 18:04:59 +0100 Subject: [PATCH 1/3] Add check for decl nullptr to `ASTNodeTraverser::Visit` --- clang/include/clang/AST/ASTNodeTraverser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h index 8d02a50e2e8a5..8ebabb2bde10d 100644 --- a/clang/include/clang/AST/ASTNodeTraverser.h +++ b/clang/include/clang/AST/ASTNodeTraverser.h @@ -99,7 +99,7 @@ class ASTNodeTraverser TraversalKind GetTraversalKind() const { return Traversal; } void Visit(const Decl *D, bool VisitLocs = false) { -if (Traversal == TK_IgnoreUnlessSpelledInSource && D->isImplicit()) +if (Traversal == TK_IgnoreUnlessSpelledInSource && D && D->isImplicit()) return; getNodeDelegate().AddChild([=] { >From 5c9c8b8d5a49fd7c10cc08554e36ac4744f0ee4f Mon Sep 17 00:00:00 2001 From: Jon Marriott Date: Thu, 26 Jun 2025 17:14:57 +0100 Subject: [PATCH 2/3] Add test --- clang/unittests/AST/ASTTraverserTest.cpp | 75 1 file changed, 75 insertions(+) diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp index 8b6e3e90c0ea6..988e81d8e51de 100644 --- a/clang/unittests/AST/ASTTraverserTest.cpp +++ b/clang/unittests/AST/ASTTraverserTest.cpp @@ -28,6 +28,10 @@ class NodeTreePrinter : public TextTreeStructure { : TextTreeStructure(OS, /* showColors */ false), OS(OS) {} void Visit(const Decl *D) { +if (!D) { + OS << "<<>>"; + return; +} OS << D->getDeclKindName() << "Decl"; if (auto *ND = dyn_cast(D)) { OS << " '" << ND->getDeclName() << "'"; @@ -1932,4 +1936,75 @@ CXXRewrittenBinaryOperator } } +TEST(Traverse, CatchStatements) { + + auto AST = buildASTFromCode(R"cpp( +void test() +{ + try + { +int a; + } + catch (...) + { +int b; + } + + try + { +int a; + } + catch (const int&) + { +int b; + } +} +)cpp"); + + auto BN = + ast_matchers::match(cxxCatchStmt().bind("catch"), AST->getASTContext()); + EXPECT_EQ(BN.size(), 2u); + const auto *catchWithoutDecl = BN[0].getNodeAs("catch"); + + llvm::StringRef Expected = R"cpp( +CXXCatchStmt +|-<<>> +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_AsIs, catchWithoutDecl), Expected); + + Expected = R"cpp( +CXXCatchStmt +|-<<>> +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, catchWithoutDecl), +Expected); + + const auto *catchWithDecl = BN[1].getNodeAs("catch"); + + Expected = R"cpp( +CXXCatchStmt +|-VarDecl '' +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_AsIs, catchWithDecl), Expected); + + Expected = R"cpp( +CXXCatchStmt +|-VarDecl '' +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, catchWithDecl), +Expected); +} + } // namespace clang >From 576beabbfeb8e6fa3afd78f8fbaf1458090308a6 Mon Sep 17 00:00:00 2001 From: Jon Marriott Date: Fri, 27 Jun 2025 19:20:07 +0100 Subject: [PATCH 3/3] Add release note --- clang/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d9847fadc21e5..fa938167d23a7 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -893,6 +893,7 @@ Bug Fixes to AST Handling - Fixed a malformed printout of certain calling convention function attributes. (#GH143160) - Fixed dependency calculation for TypedefTypes (#GH89774) - Fixed the right parenthesis source location of ``CXXTemporaryObjectExpr``. (#GH143711) +- Fixed a crash when performing an ``IgnoreUnlessSpelledInSource`` traversal of ASTs containing ``catch(...)`` statements. (#GH146103) Miscellaneous Bug Fixes ^^^ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
JonathanMarriott wrote: > Thank you for the patch! Please add a release note to > "clang/docs/ReleaseNotes.rst" so users can know the improvement. Please add a > tag to the beginning of the PR title in square brackets like other PRs. E.g. > `[clang]`. Thanks, I've added a release note in what I think is the appropriate section! https://github.com/llvm/llvm-project/pull/146103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
https://github.com/JonathanMarriott created https://github.com/llvm/llvm-project/pull/146103 Fixes #146101, in summary dumping a `catch(...)` statement using IgnoreUnlessSpelledInSource AST traversal causes a seg fault, as the variable declaration of the catch is `nullptr`. Diagnosed the cause by attaching the debugger to `clang-query`, this PR adds a fix to check for `nullptr` before accessing the `isImplicit()` method of the `Decl` pointee in the AST node traverser visitor >From 646d006e327f9d891dea5d9c334e44174a5560a3 Mon Sep 17 00:00:00 2001 From: Jon Marriott Date: Wed, 25 Jun 2025 18:04:59 +0100 Subject: [PATCH 1/2] Add check for decl nullptr to `ASTNodeTraverser::Visit` --- clang/include/clang/AST/ASTNodeTraverser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h index 8d02a50e2e8a5..8ebabb2bde10d 100644 --- a/clang/include/clang/AST/ASTNodeTraverser.h +++ b/clang/include/clang/AST/ASTNodeTraverser.h @@ -99,7 +99,7 @@ class ASTNodeTraverser TraversalKind GetTraversalKind() const { return Traversal; } void Visit(const Decl *D, bool VisitLocs = false) { -if (Traversal == TK_IgnoreUnlessSpelledInSource && D->isImplicit()) +if (Traversal == TK_IgnoreUnlessSpelledInSource && D && D->isImplicit()) return; getNodeDelegate().AddChild([=] { >From 5c9c8b8d5a49fd7c10cc08554e36ac4744f0ee4f Mon Sep 17 00:00:00 2001 From: Jon Marriott Date: Thu, 26 Jun 2025 17:14:57 +0100 Subject: [PATCH 2/2] Add test --- clang/unittests/AST/ASTTraverserTest.cpp | 75 1 file changed, 75 insertions(+) diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp index 8b6e3e90c0ea6..988e81d8e51de 100644 --- a/clang/unittests/AST/ASTTraverserTest.cpp +++ b/clang/unittests/AST/ASTTraverserTest.cpp @@ -28,6 +28,10 @@ class NodeTreePrinter : public TextTreeStructure { : TextTreeStructure(OS, /* showColors */ false), OS(OS) {} void Visit(const Decl *D) { +if (!D) { + OS << "<<>>"; + return; +} OS << D->getDeclKindName() << "Decl"; if (auto *ND = dyn_cast(D)) { OS << " '" << ND->getDeclName() << "'"; @@ -1932,4 +1936,75 @@ CXXRewrittenBinaryOperator } } +TEST(Traverse, CatchStatements) { + + auto AST = buildASTFromCode(R"cpp( +void test() +{ + try + { +int a; + } + catch (...) + { +int b; + } + + try + { +int a; + } + catch (const int&) + { +int b; + } +} +)cpp"); + + auto BN = + ast_matchers::match(cxxCatchStmt().bind("catch"), AST->getASTContext()); + EXPECT_EQ(BN.size(), 2u); + const auto *catchWithoutDecl = BN[0].getNodeAs("catch"); + + llvm::StringRef Expected = R"cpp( +CXXCatchStmt +|-<<>> +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_AsIs, catchWithoutDecl), Expected); + + Expected = R"cpp( +CXXCatchStmt +|-<<>> +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, catchWithoutDecl), +Expected); + + const auto *catchWithDecl = BN[1].getNodeAs("catch"); + + Expected = R"cpp( +CXXCatchStmt +|-VarDecl '' +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_AsIs, catchWithDecl), Expected); + + Expected = R"cpp( +CXXCatchStmt +|-VarDecl '' +`-CompoundStmt + `-DeclStmt +`-VarDecl 'b' +)cpp"; + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, catchWithDecl), +Expected); +} + } // namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
https://github.com/JonathanMarriott ready_for_review https://github.com/llvm/llvm-project/pull/146103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
JonathanMarriott wrote: CC: @AaronBallman @Sirraide https://github.com/llvm/llvm-project/pull/146103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
https://github.com/JonathanMarriott edited https://github.com/llvm/llvm-project/pull/146103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)
https://github.com/JonathanMarriott edited https://github.com/llvm/llvm-project/pull/146103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits