[AST] Fix potential nullptr dereference in Expr::HasSideEffects

[jkorous]

Summary

Inspired by: https://reviews.llvm.org/D82629

[ckandeler]

I can confirm that the test case crashes without the null pointer check and passes with the check.

[riccibruno]

I think that adding an unittest for such a simple fix is a bit heavy handed. What about just exercising this code path. For example:

void Test(int N) {
  int arr[N];
  decltype([&arr]{}) *p; // expected-error {{lambda expression in an unevaluated operand}}
}

[jkorous]

Sounds like a good idea.

clang/include/clang/AST/ExprCXX.h

public:
   capture_iterator implicit_capture_end() const;
 
   /// Iterator that walks over the capture initialization
   /// arguments.
   using capture_init_iterator = Expr **;
 
   /// Const iterator that walks over the capture initialization
   /// arguments.
+  /// FIXME: This interface is prone to being used incorrectly.
   using const_capture_init_iterator = Expr *const *;
 
   /// Retrieve the initialization expressions for this lambda's captures.
   llvm::iterator_range<capture_init_iterator> capture_inits() {
     return llvm::make_range(capture_init_begin(), capture_init_end());
   }
 
   /// Retrieve the initialization expressions for this lambda's captures.

clang/lib/AST/Expr.cpp

case CXXInheritedCtorInitExprClass: {
     if (!ICIE->getConstructor()->isTrivial() && IncludePossibleEffects)
       return true;
     break;
   }
 
   case LambdaExprClass: {
     const LambdaExpr *LE = cast<LambdaExpr>(this);
     for (Expr *E : LE->capture_inits())
-      if (E->HasSideEffects(Ctx, IncludePossibleEffects))
+      if (E && E->HasSideEffects(Ctx, IncludePossibleEffects))
         return true;
     return false;
   }
 
   case PseudoObjectExprClass: {
     // Only look for side-effects in the semantic form, and look past
     // OpaqueValueExpr bindings in that form.
     const PseudoObjectExpr *PO = cast<PseudoObjectExpr>(this);

clang/unittests/AST/CMakeLists.txt

add_clang_unittest(ASTTests
   CommentLexer.cpp
   CommentParser.cpp
   CommentTextTest.cpp
   DataCollectionTest.cpp
   DeclPrinterTest.cpp
   DeclTest.cpp
   EvaluateAsRValueTest.cpp
   ExternalASTSourceTest.cpp
+  HasSideEffectsTest.cpp
   NamedDeclPrinterTest.cpp
   RecursiveASTVisitorTest.cpp
   SizelessTypesTest.cpp
   SourceLocationTest.cpp
   StmtPrinterTest.cpp
   StructuralEquivalenceTest.cpp
   )
 

clang/unittests/AST/HasSideEffectsTest.cpp

+//===- unittest/AST/HasSideEffectsTest.cpp --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <cassert>
+
+using namespace clang;
+
+namespace {
+class ProcessASTAction : public clang::ASTFrontendAction {
+public:
+  ProcessASTAction(llvm::unique_function<void(clang::ASTContext &)> Process)
+      : Process(std::move(Process)) {
+    assert(this->Process);
+  }
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef InFile) {
+    class Consumer : public ASTConsumer {
+    public:
+      Consumer(llvm::function_ref<void(ASTContext &CTx)> Process)
+          : Process(Process) {}
+
+      void HandleTranslationUnit(ASTContext &Ctx) override { Process(Ctx); }
+
+    private:
+      llvm::function_ref<void(ASTContext &CTx)> Process;
+    };
+
+    return std::make_unique<Consumer>(Process);
+  }
+
+private:
+  llvm::unique_function<void(clang::ASTContext &)> Process;
+};
+
+class RunHasSideEffects
+    : public RecursiveASTVisitor<RunHasSideEffects> {
+public:
+  RunHasSideEffects(ASTContext& Ctx)
+  : Ctx(Ctx) {}
+
+  bool VisitLambdaExpr(LambdaExpr *LE) {
+    LE->HasSideEffects(Ctx);
+    return true;
+  }
+
+  ASTContext& Ctx;
+};
+} // namespace
+
+TEST(HasSideEffectsTest, All) {
+  llvm::StringRef Code = R"cpp(
+void Test() {
+  int msize = 4;
+  float arr[msize];
+  [&arr] {};
+}
+  )cpp";
+
+  ASSERT_NO_FATAL_FAILURE(
+    clang::tooling::runToolOnCode(
+      std::make_unique<ProcessASTAction>(
+          [&](clang::ASTContext &Ctx) {
+              RunHasSideEffects Visitor(Ctx);
+              Visitor.TraverseAST(Ctx);
+          }
+      ),
+      Code)
+  );
+
+}