Commit 3709ce4c authored by Jakob Kummerow's avatar Jakob Kummerow Committed by V8 LUCI CQ

[wasm-gc] Fix struct size extreme cases

Structs with zero fields weren't handled correctly, because the GC
has a requirement that each object occupies at least two pointers.

On the high end, Wasm structs accidentally had a limit of 255 pointers
including object header. This CL bumps that to the intended limit
of 999 fields (which is arbitrary and could be raised if needed).

Bug: v8:7748
Change-Id: I13a3f45b3ddb28023c76775da32be0d07ec2ffd0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2874653
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarManos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74436}
parent a0a5aeec
......@@ -287,8 +287,9 @@ builtin WasmAllocateRtt(typeIndex: intptr, parent: Map): Map {
}
builtin WasmAllocateStructWithRtt(rtt: Map): HeapObject {
const instanceSize: intptr =
unsafe::TimesTaggedSize(Convert<intptr>(rtt.instance_size_in_words));
const typeInfo: WasmTypeInfo = %RawDownCast<WasmTypeInfo>(
rtt.constructor_or_back_pointer_or_native_context);
const instanceSize: intptr = SmiUntag(typeInfo.instance_size);
const result: HeapObject = unsafe::Allocate(
instanceSize, AllocationFlag::kAllowLargeObjectAllocation);
*UnsafeConstCast(&result.map) = rtt;
......
......@@ -5526,6 +5526,16 @@ Node* WasmGraphBuilder::StructNewWithRtt(uint32_t struct_index,
for (uint32_t i = 0; i < type->field_count(); i++) {
gasm_->StoreStructField(s, type, i, fields[i]);
}
if (type->field_count() == 0) {
static_assert(Heap::kMinObjectSizeInTaggedWords == 2 &&
WasmStruct::kHeaderSize == kTaggedSize,
"empty structs need exactly one padding field");
wasm::ValueType fake_type = wasm::kWasmAnyRef;
Node* padding_offset = gasm_->IntPtrConstant(
wasm::ObjectAccess::ToTagged(WasmStruct::kHeaderSize));
gasm_->StoreToObject(ObjectAccessForGCStores(fake_type), s, padding_offset,
RefNull());
}
return s;
}
......
......@@ -1361,7 +1361,8 @@ Handle<Foreign> Factory::NewForeign(Address addr) {
#if V8_ENABLE_WEBASSEMBLY
Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(Address type_address,
Handle<Map> opt_parent) {
Handle<Map> opt_parent,
int instance_size_bytes) {
// We pretenure WasmTypeInfo objects because they are refererenced by Maps,
// which are assumed to be long-lived. The supertypes list is constant
// after initialization, so we pretenure that too.
......@@ -1385,6 +1386,7 @@ Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(Address type_address,
result.set_foreign_address(isolate(), type_address);
result.set_supertypes(*supertypes, SKIP_WRITE_BARRIER);
result.set_subtypes(*subtypes);
result.set_instance_size(instance_size_bytes);
return handle(result, isolate());
}
......
......@@ -554,7 +554,8 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
#if V8_ENABLE_WEBASSEMBLY
Handle<WasmTypeInfo> NewWasmTypeInfo(Address type_address,
Handle<Map> opt_parent);
Handle<Map> opt_parent,
int instance_size_bytes);
Handle<WasmExportedFunctionData> NewWasmExportedFunctionData(
Handle<Code> export_wrapper, Handle<WasmInstanceObject> instance,
Address call_target, Handle<Object> ref, int func_index,
......
......@@ -699,7 +699,7 @@ class WasmStruct::BodyDescriptor final : public BodyDescriptorBase {
}
static inline int SizeOf(Map map, HeapObject object) {
return map.instance_size();
return WasmStruct::GcSafeSize(map);
}
};
......
......@@ -2361,6 +2361,9 @@ int HeapObject::SizeFromMap(Map map) const {
CoverageInfo::unchecked_cast(*this).slot_count());
}
#if V8_ENABLE_WEBASSEMBLY
if (instance_type == WASM_STRUCT_TYPE) {
return WasmStruct::GcSafeSize(map);
}
if (instance_type == WASM_ARRAY_TYPE) {
return WasmArray::GcSafeSizeFor(map, WasmArray::cast(*this).length());
}
......
......@@ -4812,6 +4812,17 @@ class LiftoffCompiler {
StoreObjectField(obj.gp(), no_reg, offset, value, pinned, field_kind);
pinned.clear(value);
}
if (imm.struct_type->field_count() == 0) {
static_assert(Heap::kMinObjectSizeInTaggedWords == 2 &&
WasmStruct::kHeaderSize == kTaggedSize,
"empty structs need exactly one padding field");
ValueKind field_kind = ValueKind::kRef;
LiftoffRegister value = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
LoadNullValue(value.gp(), pinned);
StoreObjectField(obj.gp(), no_reg, WasmStruct::kHeaderSize, value, pinned,
field_kind);
pinned.clear(value);
}
__ PushRegister(kRef, obj);
}
......
......@@ -123,16 +123,17 @@ Handle<Map> CreateStructMap(Isolate* isolate, const WasmModule* module,
int struct_index, Handle<Map> opt_rtt_parent) {
const wasm::StructType* type = module->struct_type(struct_index);
const int inobject_properties = 0;
DCHECK_LE(type->total_fields_size(), kMaxInt - WasmStruct::kHeaderSize);
const int instance_size =
WasmStruct::kHeaderSize + static_cast<int>(type->total_fields_size());
// We have to use the variable size sentinel because the instance size
// stored directly in a Map is capped at 255 pointer sizes.
const int map_instance_size = kVariableSizeSentinel;
const int real_instance_size = WasmStruct::Size(type);
const InstanceType instance_type = WASM_STRUCT_TYPE;
// TODO(jkummerow): If NO_ELEMENTS were supported, we could use that here.
const ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND;
Handle<WasmTypeInfo> type_info = isolate->factory()->NewWasmTypeInfo(
reinterpret_cast<Address>(type), opt_rtt_parent);
reinterpret_cast<Address>(type), opt_rtt_parent, real_instance_size);
Handle<Map> map = isolate->factory()->NewMap(
instance_type, instance_size, elements_kind, inobject_properties);
instance_type, map_instance_size, elements_kind, inobject_properties);
map->set_wasm_type_info(*type_info);
return map;
}
......@@ -142,10 +143,12 @@ Handle<Map> CreateArrayMap(Isolate* isolate, const WasmModule* module,
const wasm::ArrayType* type = module->array_type(array_index);
const int inobject_properties = 0;
const int instance_size = kVariableSizeSentinel;
// Wasm Arrays don't have a static instance size.
const int cached_instance_size = 0;
const InstanceType instance_type = WASM_ARRAY_TYPE;
const ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND;
Handle<WasmTypeInfo> type_info = isolate->factory()->NewWasmTypeInfo(
reinterpret_cast<Address>(type), opt_rtt_parent);
reinterpret_cast<Address>(type), opt_rtt_parent, cached_instance_size);
Handle<Map> map = isolate->factory()->NewMap(
instance_type, instance_size, elements_kind, inobject_properties);
map->set_wasm_type_info(*type_info);
......
......@@ -423,6 +423,20 @@ wasm::StructType* WasmStruct::GcSafeType(Map map) {
return reinterpret_cast<wasm::StructType*>(foreign.foreign_address());
}
int WasmStruct::Size(const wasm::StructType* type) {
// Object size must fit into a Smi (because of filler objects), and its
// computation must not overflow.
STATIC_ASSERT(Smi::kMaxValue <= kMaxInt);
DCHECK_LE(type->total_fields_size(), Smi::kMaxValue - kHeaderSize);
return std::max(kHeaderSize + static_cast<int>(type->total_fields_size()),
Heap::kMinObjectSizeInTaggedWords * kTaggedSize);
}
int WasmStruct::GcSafeSize(Map map) {
wasm::StructType* type = GcSafeType(map);
return Size(type);
}
wasm::StructType* WasmStruct::type() const { return type(map()); }
ObjectSlot WasmStruct::RawField(int raw_offset) {
......
......@@ -930,6 +930,8 @@ class WasmStruct : public TorqueGeneratedWasmStruct<WasmStruct, HeapObject> {
static inline wasm::StructType* type(Map map);
inline wasm::StructType* type() const;
static inline wasm::StructType* GcSafeType(Map map);
static inline int Size(const wasm::StructType* type);
static inline int GcSafeSize(Map map);
inline ObjectSlot RawField(int raw_offset);
......
......@@ -117,6 +117,8 @@ extern class AsmWasmData extends Struct {
extern class WasmTypeInfo extends Foreign {
supertypes: FixedArray;
subtypes: ArrayList;
// In bytes, used for struct allocation.
instance_size: Smi;
}
@generateCppClass
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment