[clang][bytecode] Implement placement-new #107033
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tbaederr
added
the
clang:frontend
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesIf we have a placement-new destination already, use that instead of allocating a new one. Full diff: https://github.com/llvm/llvm-project/pull/107033.diff 5 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 554e23e272e41c..92332fbfdfd4e0 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2822,10 +2822,11 @@ bool Compiler<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) {
std::optional<PrimType> ElemT = classify(ElementType);
unsigned PlacementArgs = E->getNumPlacementArgs();
bool IsNoThrow = false;
+ const Expr *PlacementDest = nullptr;
// FIXME: Better diagnostic. diag::note_constexpr_new_placement
if (PlacementArgs != 0) {
- // The only new-placement list we support is of the form (std::nothrow).
+ // new-placement list of the form (std::nothrow).
//
// FIXME: There is no restriction on this, but it's not clear that any
// other form makes any sense. We get here for cases such as:
@@ -2835,27 +2836,37 @@ bool Compiler<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) {
// (which should presumably be valid only if N is a multiple of
// alignof(int), and in any case can't be deallocated unless N is
// alignof(X) and X has new-extended alignment).
- if (PlacementArgs != 1 || !E->getPlacementArg(0)->getType()->isNothrowT())
+ if (PlacementArgs == 1) {
+ const Expr *Arg1 = E->getPlacementArg(0);
+ if (Arg1->getType()->isNothrowT()) {
+ if (!this->discard(Arg1))
+ return false;
+ IsNoThrow = true;
+ } else {
+ // If we have a placement-new destination, we'll later use that instead
+ // of allocating.
+ PlacementDest = Arg1;
+ }
+ } else {
return this->emitInvalid(E);
-
- if (!this->discard(E->getPlacementArg(0)))
- return false;
- IsNoThrow = true;
+ }
}
const Descriptor *Desc;
- if (ElemT) {
- if (E->isArray())
- Desc = nullptr; // We're not going to use it in this case.
- else
- Desc = P.createDescriptor(E, *ElemT, Descriptor::InlineDescMD,
- /*IsConst=*/false, /*IsTemporary=*/false,
- /*IsMutable=*/false);
- } else {
- Desc = P.createDescriptor(
- E, ElementType.getTypePtr(),
- E->isArray() ? std::nullopt : Descriptor::InlineDescMD,
- /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false, Init);
+ if (!PlacementDest) {
+ if (ElemT) {
+ if (E->isArray())
+ Desc = nullptr; // We're not going to use it in this case.
+ else
+ Desc = P.createDescriptor(E, *ElemT, Descriptor::InlineDescMD,
+ /*IsConst=*/false, /*IsTemporary=*/false,
+ /*IsMutable=*/false);
+ } else {
+ Desc = P.createDescriptor(
+ E, ElementType.getTypePtr(),
+ E->isArray() ? std::nullopt : Descriptor::InlineDescMD,
+ /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false, Init);
+ }
}
if (E->isArray()) {
@@ -2872,26 +2883,42 @@ bool Compiler<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) {
PrimType SizeT = classifyPrim(Stripped->getType());
- if (!this->visit(Stripped))
- return false;
-
- if (ElemT) {
- // N primitive elements.
- if (!this->emitAllocN(SizeT, *ElemT, E, IsNoThrow, E))
+ if (PlacementDest) {
+ if (!this->visit(PlacementDest))
+ return false;
+ if (!this->visit(Stripped))
+ return false;
+ if (!this->emitCheckNewTypeMismatchArray(SizeT, E, E))
return false;
} else {
- // N Composite elements.
- if (!this->emitAllocCN(SizeT, Desc, IsNoThrow, E))
+ if (!this->visit(Stripped))
return false;
+
+ if (ElemT) {
+ // N primitive elements.
+ if (!this->emitAllocN(SizeT, *ElemT, E, IsNoThrow, E))
+ return false;
+ } else {
+ // N Composite elements.
+ if (!this->emitAllocCN(SizeT, Desc, IsNoThrow, E))
+ return false;
+ }
}
if (Init && !this->visitInitializer(Init))
return false;
} else {
- // Allocate just one element.
- if (!this->emitAlloc(Desc, E))
- return false;
+ if (PlacementDest) {
+ if (!this->visit(PlacementDest))
+ return false;
+ if (!this->emitCheckNewTypeMismatch(E, E))
+ return false;
+ } else {
+ // Allocate just one element.
+ if (!this->emitAlloc(Desc, E))
+ return false;
+ }
if (Init) {
if (ElemT) {
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 30ccceb42eb374..8a3cd55b5ad539 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -986,6 +986,51 @@ void diagnoseEnumValue(InterpState &S, CodePtr OpPC, const EnumDecl *ED,
}
}
+bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
+ std::optional<uint64_t> ArraySize) {
+ const Pointer &Ptr = S.Stk.peek<Pointer>();
+
+ if (!CheckStore(S, OpPC, Ptr))
+ return false;
+
+ const auto *NewExpr = cast<CXXNewExpr>(E);
+ QualType StorageType = Ptr.getType();
+
+ if (isa_and_nonnull<CXXNewExpr>(Ptr.getFieldDesc()->asExpr())) {
+ // FIXME: Are there other cases where this is a problem?
+ StorageType = StorageType->getPointeeType();
+ }
+
+ const ASTContext &ASTCtx = S.getASTContext();
+ QualType AllocType;
+ if (ArraySize) {
+ AllocType = ASTCtx.getConstantArrayType(
+ NewExpr->getAllocatedType(),
+ APInt(64, static_cast<uint64_t>(*ArraySize), false), nullptr,
+ ArraySizeModifier::Normal, 0);
+ } else {
+ AllocType = NewExpr->getAllocatedType();
+ }
+
+ unsigned StorageSize = 1;
+ unsigned AllocSize = 1;
+ if (const auto *CAT = dyn_cast<ConstantArrayType>(AllocType))
+ AllocSize = CAT->getZExtSize();
+ if (const auto *CAT = dyn_cast<ConstantArrayType>(StorageType))
+ StorageSize = CAT->getZExtSize();
+
+ if (AllocSize > StorageSize ||
+ !ASTCtx.hasSimilarType(ASTCtx.getBaseElementType(AllocType),
+ ASTCtx.getBaseElementType(StorageType))) {
+ S.FFDiag(S.Current->getLocation(OpPC),
+ diag::note_constexpr_placement_new_wrong_type)
+ << StorageType << AllocType;
+ return false;
+ }
+
+ return true;
+}
+
bool Interpret(InterpState &S, APValue &Result) {
// The current stack frame when we started Interpret().
// This is being used by the ops to determine wheter
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index c1423a060bcb97..a66723acbaf62c 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -3151,6 +3151,16 @@ inline bool CheckLiteralType(InterpState &S, CodePtr OpPC, const Type *T) {
return false;
}
+/// Check if the initializer and storage types of a placement-new expression
+/// match.
+bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
+ std::optional<uint64_t> ArraySize = std::nullopt);
+
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+bool CheckNewTypeMismatchArray(InterpState &S, CodePtr OpPC, const Expr *E) {
+ const auto &Size = S.Stk.pop<T>();
+ return CheckNewTypeMismatch(S, OpPC, E, static_cast<uint64_t>(Size));
+}
//===----------------------------------------------------------------------===//
// Read opcode arguments
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 46247688d4ef85..980204cf1ba071 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -786,4 +786,14 @@ def Free : Opcode {
let Args = [ArgBool];
}
+def CheckNewTypeMismatch : Opcode {
+ let Args = [ArgExpr];
+}
+
+def CheckNewTypeMismatchArray : Opcode {
+ let Types = [IntegerTypeClass];
+ let Args = [ArgExpr];
+ let HasGroup = 1;
+}
+
def IsConstantContext: Opcode;
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
new file mode 100644
index 00000000000000..1fcbdd706b18b6
--- /dev/null
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -0,0 +1,177 @@
+// RUN: %clang_cc1 -std=c++2c -fexperimental-new-constant-interpreter -verify=expected,both %s
+// RUN: %clang_cc1 -std=c++2c -verify=ref,both %s
+
+namespace std {
+ using size_t = decltype(sizeof(0));
+}
+
+void *operator new(std::size_t, void *p) { return p; }
+void* operator new[] (std::size_t, void* p) {return p;}
+
+
+consteval auto ok1() {
+ bool b;
+ new (&b) bool(true);
+ return b;
+}
+static_assert(ok1());
+
+consteval auto ok2() {
+ int b;
+ new (&b) int(12);
+ return b;
+}
+static_assert(ok2() == 12);
+
+
+consteval auto ok3() {
+ float b;
+ new (&b) float(12.0);
+ return b;
+}
+static_assert(ok3() == 12.0);
+
+
+consteval auto ok4() {
+ _BitInt(11) b;
+ new (&b) _BitInt(11)(37);
+ return b;
+}
+static_assert(ok4() == 37);
+
+consteval auto fail1() {
+ int b;
+ new (&b) float(1.0); // both-note {{placement new would change type of storage from 'int' to 'float'}}
+ return b;
+}
+static_assert(fail1() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+consteval int fail2() {
+ int i;
+ new (static_cast<void*>(&i)) float(0); // both-note {{placement new would change type of storage from 'int' to 'float'}}
+ return 0;
+}
+static_assert(fail2() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+consteval int indeterminate() {
+ int * indeterminate;
+ new (indeterminate) int(0); // both-note {{read of uninitialized object is not allowed in a constant expression}}
+ return 0;
+}
+static_assert(indeterminate() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+consteval int array1() {
+ int i[2];
+ new (&i) int[]{1,2};
+ return i[0] + i[1];
+}
+static_assert(array1() == 3);
+
+consteval int array2() {
+ int i[2];
+ new (static_cast<void*>(&i)) int[]{1,2};
+ return i[0] + i[1];
+}
+static_assert(array2() == 3);
+
+consteval int array3() {
+ int i[1];
+ new (&i) int[2]; // both-note {{placement new would change type of storage from 'int[1]' to 'int[2]'}}
+ return 0;
+}
+static_assert(array3() == 0); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+consteval int array4() {
+ int i[2];
+ new (&i) int[]{12};
+ return i[0];
+}
+static_assert(array4() == 12);
+
+constexpr int *intptr() {
+ return new int;
+}
+constexpr bool yay() {
+ int *ptr = new (intptr()) int(42);
+ bool ret = *ptr == 42;
+ delete ptr;
+ return ret;
+}
+static_assert(yay());
+
+
+constexpr bool blah() {
+ int *ptr = new (intptr()) int[3]{ 1, 2, 3 }; // both-note {{placement new would change type of storage from 'int' to 'int[3]'}}
+ bool ret = ptr[0] == 1 && ptr[1] == 2 && ptr[2] == 3;
+ delete [] ptr;
+ return ret;
+}
+static_assert(blah()); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to 'blah()'}}
+
+
+constexpr int *get_indeterminate() {
+ int *evil;
+ return evil; // both-note {{read of uninitialized object is not allowed in a constant expression}}
+}
+
+constexpr bool bleh() {
+ int *ptr = new (get_indeterminate()) int; // both-note {{in call to 'get_indeterminate()'}}
+ return true;
+}
+static_assert(bleh()); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to 'bleh()'}}
+
+namespace records {
+ class S {
+ public:
+ float f;
+ };
+
+ constexpr bool record1() {
+ S s(13);
+ new (&s) S(42);
+ return s.f == 42;
+ }
+ static_assert(record1());
+
+ S GlobalS;
+ constexpr bool record2() {
+ new (&GlobalS) S(42); // both-note {{a constant expression cannot modify an object that is visible outside that expression}}
+ return GlobalS.f == 42;
+ }
+ static_assert(record2()); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+
+
+ constexpr bool record3() {
+ S ss[3];
+
+ new (&ss) S[]{{1}, {2}, {3}};
+
+ return ss[0].f == 1 && ss[1].f == 2 && ss[2].f == 3;
+ }
+ static_assert(record3());
+
+ struct F {
+ float f;
+ };
+ struct R {
+ F f;
+ int a;
+ };
+ constexpr bool record4() {
+ R r;
+ new (&r.f) F{42.0};
+ new (&r.a) int(12);
+
+ return r.f.f == 42.0 && r.a == 12;
+ }
+ static_assert(record4());
+}
+
+
|
tbaederr
force-pushed
the
placement-new
branch
7 times, most recently
from
5286c57 to
19abb83
Compare
tbaederr
force-pushed
the
placement-new
branch
from
19abb83 to
3dca414
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/6308 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If we have a placement-new destination already, use that instead of allocating a new one.
Tests are partially based on
test/SemaCXX/cxx2c-constexpr-placement-new.cpp.