[clang] [clang-tools-extra] [clang][AST] Fix crash in MatchChildASTVisitor::TraverseLambdaExpr (PR #70962)

[Piotr Zegar via cfe-commits]

https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/70962

When a lambda expression captures a VLA array by reference, the 'capture_init' array contains one element, which is 'nullptr'. While traversing the AST with the 'IgnoreUnlessSpelledInSource' flag, there is a dereference of this 'nullptr'.

This change introduces a verification step to check if 'capture_init' is 'nullptr' before attempting to dereference it. Additionally, it includes tests for matchers and for the clang-tidy check that originally revealed the issue.

>From 3e37e7e4afe806993acbba3d7663fdbfe80120c7 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Wed, 1 Nov 2023 17:01:25 +0000
Subject: [PATCH] [clang][AST] Fix crash in
 MatchChildASTVisitor::TraverseLambdaExpr

When a lambda expression captures a VLA array by reference,
the 'capture_init' array contains one element, which is 'nullptr'.
While traversing the AST with the 'IgnoreUnlessSpelledInSource'
flag, there is a dereference of this 'nullptr'.

This change introduces a verification step to check if
'capture_init' is 'nullptr' before attempting to dereference it.
Additionally, it includes tests for matchers and for the
clang-tidy check that originally revealed the issue.
---
 .../checkers/cppcoreguidelines/owning-memory.cpp    | 10 ++++++++++
 clang/lib/ASTMatchers/ASTMatchFinder.cpp            |  3 ++-
 .../ASTMatchers/ASTMatchersTraversalTest.cpp        | 13 +++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
index eb8ad1b8b879250..7ab5ecb9cfe3e2b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
@@ -395,3 +395,13 @@ namespace PR63994 {
     // CHECK-NOTES: [[@LINE-1]]:5: warning: returning a newly created resource of type 'A *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
   }
 }
+
+namespace PR70460 {
+  template<typename T>
+  void h(T&& x) { x(); }
+
+  void f(int N) {
+    int a[N];
+    h([&a]() {});
+  }
+}
diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 0bac2ed63a927ef..08739b7079c1a82 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -290,7 +290,8 @@ class MatchChildASTVisitor
         continue;
       if (Node->isInitCapture(C) && !match(*C->getCapturedVar()))
         return false;
-      if (!match(*Node->capture_init_begin()[I]))
+      const Expr *CI = Node->capture_init_begin()[I];
+      if (CI && !match(*CI))
         return false;
     }
 
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index d4a695b974bf0e5..87fdc3da480f076 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -606,6 +606,19 @@ void foo()
                                   varDecl(hasName("lPtrDecay"))))))));
 }
 
+TEST(Matcher, NotCrashesOnLambdaThatCapturesVla) {
+  StringRef Code = R"cpp(
+void f(int N) {
+    int a[N];
+    [&a]() {};
+}
+)cpp";
+  EXPECT_FALSE(
+      matches(Code, traverse(TK_AsIs, lambdaExpr(hasDescendant(expr())))));
+  EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+                                      lambdaExpr(hasDescendant(expr())))));
+}
+
 TEST(Matcher, MatchesCoroutine) {
   FileContentMappings M;
   M.push_back(std::make_pair("/coro_header", R"cpp(