Commit 704c571d authored by Thibaud Michaud's avatar Thibaud Michaud Committed by V8 LUCI CQ

[wasm] Trap on invalid suspender object

Trap if the suspender argument provided to the JSPI import
wrapper is invalid.

For now, the suspender argument is expected to be the active
suspender. In the future, it will also be possible to suspend
to a parent of the current suspender. This will only be possible
once wasm-to-wasm suspending wrappers are supported, or if and
when JSPI suspenders become compatible with their core
stack-switching counterpart (e.g. Fibers in the fiber proposal).

R=jkummerow@chromium.org

Bug: v8:12191
Change-Id: I650454ed076bd251b0aa18656774d4c4b2d3bfdc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3892697Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83173}
parent 2847ad2e
...@@ -3971,11 +3971,6 @@ void Builtins::Generate_WasmSuspend(MacroAssembler* masm) { ...@@ -3971,11 +3971,6 @@ void Builtins::Generate_WasmSuspend(MacroAssembler* masm) {
__ subq(rsp, Immediate(-(BuiltinWasmWrapperConstants::kGCScanSlotCountOffset - __ subq(rsp, Immediate(-(BuiltinWasmWrapperConstants::kGCScanSlotCountOffset -
TypedFrameConstants::kFixedFrameSizeFromFp))); TypedFrameConstants::kFixedFrameSizeFromFp)));
// TODO(thibaudm): Throw if any of the following holds:
// - caller is null
// - ActiveSuspender is undefined
// - 'suspender' is not the active suspender
// ------------------------------------------- // -------------------------------------------
// Save current state in active jump buffer. // Save current state in active jump buffer.
// ------------------------------------------- // -------------------------------------------
......
...@@ -654,6 +654,7 @@ namespace internal { ...@@ -654,6 +654,7 @@ namespace internal {
T(WasmTrapStringInvalidUtf8, "invalid UTF-8 string") \ T(WasmTrapStringInvalidUtf8, "invalid UTF-8 string") \
T(WasmTrapStringInvalidWtf8, "invalid WTF-8 string") \ T(WasmTrapStringInvalidWtf8, "invalid WTF-8 string") \
T(WasmTrapStringOffsetOutOfBounds, "string offset out of bounds") \ T(WasmTrapStringOffsetOutOfBounds, "string offset out of bounds") \
T(WasmTrapBadSuspender, "invalid suspender object for suspend") \
T(WasmTrapStringIsolatedSurrogate, \ T(WasmTrapStringIsolatedSurrogate, \
"Failed to encode string as UTF-8: contains unpaired surrogate") \ "Failed to encode string as UTF-8: contains unpaired surrogate") \
T(WasmExceptionError, "wasm exception") \ T(WasmExceptionError, "wasm exception") \
......
...@@ -6891,12 +6891,20 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { ...@@ -6891,12 +6891,20 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
// If value is a promise, suspend to the js-to-wasm prompt, and resume later // If value is a promise, suspend to the js-to-wasm prompt, and resume later
// with the promise's resolved value. // with the promise's resolved value.
auto resume = gasm_->MakeLabel(MachineRepresentation::kTagged); auto resume = gasm_->MakeLabel(MachineRepresentation::kTagged);
gasm_->GotoIf(IsSmi(value), &resume, value); // Trap if the suspender argument is not the active suspender or if there is
gasm_->GotoIfNot(gasm_->HasInstanceType(value, JS_PROMISE_TYPE), &resume, // no active suspender.
BranchHint::kTrue, value); auto bad_suspender = gasm_->MakeDeferredLabel();
Node* native_context = gasm_->Load( Node* native_context = gasm_->Load(
MachineType::TaggedPointer(), api_function_ref, MachineType::TaggedPointer(), api_function_ref,
wasm::ObjectAccess::ToTagged(WasmApiFunctionRef::kNativeContextOffset)); wasm::ObjectAccess::ToTagged(WasmApiFunctionRef::kNativeContextOffset));
Node* active_suspender = LOAD_ROOT(ActiveSuspender, active_suspender);
gasm_->GotoIf(gasm_->TaggedEqual(active_suspender, UndefinedValue()),
&bad_suspender, BranchHint::kFalse);
gasm_->GotoIfNot(gasm_->TaggedEqual(suspender, active_suspender),
&bad_suspender, BranchHint::kFalse);
gasm_->GotoIf(IsSmi(value), &resume, value);
gasm_->GotoIfNot(gasm_->HasInstanceType(value, JS_PROMISE_TYPE), &resume,
BranchHint::kTrue, value);
auto* call_descriptor = GetBuiltinCallDescriptor( auto* call_descriptor = GetBuiltinCallDescriptor(
Builtin::kWasmSuspend, zone_, StubCallMode::kCallWasmRuntimeStub); Builtin::kWasmSuspend, zone_, StubCallMode::kCallWasmRuntimeStub);
Node* call_target = mcgraph()->RelocatableIntPtrConstant( Node* call_target = mcgraph()->RelocatableIntPtrConstant(
...@@ -6907,6 +6915,10 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { ...@@ -6907,6 +6915,10 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
Node* resolved = Node* resolved =
gasm_->Call(call_descriptor, call_target, chained_promise, suspender); gasm_->Call(call_descriptor, call_target, chained_promise, suspender);
gasm_->Goto(&resume, resolved); gasm_->Goto(&resume, resolved);
gasm_->Bind(&bad_suspender);
BuildCallToRuntimeWithContext(Runtime::kThrowBadSuspenderError,
native_context, nullptr, 0);
TerminateThrow(effect(), control());
gasm_->Bind(&resume); gasm_->Bind(&resume);
return resume.PhiAt(0); return resume.PhiAt(0);
} }
......
...@@ -194,6 +194,15 @@ RUNTIME_FUNCTION(Runtime_WasmThrowJSTypeError) { ...@@ -194,6 +194,15 @@ RUNTIME_FUNCTION(Runtime_WasmThrowJSTypeError) {
isolate, NewTypeError(MessageTemplate::kWasmTrapJSTypeError)); isolate, NewTypeError(MessageTemplate::kWasmTrapJSTypeError));
} }
// This error is thrown from a wasm-to-JS wrapper, so unlike
// Runtime_ThrowWasmError, this function does not check or unset the
// thread-in-wasm flag.
RUNTIME_FUNCTION(Runtime_ThrowBadSuspenderError) {
HandleScope scope(isolate);
DCHECK_EQ(0, args.length());
return ThrowWasmError(isolate, MessageTemplate::kWasmTrapBadSuspender);
}
RUNTIME_FUNCTION(Runtime_WasmThrow) { RUNTIME_FUNCTION(Runtime_WasmThrow) {
ClearThreadInWasmScope clear_wasm_flag(isolate); ClearThreadInWasmScope clear_wasm_flag(isolate);
HandleScope scope(isolate); HandleScope scope(isolate);
......
...@@ -598,6 +598,7 @@ namespace internal { ...@@ -598,6 +598,7 @@ namespace internal {
F(TypedArraySortFast, 1, 1) F(TypedArraySortFast, 1, 1)
#define FOR_EACH_INTRINSIC_WASM(F, I) \ #define FOR_EACH_INTRINSIC_WASM(F, I) \
F(ThrowBadSuspenderError, 0, 1) \
F(ThrowWasmError, 1, 1) \ F(ThrowWasmError, 1, 1) \
F(ThrowWasmStackOverflow, 0, 1) \ F(ThrowWasmStackOverflow, 0, 1) \
F(WasmI32AtomicWait, 4, 1) \ F(WasmI32AtomicWait, 4, 1) \
......
...@@ -495,3 +495,29 @@ function TestNestedSuspenders(suspend) { ...@@ -495,3 +495,29 @@ function TestNestedSuspenders(suspend) {
let wrapper = ToPromising(instance.exports.test); let wrapper = ToPromising(instance.exports.test);
assertThrows(wrapper, RangeError, /Maximum call stack size exceeded/); assertThrows(wrapper, RangeError, /Maximum call stack size exceeded/);
})(); })();
(function TestBadSuspender() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let import_index = builder.addImport('m', 'import', kSig_i_r);
builder.addFunction("test", kSig_i_r)
.addBody([
kExprLocalGet, 0,
kExprCallFunction, import_index, // suspend
]).exportFunc();
builder.addFunction("return_suspender", kSig_r_r)
.addBody([
kExprLocalGet, 0
]).exportFunc();
let js_import = new WebAssembly.Function(
{parameters: ['externref'], results: ['i32']},
() => Promise.resolve(42),
{suspending: 'first'});
let instance = builder.instantiate({m: {import: js_import}});
let suspender = ToPromising(instance.exports.return_suspender)();
for (s of [suspender, null, undefined, {}]) {
assertThrows(() => instance.exports.test(s),
WebAssembly.RuntimeError,
/invalid suspender object for suspend/);
}
})();
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