Skip to content

Commit

Permalink
[Wasm-GC] Handle OOM for allocations consistently
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264454

Reviewed by Justin Michaud.

Check for OOM and raise an exception consistently for all Wasm GC allocation
points. Refactors some function names to match.

* JSTests/wasm/gc/array_new_data.js:
(testBadOffset):
(testReadOutOfBounds):
(testInt32Overflow):
* JSTests/wasm/gc/array_new_elem.js:
(testInt32Overflow):
(testAllElementSegmentKinds):
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::addArrayNew):
(JSC::Wasm::B3IRGenerator::pushArrayNewFromSegment):
(JSC::Wasm::B3IRGenerator::addArrayNewDefault):
(JSC::Wasm::B3IRGenerator::addArrayNewData):
(JSC::Wasm::B3IRGenerator::addArrayNewElem):
(JSC::Wasm::B3IRGenerator::addArrayNewFixed):
(JSC::Wasm::B3IRGenerator::addStructNew):
(JSC::Wasm::B3IRGenerator::addStructNewDefault):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addArrayNewData):
(JSC::Wasm::BBQJITImpl::BBQJIT::addArrayNewElem):
* Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addStructNewDefault):
(JSC::Wasm::BBQJITImpl::BBQJIT::addStructNew):
* Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addStructNewDefault):
(JSC::Wasm::BBQJITImpl::BBQJIT::addStructNew):
* Source/JavaScriptCore/wasm/WasmConstExprGenerator.cpp:
(JSC::Wasm::ConstExprGenerator::ConstExprValue::ConstExprValue):
(JSC::Wasm::ConstExprGenerator::ConstExprValue::isInvalid):
(JSC::Wasm::ConstExprGenerator::createNewArray):
(JSC::Wasm::ConstExprGenerator::addArrayNew):
(JSC::Wasm::ConstExprGenerator::addArrayNewDefault):
(JSC::Wasm::ConstExprGenerator::addArrayNewFixed):
(JSC::Wasm::ConstExprGenerator::createNewStruct):
(JSC::Wasm::ConstExprGenerator::addStructNewDefault):
(JSC::Wasm::ConstExprGenerator::addStructNew):
* Source/JavaScriptCore/wasm/WasmExceptionType.h:
(JSC::Wasm::isTypeErrorExceptionType):
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmOperationsInlines.h:
(JSC::Wasm::fillArray):
(JSC::Wasm::arrayNew):
(JSC::Wasm::copyElementsInReverse):
(JSC::Wasm::arrayNewFixed):
(JSC::Wasm::createArrayValue):
(JSC::Wasm::structNew):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::WASM_SLOW_PATH_DECL):
* Source/JavaScriptCore/wasm/js/JSWebAssemblyArray.h:
* Source/JavaScriptCore/wasm/js/JSWebAssemblyStruct.cpp:
(JSC::JSWebAssemblyStruct::tryCreate):
(JSC::JSWebAssemblyStruct::create): Deleted.
* Source/JavaScriptCore/wasm/js/JSWebAssemblyStruct.h:

Canonical link: https://commits.webkit.org/275059@main
  • Loading branch information
takikawa committed Feb 20, 2024
1 parent 61b0604 commit 6b7295e
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 80 deletions.
32 changes: 16 additions & 16 deletions JSTests/wasm/gc/array_new_data.js
Expand Up @@ -150,7 +150,7 @@ function testBadOffset() {
(i32.const 0)
(array.get_u 0)))`)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => check(instantiate(`
(module
(memory (export "memory") 1)
Expand All @@ -163,7 +163,7 @@ function testBadOffset() {
(i32.const 0)
(array.get_u 0)))`)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
}

function testOffsets() {
Expand Down Expand Up @@ -211,7 +211,7 @@ function testReadOutOfBounds() {
(i32.const 0)
(array.get` + suffix + ` 0)))`)).exports.f(),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment")
"Out of bounds or failed to allocate in array.new_data")
};
// Test {i8, i16, i32, i64}
for (const type of [8, 16, 32, 64]) {
Expand Down Expand Up @@ -243,28 +243,28 @@ function testInt32Overflow() {
let maxUint32 = 0xffffffff;
assert.throws(() => instantiate(f(1, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(2, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(2, maxUint32 - 1)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(10, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(maxUint32, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(maxUint32 - 4, 5)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(maxUint32, 1)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(maxUint32 - 1, 2)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
// Check for overflow when multiplying element size by array size
f = (offset, len) => instantiate(`
(module
Expand All @@ -280,10 +280,10 @@ function testInt32Overflow() {
let badArraySize = 0x40000000;
assert.throws(() => instantiate(f(0, badArraySize)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
assert.throws(() => instantiate(f(1, badArraySize - 1)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the size of a data segment");
"Out of bounds or failed to allocate in array.new_data");
}


Expand All @@ -308,7 +308,7 @@ function testZeroLengthArray() {
// zero-length array from zero-length data segment; non-zero offset; should throw
m = f("", 3);
assert.throws(() => instantiate(m).exports.f(),
WebAssembly.RuntimeError, "Offset + array length would exceed the size of a data segment");
WebAssembly.RuntimeError, "Out of bounds or failed to allocate in array.new_data");
// zero-length array from non-zero-length data segment; non-zero offset
m = f("xyz", 1);
assert.eq(instantiate(m).exports.f(), 0);
Expand All @@ -318,7 +318,7 @@ function testZeroLengthArray() {
assert.eq(instantiate(m).exports.f(), 0);
// zero-length array from non-zero-length data segment; offset > length; should throw
m = f("xyz", 4);
assert.throws(() => instantiate(m).exports.f(), WebAssembly.RuntimeError, "Offset + array length would exceed the size of a data segment");
assert.throws(() => instantiate(m).exports.f(), WebAssembly.RuntimeError, "Out of bounds or failed to allocate in array.new_data");
}

function testSingletonArray() {
Expand All @@ -332,7 +332,7 @@ function testSingletonArray() {
(i32.const 1)
(array.new_data 0 0)
(array.len)))`);
let msg = "Offset + array length would exceed the size of a data segment";
let msg = "Out of bounds or failed to allocate in array.new_data";
// singleton array from 0-length data segment -- should throw
var m = f("", 0);
assertFails(m, msg);
Expand Down
27 changes: 13 additions & 14 deletions JSTests/wasm/gc/array_new_elem.js
Expand Up @@ -257,32 +257,31 @@ function testInt32Overflow() {
(array.len)))`).exports.f();
let maxUint32 = 0xffffffff;
assert.throws(() => instantiate(f(0, maxUint32)),
WebAssembly.RuntimeError, "Offset + array length would exceed the length of an element segment");
WebAssembly.RuntimeError, "Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(1, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(2, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(2, maxUint32 - 1)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(10, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(maxUint32, maxUint32)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(maxUint32 - 4, 5)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(maxUint32, 1)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");
"Out of bounds or failed to allocate in array.new_elem");
assert.throws(() => instantiate(f(maxUint32 - 1, 2)),
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment");

"Out of bounds or failed to allocate in array.new_elem");
}

// Creating a zero-length array should work, as long as the element segment offset is valid
Expand All @@ -306,7 +305,7 @@ function testZeroLengthArray() {
// zero-length array from zero-length element segment; non-zero offset; should throw
m = f("", 3);
assert.throws(() => instantiate(m).exports.f(),
WebAssembly.RuntimeError, "Offset + array length would exceed the length of an element segment");
WebAssembly.RuntimeError, "Out of bounds or failed to allocate in array.new_elem");
// zero-length array from non-zero-length data segment; non-zero offset
m = f("(ref.func $f) (ref.func $f)", 1);
assert.eq(instantiate(m).exports.f(), 0);
Expand All @@ -316,7 +315,7 @@ function testZeroLengthArray() {
assert.eq(instantiate(m).exports.f(), 0);
// zero-length array from non-zero-length element segment; offset > length; should throw
m = f("(ref.func $f) (ref.func $f) (ref.func $f)", 4);
assert.throws(() => instantiate(m).exports.f(), WebAssembly.RuntimeError, "Offset + array length would exceed the length of an element segment");
assert.throws(() => instantiate(m).exports.f(), WebAssembly.RuntimeError, "Out of bounds or failed to allocate in array.new_elem");
}

function testArrayNewCanonElemSubtype() {
Expand Down Expand Up @@ -629,8 +628,8 @@ function testAllElementSegmentKinds() {
assert.throws(
() => { m.exports.test(0) },
WebAssembly.RuntimeError,
"Offset + array length would exceed the length of an element segment (evaluating 'm.exports.test(0)')"
)
"Out of bounds or failed to allocate in array.new_elem"
);

m = instantiate(`
(module
Expand Down
36 changes: 25 additions & 11 deletions Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Expand Up @@ -2992,6 +2992,8 @@ auto B3IRGenerator::addArrayNew(uint32_t typeIndex, ExpressionType size, Express

result = pushArrayNew(typeIndex, initValue, size);

emitArrayNullCheck(get(result), ExceptionType::BadArrayNew);

return { };
}

Expand All @@ -3002,12 +3004,8 @@ Variable* B3IRGenerator::pushArrayNewFromSegment(arraySegmentOperation operation
m_currentBlock->appendNew<Const32Value>(m_proc, origin(), segmentIndex),
get(arraySize), get(offset));

// Check for null return value (indicating that this access is out of bounds for the segment)
CheckValue* check = m_currentBlock->appendNew<CheckValue>(m_proc, Check, origin(),
m_currentBlock->appendNew<Value>(m_proc, Equal, origin(), resultValue, m_currentBlock->appendNew<Const64Value>(m_proc, origin(), JSValue::encode(jsNull()))));
check->setGenerator([=, this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
this->emitExceptionCheck(jit, exceptionType);
});
// Indicates out of bounds for the segment or allocation failure.
emitArrayNullCheck(resultValue, exceptionType);

return push(resultValue);
}
Expand All @@ -3020,19 +3018,21 @@ auto B3IRGenerator::addArrayNewDefault(uint32_t typeIndex, ExpressionType size,
result = push(callWasmOperation(m_currentBlock, toB3Type(resultType), operationWasmArrayNewEmpty,
instanceValue(), m_currentBlock->appendNew<Const32Value>(m_proc, origin(), typeIndex), get(size)));

emitArrayNullCheck(get(result), ExceptionType::BadArrayNew);

return { };
}

auto B3IRGenerator::addArrayNewData(uint32_t typeIndex, uint32_t dataIndex, ExpressionType arraySize, ExpressionType offset, ExpressionType& result) -> PartialResult
{
result = pushArrayNewFromSegment(operationWasmArrayNewData, typeIndex, dataIndex, arraySize, offset, ExceptionType::OutOfBoundsDataSegmentAccess);
result = pushArrayNewFromSegment(operationWasmArrayNewData, typeIndex, dataIndex, arraySize, offset, ExceptionType::BadArrayNewInitData);

return { };
}

auto B3IRGenerator::addArrayNewElem(uint32_t typeIndex, uint32_t elemSegmentIndex, ExpressionType arraySize, ExpressionType offset, ExpressionType& result) -> PartialResult
{
result = pushArrayNewFromSegment(operationWasmArrayNewElem, typeIndex, elemSegmentIndex, arraySize, offset, ExceptionType::OutOfBoundsElementSegmentAccess);
result = pushArrayNewFromSegment(operationWasmArrayNewElem, typeIndex, elemSegmentIndex, arraySize, offset, ExceptionType::BadArrayNewInitElem);
return { };
}

Expand All @@ -3050,9 +3050,7 @@ auto B3IRGenerator::addArrayNewFixed(uint32_t typeIndex, Vector<ExpressionType>&
instanceValue(), m_currentBlock->appendNew<Const32Value>(m_proc, origin(), typeIndex),
m_currentBlock->appendNew<Const32Value>(m_proc, origin(), args.size()));

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=264454
// Ensure array is non-null
emitArrayNullCheck(arrayValue, ExceptionType::NullArraySet);
emitArrayNullCheck(arrayValue, ExceptionType::BadArrayNew);

for (uint32_t i = 0; i < args.size(); ++i) {
// Emit the array set code -- note that this omits the bounds check, since
Expand Down Expand Up @@ -3326,6 +3324,14 @@ auto B3IRGenerator::addStructNew(uint32_t typeIndex, Vector<ExpressionType>& arg
instanceValue(),
m_currentBlock->appendNew<Const32Value>(m_proc, origin(), typeIndex));

{
CheckValue* check = m_currentBlock->appendNew<CheckValue>(m_proc, Check, origin(),
m_currentBlock->appendNew<Value>(m_proc, Equal, origin(), structValue, m_currentBlock->appendNew<Const64Value>(m_proc, origin(), JSValue::encode(jsNull()))));
check->setGenerator([=, this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
this->emitExceptionCheck(jit, ExceptionType::BadStructNew);
});
}

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
for (uint32_t i = 0; i < args.size(); ++i)
emitStructSet(structValue, i, structType, get(args[i]));
Expand All @@ -3345,6 +3351,14 @@ auto B3IRGenerator::addStructNewDefault(uint32_t typeIndex, ExpressionType& resu
instanceValue(),
m_currentBlock->appendNew<Const32Value>(m_proc, origin(), typeIndex));

{
CheckValue* check = m_currentBlock->appendNew<CheckValue>(m_proc, Check, origin(),
m_currentBlock->appendNew<Value>(m_proc, Equal, origin(), structValue, m_currentBlock->appendNew<Const64Value>(m_proc, origin(), JSValue::encode(jsNull()))));
check->setGenerator([=, this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
this->emitExceptionCheck(jit, ExceptionType::BadStructNew);
});
}

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
for (StructFieldCount i = 0; i < structType.fieldCount(); ++i) {
Value* initValue;
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
Expand Up @@ -1417,14 +1417,14 @@ void BBQJIT::pushArrayNewFromSegment(arraySegmentOperation operation, uint32_t t

PartialResult WARN_UNUSED_RETURN BBQJIT::addArrayNewData(uint32_t typeIndex, uint32_t dataIndex, ExpressionType arraySize, ExpressionType offset, ExpressionType& result)
{
pushArrayNewFromSegment(operationWasmArrayNewData, typeIndex, dataIndex, arraySize, offset, ExceptionType::OutOfBoundsDataSegmentAccess, result);
pushArrayNewFromSegment(operationWasmArrayNewData, typeIndex, dataIndex, arraySize, offset, ExceptionType::BadArrayNewInitData, result);
LOG_INSTRUCTION("ArrayNewData", typeIndex, dataIndex, arraySize, offset, RESULT(result));
return { };
}

PartialResult WARN_UNUSED_RETURN BBQJIT::addArrayNewElem(uint32_t typeIndex, uint32_t elemSegmentIndex, ExpressionType arraySize, ExpressionType offset, ExpressionType& result)
{
pushArrayNewFromSegment(operationWasmArrayNewElem, typeIndex, elemSegmentIndex, arraySize, offset, ExceptionType::OutOfBoundsElementSegmentAccess, result);
pushArrayNewFromSegment(operationWasmArrayNewElem, typeIndex, elemSegmentIndex, arraySize, offset, ExceptionType::BadArrayNewInitElem, result);
LOG_INSTRUCTION("ArrayNewElem", typeIndex, elemSegmentIndex, arraySize, offset, RESULT(result));
return { };
}
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp
Expand Up @@ -1955,10 +1955,9 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addStructNewDefault(uint32_t typeIndex,
result = topValue(TypeKind::I64);
emitCCall(operationWasmStructNewEmpty, arguments, result);

// FIXME: What about OOM?

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location structLocation = allocate(result);
emitThrowOnNullReference(ExceptionType::BadStructNew, structLocation);
m_jit.loadPtr(MacroAssembler::Address(structLocation.asGPRlo(), JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
for (StructFieldCount i = 0; i < structType.fieldCount(); ++i) {
if (Wasm::isRefType(structType.field(i).type))
Expand Down Expand Up @@ -1986,6 +1985,7 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addStructNew(uint32_t typeIndex, Vector

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location structLocation = allocate(allocationResult);
emitThrowOnNullReference(ExceptionType::BadStructNew, structLocation);
m_jit.loadPtr(MacroAssembler::Address(structLocation.asGPRlo(), JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
bool hasRefTypeField = false;
for (uint32_t i = 0; i < args.size(); ++i) {
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp
Expand Up @@ -2132,10 +2132,9 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addStructNewDefault(uint32_t typeIndex,
result = topValue(TypeKind::I64);
emitCCall(operationWasmStructNewEmpty, arguments, result);

// FIXME: What about OOM?

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location structLocation = allocate(result);
emitThrowOnNullReference(ExceptionType::BadStructNew, structLocation);
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))
Expand Down Expand Up @@ -2166,6 +2165,7 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addStructNew(uint32_t typeIndex, Vector

const auto& structType = *m_info.typeSignatures[typeIndex]->expand().template as<StructType>();
Location structLocation = allocate(allocationResult);
emitThrowOnNullReference(ExceptionType::BadStructNew, structLocation);
m_jit.loadPtr(MacroAssembler::Address(structLocation.asGPR(), JSWebAssemblyStruct::offsetOfPayload()), wasmScratchGPR);
bool hasRefTypeField = false;
for (uint32_t i = 0; i < args.size(); ++i) {
Expand Down

0 comments on commit 6b7295e

Please sign in to comment.