[clang][bytecode] Fix dynamic array allocation return values (llvm#127387)

We need to return a pointer to the first element, not the array itself.

Author: tbaederr

clang/lib/AST/ByteCode/DynamicAllocator.cpp

@@ -54,6 +54,7 @@ Block *DynamicAllocator::allocate(const Expr *Source, PrimType T,
 Block *DynamicAllocator::allocate(const Descriptor *ElementDesc,
                                   size_t NumElements, unsigned EvalID,
                                   Form AllocForm) {
+  assert(ElementDesc->getMetadataSize() == 0);
   // Create a new descriptor for an array of the specified size and
   // element type.
   const Descriptor *D = allocateDescriptor(
@@ -72,6 +73,7 @@ Block *DynamicAllocator::allocate(const Descriptor *D, unsigned EvalID,
   auto *B = new (Memory.get()) Block(EvalID, D, /*isStatic=*/false);
   B->invokeCtor();
 
+  assert(D->getMetadataSize() == sizeof(InlineDescriptor));
   InlineDescriptor *ID = reinterpret_cast<InlineDescriptor *>(B->rawData());
   ID->Desc = D;
   ID->IsActive = true;

clang/lib/AST/ByteCode/Interp.h

@@ -2896,9 +2896,7 @@ inline bool Alloc(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
   Block *B = Allocator.allocate(Desc, S.Ctx.getEvalID(),
                                 DynamicAllocator::Form::NonArray);
   assert(B);
-
   S.Stk.push<Pointer>(B);
-
   return true;
 }
 
@@ -2923,8 +2921,7 @@ inline bool AllocN(InterpState &S, CodePtr OpPC, PrimType T, const Expr *Source,
       Allocator.allocate(Source, T, static_cast<size_t>(NumElements),
                          S.Ctx.getEvalID(), DynamicAllocator::Form::Array);
   assert(B);
-  S.Stk.push<Pointer>(B, sizeof(InlineDescriptor));
-
+  S.Stk.push<Pointer>(B);
   return true;
 }
 
@@ -2950,9 +2947,7 @@ inline bool AllocCN(InterpState &S, CodePtr OpPC, const Descriptor *ElementDesc,
       Allocator.allocate(ElementDesc, static_cast<size_t>(NumElements),
                          S.Ctx.getEvalID(), DynamicAllocator::Form::Array);
   assert(B);
-
-  S.Stk.push<Pointer>(B, sizeof(InlineDescriptor));
-
+  S.Stk.push<Pointer>(B);
   return true;
 }
 

clang/lib/AST/ByteCode/InterpBuiltin.cpp

@@ -1655,49 +1655,50 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
     return false;
   }
 
+  bool IsArray = NumElems.ugt(1);
   std::optional<PrimType> ElemT = S.getContext().classify(ElemType);
   DynamicAllocator &Allocator = S.getAllocator();
   if (ElemT) {
-    if (NumElems.ule(1)) {
-      const Descriptor *Desc =
-          S.P.createDescriptor(NewCall, *ElemT, Descriptor::InlineDescMD,
-                               /*IsConst=*/false, /*IsTemporary=*/false,
-                               /*IsMutable=*/false);
-      Block *B = Allocator.allocate(Desc, S.getContext().getEvalID(),
+    if (IsArray) {
+      Block *B = Allocator.allocate(NewCall, *ElemT, NumElems.getZExtValue(),
+                                    S.Ctx.getEvalID(),
                                     DynamicAllocator::Form::Operator);
       assert(B);
-
-      S.Stk.push<Pointer>(B);
+      S.Stk.push<Pointer>(Pointer(B).atIndex(0));
       return true;
     }
-    assert(NumElems.ugt(1));
 
-    Block *B =
-        Allocator.allocate(NewCall, *ElemT, NumElems.getZExtValue(),
-                           S.Ctx.getEvalID(), DynamicAllocator::Form::Operator);
+    const Descriptor *Desc =
+        S.P.createDescriptor(NewCall, *ElemT, Descriptor::InlineDescMD,
+                             /*IsConst=*/false, /*IsTemporary=*/false,
+                             /*IsMutable=*/false);
+    Block *B = Allocator.allocate(Desc, S.getContext().getEvalID(),
+                                  DynamicAllocator::Form::Operator);
     assert(B);
+
     S.Stk.push<Pointer>(B);
     return true;
   }
 
   assert(!ElemT);
   // Structs etc.
   const Descriptor *Desc = S.P.createDescriptor(
-      NewCall, ElemType.getTypePtr(), Descriptor::InlineDescMD,
+      NewCall, ElemType.getTypePtr(),
+      IsArray ? std::nullopt : Descriptor::InlineDescMD,
       /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false,
       /*Init=*/nullptr);
 
-  if (NumElems.ule(1)) {
-    Block *B = Allocator.allocate(Desc, S.getContext().getEvalID(),
-                                  DynamicAllocator::Form::Operator);
+  if (IsArray) {
+    Block *B =
+        Allocator.allocate(Desc, NumElems.getZExtValue(), S.Ctx.getEvalID(),
+                           DynamicAllocator::Form::Operator);
     assert(B);
-    S.Stk.push<Pointer>(B);
+    S.Stk.push<Pointer>(Pointer(B).atIndex(0));
     return true;
   }
 
-  Block *B =
-      Allocator.allocate(Desc, NumElems.getZExtValue(), S.Ctx.getEvalID(),
-                         DynamicAllocator::Form::Operator);
+  Block *B = Allocator.allocate(Desc, S.getContext().getEvalID(),
+                                DynamicAllocator::Form::Operator);
   assert(B);
   S.Stk.push<Pointer>(B);
   return true;

clang/lib/AST/ByteCode/Program.cpp

@@ -432,8 +432,8 @@ Descriptor *Program::createDescriptor(const DeclTy &D, const Type *Ty,
         return allocateDescriptor(D, *T, MDSize, IsTemporary,
                                   Descriptor::UnknownSize{});
       } else {
-        const Descriptor *Desc = createDescriptor(D, ElemTy.getTypePtr(),
-                                                  MDSize, IsConst, IsTemporary);
+        const Descriptor *Desc = createDescriptor(
+            D, ElemTy.getTypePtr(), std::nullopt, IsConst, IsTemporary);
         if (!Desc)
           return nullptr;
         return allocateDescriptor(D, Desc, MDSize, IsTemporary,

clang/test/AST/ByteCode/allocate-arrays.cpp

@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++2c -fexperimental-new-constant-interpreter -verify=expected,both %s
+// RUN: %clang_cc1 -std=c++2c  -verify=ref,both %s
+
+
+/// This example used to cause an invalid read because allocating
+/// an array needs to return a pointer to the first element,
+/// not to the array.
+
+namespace std {
+  using size_t = decltype(sizeof(0));
+
+  template <class _Tp>
+  class allocator {
+  public:
+    typedef size_t size_type;
+    typedef _Tp value_type;
+    constexpr _Tp *allocate(size_t __n) {
+      return static_cast<_Tp *>(::operator new(__n * sizeof(_Tp)));
+    }
+  };
+}
+
+void *operator new(std::size_t, void *p) { return p; }
+void* operator new[] (std::size_t, void* p) {return p;}
+
+namespace std {
+  template <class _Ep>
+  class initializer_list {
+    const _Ep *__begin_;
+    __SIZE_TYPE__ __size_;
+
+  public:
+    typedef _Ep value_type;
+    typedef const _Ep &reference;
+    constexpr __SIZE_TYPE__ size() const noexcept { return __size_; }
+    constexpr const _Ep *begin() const noexcept { return __begin_; }
+    constexpr const _Ep *end() const noexcept { return __begin_ + __size_; }
+  };
+}
+
+template<typename T>
+class vector {
+public:
+  constexpr vector(std::initializer_list<T> Ts) {
+    A = B = std::allocator<T>{}.allocate(Ts.size()); // both-note {{heap allocation performed here}}
+
+    new (A) T(*Ts.begin());
+  }
+private:
+  T *A = nullptr;
+  T *B = nullptr;
+};
+
+constexpr vector<vector<int>> ints = {{3}, {4}}; // both-error {{must be initialized by a constant expression}} \
+                                                 // both-note {{pointer to}}