[clang] [clang] Fix AST IgnoreUnlessSpelledInSource traversal nullptr dereference (PR #146103)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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)

2025-06-27 Thread Jonathan Marriott via cfe-commits

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