Skip to content

Commit

Permalink
[Wasm-GC] Fix missing write barrier in BBQJIT struct.set
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=266043

Reviewed by Yusuke Suzuki.

Adds a missing write barrier for struct.set in BBQJIT. Also optimize how struct
mutation is done for initialization (e.g., struct.new), because the old approach
was compiling duplicate loads for the payload pointer.

* JSTests/wasm/gc/bug266043.js: Added.
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::emitStructSet):
(JSC::Wasm::BBQJIT::emitStructPayloadSet):
(JSC::Wasm::BBQJIT::addStructNewDefault):
(JSC::Wasm::BBQJIT::addStructNew):

Canonical link: https://commits.webkit.org/271740@main
  • Loading branch information
@takikawa
takikawa committed Dec 8, 2023
1 parent 1f9e3fc commit 50def56
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 16 deletions.
34 changes: 34 additions & 0 deletions JSTests/wasm/gc/bug266043.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//@ runWebAssemblySuite("--useWebAssemblyTypedFunctionReferences=true", "--useWebAssemblyGC=true")

import * as assert from "../assert.js";
import { compile, instantiate } from "./wast-wrapper.js";

const m1 = instantiate(`
(module
(type (struct (field i32)))
(type (struct (field (mut (ref null 0)))))
(func (export "f") (result (ref any))
(struct.new 1 (ref.null 0)))
)
`);

// GC to ensure struct is an old object.
const struct = m1.exports.f()
gc();

const m2 = instantiate(`
(module
(type (struct (field i32)))
(type (struct (field (mut (ref null 0)))))
(func (export "g") (param (ref 1))
(struct.set 1 0 (local.get 0) (struct.new 0 (i32.const 42))))
(func (export "h") (param (ref 1)) (result i32)
(struct.get 0 0 (struct.get 1 0 (local.get 0))))
)
`);

// Do an eden GC for the new struct allocated in m2. Write barriers
// should ensure that the object will be kept live.
m2.exports.g(struct);
edenGC();
assert.eq(m2.exports.h(struct), 42);
50 changes: 34 additions & 16 deletions Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3959,6 +3959,13 @@ class BBQJIT {
void emitStructSet(GPRReg structGPR, const StructType& structType, uint32_t fieldIndex, Value value)
{
m_jit.loadPtr(MacroAssembler::Address(structGPR, JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
emitStructPayloadSet(wasmScratchGPR, structType, fieldIndex, value);
if (isRefType(structType.field(fieldIndex).type))
emitWriteBarrier(structGPR);
}

void emitStructPayloadSet(GPRReg payloadGPR, const StructType& structType, uint32_t fieldIndex, Value value)
{
unsigned fieldOffset = *structType.offsetOfField(fieldIndex);
RELEASE_ASSERT((std::numeric_limits<int32_t>::max() & fieldOffset) == fieldOffset);

Expand All @@ -3972,22 +3979,22 @@ class BBQJIT {
emitMoveConst(value, Location::fromGPR(scratches.gpr(0)));
switch (structType.field(fieldIndex).type.as<PackedType>()) {
case PackedType::I8:
m_jit.store8(scratches.gpr(0), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store8(scratches.gpr(0), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case PackedType::I16:
m_jit.store16(scratches.gpr(0), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store16(scratches.gpr(0), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
}
break;
}
m_jit.store32(MacroAssembler::Imm32(value.asI32()), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store32(MacroAssembler::Imm32(value.asI32()), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case TypeKind::F32:
m_jit.store32(MacroAssembler::Imm32(value.asI32()), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store32(MacroAssembler::Imm32(value.asI32()), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case TypeKind::I64:
case TypeKind::F64:
m_jit.store64(MacroAssembler::Imm64(value.asI64()), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store64(MacroAssembler::Imm64(value.asI64()), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
default:
RELEASE_ASSERT_NOT_REACHED();
Expand All @@ -4002,24 +4009,24 @@ class BBQJIT {
if (structType.field(fieldIndex).type.is<PackedType>()) {
switch (structType.field(fieldIndex).type.as<PackedType>()) {
case PackedType::I8:
m_jit.store8(valueLocation.asGPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store8(valueLocation.asGPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case PackedType::I16:
m_jit.store16(valueLocation.asGPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store16(valueLocation.asGPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
}
break;
}
m_jit.store32(valueLocation.asGPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store32(valueLocation.asGPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case TypeKind::I64:
m_jit.store64(valueLocation.asGPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.store64(valueLocation.asGPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case TypeKind::F32:
m_jit.storeFloat(valueLocation.asFPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.storeFloat(valueLocation.asFPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
case TypeKind::F64:
m_jit.storeDouble(valueLocation.asFPR(), MacroAssembler::Address(wasmScratchGPR, fieldOffset));
m_jit.storeDouble(valueLocation.asFPR(), MacroAssembler::Address(payloadGPR, fieldOffset));
break;
default:
RELEASE_ASSERT_NOT_REACHED();
Expand All @@ -4042,13 +4049,16 @@ class BBQJIT {

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location structLocation = allocate(result);
m_jit.loadPtr(MacroAssembler::Address(structLocation.asGPR(), JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
for (StructFieldCount i = 0; i < structType.fieldCount(); ++i) {
if (Wasm::isRefType(structType.field(i).type))
emitStructSet(structLocation.asGPR(), structType, i, Value::fromRef(TypeKind::RefNull, JSValue::encode(jsNull())));
emitStructPayloadSet(wasmScratchGPR, structType, i, Value::fromRef(TypeKind::RefNull, JSValue::encode(jsNull())));
else
emitStructSet(structLocation.asGPR(), structType, i, Value::fromI64(0));
emitStructPayloadSet(wasmScratchGPR, structType, i, Value::fromI64(0));
}

// No write barrier needed here as all fields are set to constants.

LOG_INSTRUCTION("StructNewDefault", typeIndex, RESULT(result));

return { };
Expand All @@ -4065,9 +4075,17 @@ class BBQJIT {
emitCCall(operationWasmStructNewEmpty, arguments, allocationResult);

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location allocationLocation = allocate(allocationResult);
for (uint32_t i = 0; i < args.size(); ++i)
emitStructSet(allocationLocation.asGPR(), structType, i, args[i]);
Location structLocation = allocate(allocationResult);
m_jit.loadPtr(MacroAssembler::Address(structLocation.asGPR(), JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
bool hasRefTypeField = false;
for (uint32_t i = 0; i < args.size(); ++i) {
if (isRefType(structType.field(i).type))
hasRefTypeField = true;
emitStructPayloadSet(wasmScratchGPR, structType, i, args[i]);
}

if (hasRefTypeField)
emitWriteBarrier(structLocation.asGPR());

result = topValue(TypeKind::Structref);
Location resultLocation = allocate(result);
Expand Down

0 comments on commit 50def56

Please sign in to comment.