[Liftoff] Zero-extend i32 stack parameters
i32 stack parameters can be loaded by Turbofan as 64-bit value, hence they would not be zero extended. If this loaded value is then passed to Liftoff (which assumes zero-extended i32 values), we could use it for memory accesses, which would be out of bounds. R=mstarzinger@chromium.org Bug: chromium:864509, v8:6600 Change-Id: I0f45a269b1fb1c2befc2e6bc660c559a88323767 Reviewed-on: https://chromium-review.googlesource.com/1140168 Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#54500}
This commit is contained in:
parent
e3a5b1e402
commit
16af1baac4
@ -1452,7 +1452,18 @@ void LiftoffStackSlots::Construct() {
|
|||||||
const LiftoffAssembler::VarState& src = slot.src_;
|
const LiftoffAssembler::VarState& src = slot.src_;
|
||||||
switch (src.loc()) {
|
switch (src.loc()) {
|
||||||
case LiftoffAssembler::VarState::kStack:
|
case LiftoffAssembler::VarState::kStack:
|
||||||
asm_->pushq(liftoff::GetStackSlot(slot.src_index_));
|
if (src.type() == kWasmI32) {
|
||||||
|
// Load i32 values to a register first to ensure they are zero
|
||||||
|
// extended.
|
||||||
|
asm_->movl(kScratchRegister, liftoff::GetStackSlot(slot.src_index_));
|
||||||
|
asm_->pushq(kScratchRegister);
|
||||||
|
} else {
|
||||||
|
// For all other types, just push the whole (8-byte) stack slot.
|
||||||
|
// This is also ok for f32 values (even though we copy 4 uninitialized
|
||||||
|
// bytes), because f32 and f64 values are clearly distinguished in
|
||||||
|
// Turbofan, so the uninitialized bytes are never accessed.
|
||||||
|
asm_->pushq(liftoff::GetStackSlot(slot.src_index_));
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
case LiftoffAssembler::VarState::kRegister:
|
case LiftoffAssembler::VarState::kRegister:
|
||||||
liftoff::push(asm_, src.reg(), src.type());
|
liftoff::push(asm_, src.reg(), src.type());
|
||||||
|
62
test/mjsunit/regress/wasm/regress-864509.js
Normal file
62
test/mjsunit/regress/wasm/regress-864509.js
Normal file
@ -0,0 +1,62 @@
|
|||||||
|
// Copyright 2018 the V8 project authors. All rights reserved.
|
||||||
|
// Use of this source code is governed by a BSD-style license that can be
|
||||||
|
// found in the LICENSE file.
|
||||||
|
|
||||||
|
// Flags: --liftoff --no-wasm-tier-up --no-future --wasm-tier-mask-for-testing=2
|
||||||
|
|
||||||
|
load('test/mjsunit/wasm/wasm-constants.js');
|
||||||
|
load('test/mjsunit/wasm/wasm-module-builder.js');
|
||||||
|
|
||||||
|
const builder = new WasmModuleBuilder();
|
||||||
|
builder.addMemory(1, 1);
|
||||||
|
// First function is Liftoff. The first parameter is used as memory offset.
|
||||||
|
builder.addFunction(undefined, kSig_v_i).addBody([
|
||||||
|
kExprGetLocal, 0, // get_local 0
|
||||||
|
kExprI32Const, 0, // i32.const 0
|
||||||
|
kExprI32StoreMem, 0, 0, // i32.store offset=0
|
||||||
|
]);
|
||||||
|
// Second function is Turbofan. It loads the sixth parameter from the stack
|
||||||
|
// into a register for the first argument. Even though it's a 32-bit value, it
|
||||||
|
// is loaded as 64-bit value on x64.
|
||||||
|
builder.addFunction(undefined, makeSig(new Array(6).fill(kWasmI32), []))
|
||||||
|
.addBody([
|
||||||
|
kExprGetLocal, 5, // get_local 5
|
||||||
|
kExprCallFunction, 0 // call 0
|
||||||
|
]);
|
||||||
|
// The third function is Liftoff again. A value is spilled on the stack as i32,
|
||||||
|
// then used as a call argument, passed via the stack. The full 64-bit are
|
||||||
|
// copied on the stack, even though just 32-bit were written before. Hence, the
|
||||||
|
// stack slot is not zero-extended.
|
||||||
|
const gen_i32_code = [
|
||||||
|
kExprTeeLocal, 0, // tee_local 0
|
||||||
|
kExprGetLocal, 0, // get_local 0
|
||||||
|
kExprI32Const, 1, // i32.const 1
|
||||||
|
kExprI32Add // i32.add --> 2nd param
|
||||||
|
];
|
||||||
|
builder.addFunction(undefined, kSig_v_v).addLocals({i32_count: 1}).addBody([
|
||||||
|
// Generate six values on the stack, then six more to force the other six on
|
||||||
|
// the stack.
|
||||||
|
...wasmI32Const(0), // i32.const 0
|
||||||
|
...wasmI32Const(1), // i32.const 1
|
||||||
|
kExprI32Add, // i32.add --> 1st param
|
||||||
|
...gen_i32_code, // --> 2nd param
|
||||||
|
...gen_i32_code, // --> 3rd param
|
||||||
|
...gen_i32_code, // --> 4th param
|
||||||
|
...gen_i32_code, // --> 5th param
|
||||||
|
...gen_i32_code, // --> 6th param
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
...gen_i32_code, // --> garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprDrop, // drop garbage
|
||||||
|
kExprCallFunction, 1 // call 1
|
||||||
|
]).exportAs('three');
|
||||||
|
const instance = builder.instantiate();
|
||||||
|
instance.exports.three();
|
Loading…
Reference in New Issue
Block a user