[clang] fdb6953 - [AST] Fix potential nullptr dereference in Expr::HasSideEffects

[Jan Korous via cfe-commits]

Author: Jan Korous
Date: 2020-07-13T11:08:51-07:00
New Revision: fdb69539bcd250f6e4f49197c9b8149a7542e3ff

URL: https://github.com/llvm/llvm-project/commit/fdb69539bcd250f6e4f49197c9b8149a7542e3ff
DIFF: https://github.com/llvm/llvm-project/commit/fdb69539bcd250f6e4f49197c9b8149a7542e3ff.diff

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

Array returned by LambdaExpr::capture_inits() can contain nullptrs.

Differential Revision: https://reviews.llvm.org/D83438

Added: 
    clang/unittests/AST/HasSideEffectsTest.cpp

Modified: 
    clang/include/clang/AST/ExprCXX.h
    clang/lib/AST/Expr.cpp
    clang/unittests/AST/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 178f4db77061..6f0b68479b9d 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1931,6 +1931,7 @@ class LambdaExpr final : public 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.

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 343a271c3394..399e7e13c445 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3629,7 +3629,7 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
   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;
   }

diff  --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index 2e750ac9ea92..185995d5b5a2 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -26,6 +26,7 @@ add_clang_unittest(ASTTests
   DeclTest.cpp
   EvaluateAsRValueTest.cpp
   ExternalASTSourceTest.cpp
+  HasSideEffectsTest.cpp
   NamedDeclPrinterTest.cpp
   RecursiveASTVisitorTest.cpp
   SizelessTypesTest.cpp

diff  --git a/clang/unittests/AST/HasSideEffectsTest.cpp b/clang/unittests/AST/HasSideEffectsTest.cpp
new file mode 100644
index 000000000000..842afd8d7a9c
--- /dev/null
+++ b/clang/unittests/AST/HasSideEffectsTest.cpp
@@ -0,0 +1,86 @@
+//===- 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)
+  );
+
+}