Commit 62b4d3c1 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by V8 LUCI CQ

[wasm] Fix ReturnPromiseOnSuspend frame visiting

Add a test where the GC gets called during parameter conversion, and fix
two related issues:
- Reorder spilled references so that they are at the top of the stack
  before the builtin call
- Add the missing frame marker on the new stack

R=ahaas@chromium.org

Bug: v8:12191
Change-Id: I3f68c675123c726543df6942d110fe06bc6c0efb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780530
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81873}
parent ce570aa5
...@@ -2932,12 +2932,12 @@ void PrepareForBuiltinCall(MacroAssembler* masm, MemOperand GCScanSlotPlace, ...@@ -2932,12 +2932,12 @@ void PrepareForBuiltinCall(MacroAssembler* masm, MemOperand GCScanSlotPlace,
__ pushq(current_int_param_slot); __ pushq(current_int_param_slot);
__ pushq(current_float_param_slot); __ pushq(current_float_param_slot);
__ pushq(valuetypes_array_ptr); __ pushq(valuetypes_array_ptr);
__ pushq(wasm_instance);
__ pushq(function_data);
if (original_fp != rbp) { if (original_fp != rbp) {
// For stack-switching only: keep the pointer to the parent stack alive. // For stack-switching only: keep the pointer to the parent stack alive.
__ pushq(original_fp); __ pushq(original_fp);
} }
__ pushq(wasm_instance);
__ pushq(function_data);
// We had to prepare the parameters for the Call: we have to put the context // We had to prepare the parameters for the Call: we have to put the context
// into rsi. // into rsi.
__ LoadAnyTaggedField( __ LoadAnyTaggedField(
...@@ -2955,11 +2955,11 @@ void RestoreAfterBuiltinCall(MacroAssembler* masm, Register function_data, ...@@ -2955,11 +2955,11 @@ void RestoreAfterBuiltinCall(MacroAssembler* masm, Register function_data,
Register original_fp) { Register original_fp) {
// Pop and load values from the stack in order into the registers after // Pop and load values from the stack in order into the registers after
// builtin calls for the GenericJSToWasmWrapper. // builtin calls for the GenericJSToWasmWrapper.
__ popq(function_data);
__ popq(wasm_instance);
if (original_fp != rbp) { if (original_fp != rbp) {
__ popq(original_fp); __ popq(original_fp);
} }
__ popq(function_data);
__ popq(wasm_instance);
__ popq(valuetypes_array_ptr); __ popq(valuetypes_array_ptr);
__ popq(current_float_param_slot); __ popq(current_float_param_slot);
__ popq(current_int_param_slot); __ popq(current_int_param_slot);
...@@ -3240,9 +3240,13 @@ void GenericJSToWasmWrapperHelper(MacroAssembler* masm, bool stack_switch) { ...@@ -3240,9 +3240,13 @@ void GenericJSToWasmWrapperHelper(MacroAssembler* masm, bool stack_switch) {
// old stack: new stack: // old stack: new stack:
// +-----------------+ // +-----------------+
// | <parent frame> | // | <parent frame> |
// r9-> +-----------------+ +-----------------+ // +-----------------+
// | <fixed> | | 0 (jmpbuf rbp) | // | pc |
// +-----------------+ rbp-> +-----------------+ // +-----------------+ +-----------------+
// | caller rbp | | 0 (jmpbuf rbp) |
// r9-> +-----------------+ rbp-> +-----------------+
// | frame marker | | frame marker |
// +-----------------+ +-----------------+
// |kGCScanSlotCount | |kGCScanSlotCount | // |kGCScanSlotCount | |kGCScanSlotCount |
// +-----------------+ +-----------------+ // +-----------------+ +-----------------+
// | kParamCount | | / | // | kParamCount | | / |
...@@ -3277,9 +3281,8 @@ void GenericJSToWasmWrapperHelper(MacroAssembler* masm, bool stack_switch) { ...@@ -3277,9 +3281,8 @@ void GenericJSToWasmWrapperHelper(MacroAssembler* masm, bool stack_switch) {
// Push the loaded rbp. We know it is null, because there is no frame yet, // Push the loaded rbp. We know it is null, because there is no frame yet,
// so we could also push 0 directly. In any case we need to push it, because // so we could also push 0 directly. In any case we need to push it, because
// this marks the base of the stack segment for the stack frame iterator. // this marks the base of the stack segment for the stack frame iterator.
__ pushq(rbp); __ EnterFrame(StackFrame::STACK_SWITCH);
__ movq(rbp, rsp); __ subq(rsp, Immediate(kNumSpillSlots * kSystemPointerSize));
__ addq(rsp, Immediate(kLastSpillOffset));
} }
Register original_fp = stack_switch ? r9 : rbp; Register original_fp = stack_switch ? r9 : rbp;
......
...@@ -213,6 +213,29 @@ load("test/mjsunit/wasm/wasm-module-builder.js"); ...@@ -213,6 +213,29 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
wrapper(suspender); wrapper(suspender);
})(); })();
// Call the GC during param conversion.
(function TestStackSwitchGC2() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let sig = makeSig([kWasmAnyRef, kWasmI32], [kWasmI32]);
let import_index = builder.addImport('m', 'import', sig);
builder.addFunction("test", sig)
.addBody([
kExprLocalGet, 0,
kExprLocalGet, 1,
kExprCallFunction, import_index,
]).exportFunc();
let js_import = WebAssembly.suspendOnReturnedPromise(
new WebAssembly.Function(
{parameters: ['i32'], results: ['externref']},
(v) => { return Promise.resolve(v) }));
let instance = builder.instantiate({'m': {'import': js_import}});
let wrapper = WebAssembly.returnPromiseOnSuspend(instance.exports.test);
let arg = { valueOf: () => { gc(); return 24; } };
let suspender = new WebAssembly.Suspender();
wrapper(suspender, arg).then((v) => assertEquals(arg.valueOf(), v));
})();
// Check that the suspender does not suspend if the import's // Check that the suspender does not suspend if the import's
// return value is not a promise. // return value is not a promise.
(function TestStackSwitchNoPromise() { (function TestStackSwitchNoPromise() {
......
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