[clang] [clang][bytecode] Fix dynamic array allocation return values (PR #127387)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 16 02:47:34 PST 2025
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/127387
We need to return a pointer to the first element, not the array itself.
>From eb8387d21d1b5455b9450802729cf407bfb9d703 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Sun, 16 Feb 2025 09:43:50 +0100
Subject: [PATCH] [clang][bytecode] Fix dynamic array allocation return values
We need to return a pointer to the first element, not the array itself.
---
clang/lib/AST/ByteCode/DynamicAllocator.cpp | 2 +
clang/lib/AST/ByteCode/Interp.h | 9 +---
clang/lib/AST/ByteCode/InterpBuiltin.cpp | 41 +++++++--------
clang/lib/AST/ByteCode/Program.cpp | 4 +-
clang/test/AST/ByteCode/allocate-arrays.cpp | 55 +++++++++++++++++++++
5 files changed, 82 insertions(+), 29 deletions(-)
create mode 100644 clang/test/AST/ByteCode/allocate-arrays.cpp
diff --git a/clang/lib/AST/ByteCode/DynamicAllocator.cpp b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
index 819fbdb8b070b..3ef8c2e1f3e7c 100644
--- a/clang/lib/AST/ByteCode/DynamicAllocator.cpp
+++ b/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;
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 5cc371c7ee495..73cc107b7dbff 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/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;
}
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 55ac41736344d..b964906fb6594 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1655,27 +1655,27 @@ 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;
}
@@ -1683,21 +1683,22 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
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;
diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp
index 833c9ef88d770..0754e259b7cb3 100644
--- a/clang/lib/AST/ByteCode/Program.cpp
+++ b/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,
diff --git a/clang/test/AST/ByteCode/allocate-arrays.cpp b/clang/test/AST/ByteCode/allocate-arrays.cpp
new file mode 100644
index 0000000000000..f1e5af6cab2aa
--- /dev/null
+++ b/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}}
More information about the cfe-commits
mailing list