Commit 64b59264 authored by evih's avatar evih Committed by Commit Bot

[wasm] Reverse the evaluation of params in js-to-wasm generic wrapper

The parameters should be processed and evaluated in an increasing order
(starting with the 1st param).
Before we started with the last (n-th) parameter which was not correct.

Bug: v8:10701, chromium:1124940
Change-Id: I8e0d8b1f0c53832c8f2d09551879c1a4413e1598
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2396085Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Eva Herencsárová <evih@google.com>
Cr-Commit-Position: refs/heads/master@{#69739}
parent 5286799c
......@@ -3341,7 +3341,9 @@ void Builtins::Generate_GenericJSToWasmWrapper(MacroAssembler* masm) {
// pointer.
__ subq(rsp, Immediate(5 * kSystemPointerSize));
// Looping through the params, starting with the n-th param.
// Looping through the params, starting with the 1st param.
// The order of processing the params is important. We have to evaluate them
// in an increasing order.
// Not reversed Reversed
// +-----------------+------+-----------------+---------------
// | receiver | | param n |
......@@ -3357,22 +3359,38 @@ void Builtins::Generate_GenericJSToWasmWrapper(MacroAssembler* masm) {
// | rbp | | rbp | Spill slots
// |- - - - - - - - -| |- - - - - - - - -|
//
// [rbp + current_param] gives us the parameter we are processing
// We iterate through half-open interval <n-th param, [rbp + param_limit])
// [rbp + current_param] gives us the parameter we are processing.
// We iterate through half-open interval <1st param, [rbp + param_limit]).
// The Wasm function expects that the params can be popped from the top of the
// stack in an increasing order. As we have to process the params in an
// increasing order too, we have to reserve param_count slots where we will
// move the processed parameters so that they can be popped in an increasing
// order.
Register js_arguments_size_in_bytes = kJavaScriptCallArgCountRegister;
__ shlq(js_arguments_size_in_bytes, Immediate(kSystemPointerSizeLog2));
__ subq(rsp, js_arguments_size_in_bytes);
js_arguments_size_in_bytes = no_reg;
Register current_param_slot = r8;
__ movq(current_param_slot, rsp);
Register current_param = r14;
Register param_limit = r15;
#ifdef V8_REVERSE_JSARGS
__ movq(current_param, param_count);
__ shlq(current_param, Immediate(kSystemPointerSizeLog2));
__ addq(current_param, Immediate(kFPOnStackSize + kPCOnStackSize));
__ movq(param_limit, Immediate(kFPOnStackSize + kPCOnStackSize));
const int increment = -kSystemPointerSize;
#else
__ movq(current_param, Immediate(kFPOnStackSize + kPCOnStackSize));
constexpr int kReceiverOnStackSize = kSystemPointerSize;
__ movq(current_param,
Immediate(kFPOnStackSize + kPCOnStackSize + kReceiverOnStackSize));
__ movq(param_limit, param_count);
__ shlq(param_limit, Immediate(kSystemPointerSizeLog2));
__ addq(param_limit, Immediate(kFPOnStackSize + kPCOnStackSize));
__ addq(param_limit,
Immediate(kFPOnStackSize + kPCOnStackSize + kReceiverOnStackSize));
const int increment = kSystemPointerSize;
#else
__ movq(current_param, param_count);
__ shlq(current_param, Immediate(kSystemPointerSizeLog2));
__ addq(current_param, Immediate(kFPOnStackSize));
__ movq(param_limit, Immediate(kFPOnStackSize));
const int increment = -kSystemPointerSize;
#endif
Register param = rax;
Label loop;
......@@ -3390,7 +3408,8 @@ void Builtins::Generate_GenericJSToWasmWrapper(MacroAssembler* masm) {
Label param_conversion_done;
__ bind(&param_conversion_done);
__ pushq(param);
__ movq(MemOperand(current_param_slot, 0), param);
__ addq(current_param_slot, Immediate(kSystemPointerSize));
__ cmpq(current_param, param_limit);
__ j(not_equal, &loop);
......@@ -3502,6 +3521,7 @@ void Builtins::Generate_GenericJSToWasmWrapper(MacroAssembler* masm) {
Immediate(kBuiltinCallGCScanSlotCount));
__ pushq(current_param);
__ pushq(param_limit);
__ pushq(current_param_slot);
__ pushq(wasm_instance);
__ pushq(function_data);
__ LoadAnyTaggedField(
......@@ -3515,6 +3535,7 @@ void Builtins::Generate_GenericJSToWasmWrapper(MacroAssembler* masm) {
__ popq(function_data);
__ popq(wasm_instance);
__ popq(current_param_slot);
__ popq(param_limit);
__ popq(current_param);
__ movq(param_count, MemOperand(rbp, kParamCountOffset));
......
......@@ -127,6 +127,18 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
let instance = builder.instantiate({ mod: { func: import_func } });
assertEquals(undefined, instance.exports.main(9, param2, param3, 0));
assertEquals(60, x);
// Now we test if the evaluation order of the parameters is correct.
x = 12;
param3 = {
valueOf: () => {
Object.defineProperty(param2, 'valueOf', {
value: () => 30
})
return 3;
}
};
assertEquals(undefined, instance.exports.main(9, param2, param3, 0));
assertEquals(60, x);
})();
let kSig_v_iiiiiiii = makeSig([kWasmI32, kWasmI32, kWasmI32, kWasmI32,
......
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