[clang] f8cedc6 - [clang] Never wrap a nullptr in CXXNewExpr::getArraySize()

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-02-22T16:27:32+01:00
New Revision: f8cedc642d9b85720cb7175ef25ddde90a3fbca2

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

LOG: [clang] Never wrap a nullptr in CXXNewExpr::getArraySize()

Otherwise callers of these functions have to check both the return value
for and the contents of the returned llvm::Optional.

Fixes #53742

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

Added: 
    clang/test/AST/issue53742.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/ExprCXX.h
    clang/lib/AST/ExprConstant.cpp
    clang/lib/AST/StmtPrinter.cpp
    clang/lib/Sema/TreeTransform.h

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 499b065fe6e07..8de1e65a83dc4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -54,6 +54,14 @@ Major New Features
   There is an analogous ``zero_call_used_regs`` attribute to allow for finer
   control of this feature.
 
+Bug Fixes
+------------------
+- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
+  wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array
+  size expression. This was fixed and ``::getArraySize()`` will now always
+  either return ``None`` or a ``llvm::Optional`` wrapping a valid ``Expr*``.
+  This fixes `Issue #53742<https://github.com/llvm/llvm-project/issues/53742>`_.
+
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -83,7 +91,8 @@ Attribute Changes in Clang
 - Added support for parameter pack expansion in `clang::annotate`.
 
 - The ``overloadable`` attribute can now be written in all of the syntactic
-  locations a declaration attribute may appear. Fixes PR53805.
+  locations a declaration attribute may appear.
+  This fixes `Issue #53805<https://github.com/llvm/llvm-project/issues/53805>`_.
 
 Windows Support
 ---------------

diff  --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 161287adce4ca..3da9290c7dfbe 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2261,15 +2261,32 @@ class CXXNewExpr final
 
   bool isArray() const { return CXXNewExprBits.IsArray; }
 
+  /// This might return None even if isArray() returns true,
+  /// since there might not be an array size expression.
+  /// If the result is not-None, it will never wrap a nullptr.
   Optional<Expr *> getArraySize() {
     if (!isArray())
       return None;
-    return cast_or_null<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()]);
+
+    if (auto *Result =
+            cast_or_null<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()]))
+      return Result;
+
+    return None;
   }
+
+  /// This might return None even if isArray() returns true,
+  /// since there might not be an array size expression.
+  /// If the result is not-None, it will never wrap a nullptr.
   Optional<const Expr *> getArraySize() const {
     if (!isArray())
       return None;
-    return cast_or_null<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()]);
+
+    if (auto *Result =
+            cast_or_null<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()]))
+      return Result;
+
+    return None;
   }
 
   unsigned getNumPlacementArgs() const {

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 163109bf7c9f5..99f136a72d6fe 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -9427,7 +9427,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
   bool ValueInit = false;
 
   QualType AllocType = E->getAllocatedType();
-  if (Optional<const Expr*> ArraySize = E->getArraySize()) {
+  if (Optional<const Expr *> ArraySize = E->getArraySize()) {
     const Expr *Stripped = *ArraySize;
     for (; auto *ICE = dyn_cast<ImplicitCastExpr>(Stripped);
          Stripped = ICE->getSubExpr())

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 746bf8c21cd72..5ad935591ecd9 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2132,10 +2132,10 @@ void StmtPrinter::VisitCXXNewExpr(CXXNewExpr *E) {
   if (E->isParenTypeId())
     OS << "(";
   std::string TypeS;
-  if (Optional<Expr *> Size = E->getArraySize()) {
+  if (E->isArray()) {
     llvm::raw_string_ostream s(TypeS);
     s << '[';
-    if (*Size)
+    if (Optional<Expr *> Size = E->getArraySize())
       (*Size)->printPretty(s, Helper, Policy);
     s << ']';
   }

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 466a156add516..0716689d4b626 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -11912,9 +11912,9 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
 
   // Transform the size of the array we're allocating (if any).
   Optional<Expr *> ArraySize;
-  if (Optional<Expr *> OldArraySize = E->getArraySize()) {
+  if (E->isArray()) {
     ExprResult NewArraySize;
-    if (*OldArraySize) {
+    if (Optional<Expr *> OldArraySize = E->getArraySize()) {
       NewArraySize = getDerived().TransformExpr(*OldArraySize);
       if (NewArraySize.isInvalid())
         return ExprError();

diff  --git a/clang/test/AST/issue53742.cpp b/clang/test/AST/issue53742.cpp
new file mode 100644
index 0000000000000..93978f2bcc11d
--- /dev/null
+++ b/clang/test/AST/issue53742.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+struct Data {
+  char *a;
+  char *b;
+  bool *c;
+};
+
+int main() {
+  Data in;
+  in.a = new char[](); // expected-error {{cannot determine allocated array size from initializer}}
+  in.c = new bool[100]();
+  in.b = new char[100]();
+}