From 1e25816ef95536587330514d128147495d9b954e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 08:19:50 +0200 Subject: [PATCH 1/7] deps: V8: cherry-pick cb4faa902e9f Original commit message: Reland "[liftoff][arm64] Use 64 bit offset reg in mem op" This is a reland of f645d0b857bc669271adcbe95cf25e1554347dd4 The issue was that converting an i64 to an i32 didn't clear the upper bits on arm64. This was not necessary before because we did the zero extension as part of the load operand, but this is required now that we use the full register. Original change's description: > [liftoff][arm64] Use 64 bit offset reg in mem op > > Accessing the Wasm memory with a 64 bit offset was truncated to 32 bit, > which is fine if we check bounds first, but not if we rely on the > trap handler to catch the OOB. > > R=clemensb@chromium.org > > Bug: v8:11587 > Change-Id: I82a3a2906e55d9d640c30e770a5c93532e3a442c > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2808942 > Reviewed-by: Clemens Backes > Commit-Queue: Thibaud Michaud > Cr-Commit-Position: refs/heads/master@{#73829} Bug: v8:11587 Change-Id: Ibc182475745c6f697a0ba6d75c260b74ddf8fe52 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2810846 Reviewed-by: Clemens Backes Commit-Queue: Thibaud Michaud Cr-Commit-Position: refs/heads/master@{#73853} Refs: https://github.com/v8/v8/commit/cb4faa902e9f9fe848b46fbe8047f70ad4a54971 --- common.gypi | 2 +- deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common.gypi b/common.gypi index b2ea540133f0af..aa42c69f96391b 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.13', + 'v8_embedder_string': '-node.14', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h index 39ef8528e5267a..af794f9a9d5e66 100644 --- a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h +++ b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h @@ -128,7 +128,7 @@ inline MemOperand GetMemOp(LiftoffAssembler* assm, UseScratchRegisterScope* temps, Register addr, Register offset, T offset_imm) { if (offset.is_valid()) { - if (offset_imm == 0) return MemOperand(addr.X(), offset.W(), UXTW); + if (offset_imm == 0) return MemOperand(addr.X(), offset.X()); Register tmp = temps->AcquireX(); DCHECK_GE(kMaxUInt32, offset_imm); assm->Add(tmp, offset.X(), offset_imm); @@ -1333,7 +1333,7 @@ bool LiftoffAssembler::emit_type_conversion(WasmOpcode opcode, LiftoffRegister src, Label* trap) { switch (opcode) { case kExprI32ConvertI64: - if (src != dst) Mov(dst.gp().W(), src.gp().W()); + Mov(dst.gp().W(), src.gp().W()); return true; case kExprI32SConvertF32: Fcvtzs(dst.gp().W(), src.fp().S()); // f32 -> i32 round to zero. From bb076a365da5b3a22c96d5d994d1a0e10cbfe7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 08:23:03 +0200 Subject: [PATCH 2/7] deps: V8: cherry-pick 53784bdb8f01 Original commit message: [liftoff] Handle constant memory indexes specially This adds detection for constant memory indexes which can statically be proven to be in-bounds (because the effective offset is within the minimum memory size). In these cases, we can skip the bounds check and the out-of-line code for the trap-handler. This often saves 1-2% of code size. R=ahaas@chromium.org Bug: v8:11802 Change-Id: I0ee094e6f1f5d132af1d6a8a7c539a4af6c3cb5e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2919827 Commit-Queue: Clemens Backes Reviewed-by: Andreas Haas Cr-Commit-Position: refs/heads/master@{#74825} Refs: https://github.com/v8/v8/commit/53784bdb8f01a6ff76fc3acd3aec4d605cb3bfcc --- common.gypi | 2 +- deps/v8/src/wasm/baseline/liftoff-compiler.cc | 140 ++++++++++++------ 2 files changed, 99 insertions(+), 43 deletions(-) diff --git a/common.gypi b/common.gypi index aa42c69f96391b..a7c109075f645c 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.14', + 'v8_embedder_string': '-node.15', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/liftoff-compiler.cc b/deps/v8/src/wasm/baseline/liftoff-compiler.cc index a26df17225204f..c4b6cbdac28d21 100644 --- a/deps/v8/src/wasm/baseline/liftoff-compiler.cc +++ b/deps/v8/src/wasm/baseline/liftoff-compiler.cc @@ -2767,33 +2767,73 @@ class LiftoffCompiler { return index; } + bool IndexStaticallyInBounds(const LiftoffAssembler::VarState& index_slot, + int access_size, uintptr_t* offset) { + if (!index_slot.is_const()) return false; + + // Potentially zero extend index (which is a 32-bit constant). + const uintptr_t index = static_cast(index_slot.i32_const()); + const uintptr_t effective_offset = index + *offset; + + if (effective_offset < index // overflow + || !base::IsInBounds(effective_offset, access_size, + env_->min_memory_size)) { + return false; + } + + *offset = effective_offset; + return true; + } + void LoadMem(FullDecoder* decoder, LoadType type, const MemoryAccessImmediate& imm, const Value& index_val, Value* result) { ValueKind kind = type.value_type().kind(); + RegClass rc = reg_class_for(kind); if (!CheckSupportedType(decoder, kind, "load")) return; - LiftoffRegister full_index = __ PopToRegister(); - Register index = BoundsCheckMem(decoder, type.size(), imm.offset, - full_index, {}, kDontForceCheck); - if (index == no_reg) return; uintptr_t offset = imm.offset; - LiftoffRegList pinned = LiftoffRegList::ForRegs(index); - index = AddMemoryMasking(index, &offset, &pinned); - DEBUG_CODE_COMMENT("load from memory"); - Register addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); - LOAD_INSTANCE_FIELD(addr, MemoryStart, kSystemPointerSize, pinned); - RegClass rc = reg_class_for(kind); - LiftoffRegister value = pinned.set(__ GetUnusedRegister(rc, pinned)); - uint32_t protected_load_pc = 0; - __ Load(value, addr, index, offset, type, pinned, &protected_load_pc, true); - if (env_->use_trap_handler) { - AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, - protected_load_pc); + Register index = no_reg; + + // Only look at the slot, do not pop it yet (will happen in PopToRegister + // below, if this is not a statically-in-bounds index). + auto& index_slot = __ cache_state()->stack_state.back(); + if (IndexStaticallyInBounds(index_slot, type.size(), &offset)) { + __ cache_state()->stack_state.pop_back(); + DEBUG_CODE_COMMENT("load from memory (constant offset)"); + LiftoffRegList pinned; + Register mem = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); + LOAD_INSTANCE_FIELD(mem, MemoryStart, kSystemPointerSize, pinned); + LiftoffRegister value = pinned.set(__ GetUnusedRegister(rc, pinned)); + __ Load(value, mem, no_reg, offset, type, pinned, nullptr, true); + __ PushRegister(kind, value); + } else { + LiftoffRegister full_index = __ PopToRegister(); + index = BoundsCheckMem(decoder, type.size(), offset, full_index, {}, + kDontForceCheck); + if (index == no_reg) return; + + DEBUG_CODE_COMMENT("load from memory"); + LiftoffRegList pinned = LiftoffRegList::ForRegs(index); + index = AddMemoryMasking(index, &offset, &pinned); + + // Load the memory start address only now to reduce register pressure + // (important on ia32). + Register mem = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); + LOAD_INSTANCE_FIELD(mem, MemoryStart, kSystemPointerSize, pinned); + LiftoffRegister value = pinned.set(__ GetUnusedRegister(rc, pinned)); + + uint32_t protected_load_pc = 0; + __ Load(value, mem, index, offset, type, pinned, &protected_load_pc, + true); + if (env_->use_trap_handler) { + AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, + protected_load_pc); + } + __ PushRegister(kind, value); } - __ PushRegister(kind, value); - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(false, type.mem_type().representation(), index, offset, decoder->position()); } @@ -2836,7 +2876,7 @@ class LiftoffCompiler { } __ PushRegister(kS128, value); - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { // Again load extend is different. MachineRepresentation mem_rep = transform == LoadTransformationKind::kExtend @@ -2878,7 +2918,7 @@ class LiftoffCompiler { __ PushRegister(kS128, result); - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(false, type.mem_type().representation(), index, offset, decoder->position()); } @@ -2889,29 +2929,45 @@ class LiftoffCompiler { const Value& index_val, const Value& value_val) { ValueKind kind = type.value_type().kind(); if (!CheckSupportedType(decoder, kind, "store")) return; + LiftoffRegList pinned; LiftoffRegister value = pinned.set(__ PopToRegister()); - LiftoffRegister full_index = __ PopToRegister(pinned); - Register index = BoundsCheckMem(decoder, type.size(), imm.offset, - full_index, pinned, kDontForceCheck); - if (index == no_reg) return; uintptr_t offset = imm.offset; - pinned.set(index); - index = AddMemoryMasking(index, &offset, &pinned); - DEBUG_CODE_COMMENT("store to memory"); - Register addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); - LOAD_INSTANCE_FIELD(addr, MemoryStart, kSystemPointerSize, pinned); - uint32_t protected_store_pc = 0; - LiftoffRegList outer_pinned; - if (FLAG_trace_wasm_memory) outer_pinned.set(index); - __ Store(addr, index, offset, value, type, outer_pinned, - &protected_store_pc, true); - if (env_->use_trap_handler) { - AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, - protected_store_pc); + Register index = no_reg; + + auto& index_slot = __ cache_state()->stack_state.back(); + if (IndexStaticallyInBounds(index_slot, type.size(), &offset)) { + __ cache_state()->stack_state.pop_back(); + DEBUG_CODE_COMMENT("store to memory (constant offset)"); + Register mem = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); + LOAD_INSTANCE_FIELD(mem, MemoryStart, kSystemPointerSize, pinned); + __ Store(mem, no_reg, offset, value, type, pinned, nullptr, true); + } else { + LiftoffRegister full_index = __ PopToRegister(pinned); + index = BoundsCheckMem(decoder, type.size(), imm.offset, full_index, + pinned, kDontForceCheck); + if (index == no_reg) return; + + pinned.set(index); + index = AddMemoryMasking(index, &offset, &pinned); + DEBUG_CODE_COMMENT("store to memory"); + uint32_t protected_store_pc = 0; + // Load the memory start address only now to reduce register pressure + // (important on ia32). + Register mem = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); + LOAD_INSTANCE_FIELD(mem, MemoryStart, kSystemPointerSize, pinned); + LiftoffRegList outer_pinned; + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) outer_pinned.set(index); + __ Store(mem, index, offset, value, type, outer_pinned, + &protected_store_pc, true); + if (env_->use_trap_handler) { + AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, + protected_store_pc); + } } - if (FLAG_trace_wasm_memory) { + + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(true, type.mem_rep(), index, offset, decoder->position()); } @@ -2940,7 +2996,7 @@ class LiftoffCompiler { AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, protected_store_pc); } - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(true, type.mem_rep(), index, offset, decoder->position()); } @@ -4156,9 +4212,9 @@ class LiftoffCompiler { Register addr = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); LOAD_INSTANCE_FIELD(addr, MemoryStart, kSystemPointerSize, pinned); LiftoffRegList outer_pinned; - if (FLAG_trace_wasm_memory) outer_pinned.set(index); + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) outer_pinned.set(index); __ AtomicStore(addr, index, offset, value, type, outer_pinned); - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(true, type.mem_rep(), index, offset, decoder->position()); } @@ -4184,7 +4240,7 @@ class LiftoffCompiler { __ AtomicLoad(value, addr, index, offset, type, pinned); __ PushRegister(kind, value); - if (FLAG_trace_wasm_memory) { + if (V8_UNLIKELY(FLAG_trace_wasm_memory)) { TraceMemoryOperation(false, type.mem_type().representation(), index, offset, decoder->position()); } From 9caa1bb423c1f03d20796744d2688f708a662c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 08:23:20 +0200 Subject: [PATCH 3/7] deps: V8: cherry-pick 2b77ca200c56 Original commit message: [wasm][liftoff] Always zero-extend 32 bit offsets The upper 32 bits of the 64 bit offset register are not guaranteed to be cleared, so a zero-extension is needed. We already do the zero-extension in the case of explicit bounds checking, but this should also be done if the trap handler is enabled. R=clemensb@chromium.org CC=jkummerow@chromium.org Bug: v8:11809 Change-Id: I21e2535c701041d11fa06c176fa683d82db0a3f1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2917612 Commit-Queue: Thibaud Michaud Reviewed-by: Clemens Backes Cr-Commit-Position: refs/heads/master@{#74881} Refs: https://github.com/v8/v8/commit/2b77ca200c56667c68895e49c96c10ff77834f09 --- common.gypi | 2 +- .../wasm/baseline/arm/liftoff-assembler-arm.h | 3 +- .../baseline/arm64/liftoff-assembler-arm64.h | 15 +++-- .../baseline/ia32/liftoff-assembler-ia32.h | 3 +- deps/v8/src/wasm/baseline/liftoff-assembler.h | 2 +- deps/v8/src/wasm/baseline/liftoff-compiler.cc | 8 ++- .../wasm/baseline/x64/liftoff-assembler-x64.h | 6 +- .../mjsunit/regress/wasm/regress-11809.js | 58 +++++++++++++++++++ 8 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/wasm/regress-11809.js diff --git a/common.gypi b/common.gypi index a7c109075f645c..ee91fb1df6d913 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.15', + 'v8_embedder_string': '-node.16', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h b/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h index 62917ab0a3456a..7acdf635c90ef0 100644 --- a/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h +++ b/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h @@ -766,7 +766,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uint32_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { // Offsets >=2GB are statically OOB on 32-bit systems. DCHECK_LE(offset_imm, std::numeric_limits::max()); liftoff::LoadInternal(this, dst, src_addr, offset_reg, diff --git a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h index af794f9a9d5e66..549bbe7f10a9b8 100644 --- a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h +++ b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h @@ -126,9 +126,13 @@ inline CPURegister AcquireByType(UseScratchRegisterScope* temps, template inline MemOperand GetMemOp(LiftoffAssembler* assm, UseScratchRegisterScope* temps, Register addr, - Register offset, T offset_imm) { + Register offset, T offset_imm, + bool i64_offset = false) { if (offset.is_valid()) { - if (offset_imm == 0) return MemOperand(addr.X(), offset.X()); + if (offset_imm == 0) { + return i64_offset ? MemOperand(addr.X(), offset.X()) + : MemOperand(addr.X(), offset.W(), UXTW); + } Register tmp = temps->AcquireX(); DCHECK_GE(kMaxUInt32, offset_imm); assm->Add(tmp, offset.X(), offset_imm); @@ -490,10 +494,11 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { UseScratchRegisterScope temps(this); - MemOperand src_op = - liftoff::GetMemOp(this, &temps, src_addr, offset_reg, offset_imm); + MemOperand src_op = liftoff::GetMemOp(this, &temps, src_addr, offset_reg, + offset_imm, i64_offset); if (protected_load_pc) *protected_load_pc = pc_offset(); switch (type.value()) { case LoadType::kI32Load8U: diff --git a/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h b/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h index 83b00d4a2ad7db..e597467c7342c7 100644 --- a/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h +++ b/deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h @@ -388,7 +388,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uint32_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { // Offsets >=2GB are statically OOB on 32-bit systems. DCHECK_LE(offset_imm, std::numeric_limits::max()); DCHECK_EQ(type.value_type() == kWasmI64, dst.is_gp_pair()); diff --git a/deps/v8/src/wasm/baseline/liftoff-assembler.h b/deps/v8/src/wasm/baseline/liftoff-assembler.h index 3090bc81659779..dbff396f82bc63 100644 --- a/deps/v8/src/wasm/baseline/liftoff-assembler.h +++ b/deps/v8/src/wasm/baseline/liftoff-assembler.h @@ -669,7 +669,7 @@ class LiftoffAssembler : public TurboAssembler { inline void Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, uint32_t* protected_load_pc = nullptr, - bool is_load_mem = false); + bool is_load_mem = false, bool i64_offset = false); inline void Store(Register dst_addr, Register offset_reg, uintptr_t offset_imm, LiftoffRegister src, StoreType type, LiftoffRegList pinned, diff --git a/deps/v8/src/wasm/baseline/liftoff-compiler.cc b/deps/v8/src/wasm/baseline/liftoff-compiler.cc index c4b6cbdac28d21..84d217b2e42163 100644 --- a/deps/v8/src/wasm/baseline/liftoff-compiler.cc +++ b/deps/v8/src/wasm/baseline/liftoff-compiler.cc @@ -2798,6 +2798,7 @@ class LiftoffCompiler { // Only look at the slot, do not pop it yet (will happen in PopToRegister // below, if this is not a statically-in-bounds index). auto& index_slot = __ cache_state()->stack_state.back(); + bool i64_offset = index_val.type == kWasmI64; if (IndexStaticallyInBounds(index_slot, type.size(), &offset)) { __ cache_state()->stack_state.pop_back(); DEBUG_CODE_COMMENT("load from memory (constant offset)"); @@ -2805,7 +2806,8 @@ class LiftoffCompiler { Register mem = pinned.set(__ GetUnusedRegister(kGpReg, pinned)).gp(); LOAD_INSTANCE_FIELD(mem, MemoryStart, kSystemPointerSize, pinned); LiftoffRegister value = pinned.set(__ GetUnusedRegister(rc, pinned)); - __ Load(value, mem, no_reg, offset, type, pinned, nullptr, true); + __ Load(value, mem, no_reg, offset, type, pinned, nullptr, true, + i64_offset); __ PushRegister(kind, value); } else { LiftoffRegister full_index = __ PopToRegister(); @@ -2824,8 +2826,8 @@ class LiftoffCompiler { LiftoffRegister value = pinned.set(__ GetUnusedRegister(rc, pinned)); uint32_t protected_load_pc = 0; - __ Load(value, mem, index, offset, type, pinned, &protected_load_pc, - true); + __ Load(value, mem, index, offset, type, pinned, &protected_load_pc, true, + i64_offset); if (env_->use_trap_handler) { AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, protected_load_pc); diff --git a/deps/v8/src/wasm/baseline/x64/liftoff-assembler-x64.h b/deps/v8/src/wasm/baseline/x64/liftoff-assembler-x64.h index e8a57bafca1f35..68619a9f1b3e49 100644 --- a/deps/v8/src/wasm/baseline/x64/liftoff-assembler-x64.h +++ b/deps/v8/src/wasm/baseline/x64/liftoff-assembler-x64.h @@ -389,7 +389,11 @@ void LiftoffAssembler::AtomicLoad(LiftoffRegister dst, Register src_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { + if (offset_reg != no_reg && !i64_offset) { + AssertZeroExtended(offset_reg); + } Operand src_op = liftoff::GetMemOp(this, src_addr, offset_reg, offset_imm); if (protected_load_pc) *protected_load_pc = pc_offset(); switch (type.value()) { diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-11809.js b/deps/v8/test/mjsunit/regress/wasm/regress-11809.js new file mode 100644 index 00000000000000..890e26c609e151 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/wasm/regress-11809.js @@ -0,0 +1,58 @@ +// Copyright 2021 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: --enable-testing-opcode-in-wasm --nowasm-tier-up --wasm-tier-mask-for-testing=2 + +load("test/mjsunit/wasm/wasm-module-builder.js"); + +var instance = (function () { + var builder = new WasmModuleBuilder(); + builder.addMemory(1, 1, false /* exported */); + + var sig_index = builder.addType(makeSig( + [kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32, + kWasmI32], + [kWasmI32])); + var sig_three = builder.addType(makeSig( + [kWasmI64, kWasmI64, kWasmI64, kWasmI64, kWasmI64, kWasmI64, kWasmI64, + kWasmI64], + [])); + + var zero = builder.addFunction("zero", kSig_i_i); + var one = builder.addFunction("one", sig_index); + var two = builder.addFunction("two", kSig_v_i); + var three = builder.addFunction("three", sig_three).addBody([]); + + zero.addBody([kExprLocalGet, 0, kExprI32LoadMem, 0, 0]); + + one.addBody([ + kExprLocalGet, 7, + kExprCallFunction, zero.index]); + + two.addBody([ + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprI64Const, 0x81, 0x80, 0x80, 0x80, 0x10, + kExprCallFunction, three.index, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprI32Const, 0, + kExprCallFunction, one.index, + kExprDrop, + ]).exportFunc(); + + return builder.instantiate({}); +})(); + +instance.exports.two() From e3ecfa68655399092b7c306f5505ba82346ef0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 08:23:33 +0200 Subject: [PATCH 4/7] deps: V8: cherry-pick 56fe020eec0c Original commit message: [wasm][arm64] Always zero-extend 32 bit offsets, for realz We've already been zero-extending 32-bit offset registers since https://chromium-review.googlesource.com/c/v8/v8/+/2917612, but that patch only covered the case where offset_imm == 0. When there is a non-zero offset, we need the same fix. Bug: chromium:1224882,v8:11809 Change-Id: I1908f735929798f411346807fc4f3c79d8e04362 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2998582 Commit-Queue: Jakob Kummerow Reviewed-by: Clemens Backes Cr-Commit-Position: refs/heads/master@{#75500} Refs: https://github.com/v8/v8/commit/56fe020eec0c35e9816590114b1d80836a504156 Fixes: https://github.com/nodejs/node/issues/39327 --- common.gypi | 2 +- .../baseline/arm64/liftoff-assembler-arm64.h | 12 +++++++++--- .../test/mjsunit/regress/wasm/regress-11809.js | 16 +++++++++++----- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/common.gypi b/common.gypi index ee91fb1df6d913..36e5de56341a51 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.16', + 'v8_embedder_string': '-node.17', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h index 549bbe7f10a9b8..bea5100ef3e9f8 100644 --- a/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h +++ b/deps/v8/src/wasm/baseline/arm64/liftoff-assembler-arm64.h @@ -133,10 +133,16 @@ inline MemOperand GetMemOp(LiftoffAssembler* assm, return i64_offset ? MemOperand(addr.X(), offset.X()) : MemOperand(addr.X(), offset.W(), UXTW); } - Register tmp = temps->AcquireX(); DCHECK_GE(kMaxUInt32, offset_imm); - assm->Add(tmp, offset.X(), offset_imm); - return MemOperand(addr.X(), tmp); + if (i64_offset) { + Register tmp = temps->AcquireX(); + assm->Add(tmp, offset.X(), offset_imm); + return MemOperand(addr.X(), tmp); + } else { + Register tmp = temps->AcquireW(); + assm->Add(tmp, offset.W(), offset_imm); + return MemOperand(addr.X(), tmp, UXTW); + } } return MemOperand(addr.X(), offset_imm); } diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-11809.js b/deps/v8/test/mjsunit/regress/wasm/regress-11809.js index 890e26c609e151..eef8c291f6e6db 100644 --- a/deps/v8/test/mjsunit/regress/wasm/regress-11809.js +++ b/deps/v8/test/mjsunit/regress/wasm/regress-11809.js @@ -2,11 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // -// Flags: --enable-testing-opcode-in-wasm --nowasm-tier-up --wasm-tier-mask-for-testing=2 +// Flags: --enable-testing-opcode-in-wasm --nowasm-tier-up +// Flags: --wasm-tier-mask-for-testing=2 load("test/mjsunit/wasm/wasm-module-builder.js"); -var instance = (function () { +function InstanceMaker(offset) { var builder = new WasmModuleBuilder(); builder.addMemory(1, 1, false /* exported */); @@ -24,7 +25,7 @@ var instance = (function () { var two = builder.addFunction("two", kSig_v_i); var three = builder.addFunction("three", sig_three).addBody([]); - zero.addBody([kExprLocalGet, 0, kExprI32LoadMem, 0, 0]); + zero.addBody([kExprLocalGet, 0, kExprI32LoadMem, 0, offset]); one.addBody([ kExprLocalGet, 7, @@ -53,6 +54,11 @@ var instance = (function () { ]).exportFunc(); return builder.instantiate({}); -})(); +} -instance.exports.two() +var instance = InstanceMaker(0); +instance.exports.two(); + +// Regression test for crbug.com/1224882. +var instance_with_offset = InstanceMaker(4); +instance_with_offset.exports.two(); From f9f5ff00759f1c095933f435272687198c221879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 13:10:39 +0200 Subject: [PATCH 5/7] deps: V8: cherry-pick 3805a698f7b6 Original commit message: PPC/s390: [wasm][liftoff] Always zero-extend 32 bit offsets Port 2b77ca200c56667c68895e49c96c10ff77834f09 Original Commit Message: The upper 32 bits of the 64 bit offset register are not guaranteed to be cleared, so a zero-extension is needed. We already do the zero-extension in the case of explicit bounds checking, but this should also be done if the trap handler is enabled. R=thibaudm@chromium.org, joransiu@ca.ibm.com, junyan@redhat.com, midawson@redhat.com BUG= LOG=N Change-Id: Ife3ae4f93b85fe1b2c76fe4b98fa408b5b51ed71 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2929661 Reviewed-by: Junliang Yan Commit-Queue: Milad Fa Cr-Commit-Position: refs/heads/master@{#74886} Refs: https://github.com/v8/v8/commit/3805a698f7b6803dd6ee002cfdda71296c71b30b --- common.gypi | 2 +- deps/v8/src/wasm/baseline/ppc/liftoff-assembler-ppc.h | 3 ++- deps/v8/src/wasm/baseline/s390/liftoff-assembler-s390.h | 8 +++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/common.gypi b/common.gypi index 36e5de56341a51..88764c8f6b75a8 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.17', + 'v8_embedder_string': '-node.18', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/ppc/liftoff-assembler-ppc.h b/deps/v8/src/wasm/baseline/ppc/liftoff-assembler-ppc.h index 4e99821a27d563..bedee1a939c007 100644 --- a/deps/v8/src/wasm/baseline/ppc/liftoff-assembler-ppc.h +++ b/deps/v8/src/wasm/baseline/ppc/liftoff-assembler-ppc.h @@ -137,7 +137,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { bailout(kUnsupportedArchitecture, "Load"); } diff --git a/deps/v8/src/wasm/baseline/s390/liftoff-assembler-s390.h b/deps/v8/src/wasm/baseline/s390/liftoff-assembler-s390.h index 8560c91553f8cc..04f30939fdbab6 100644 --- a/deps/v8/src/wasm/baseline/s390/liftoff-assembler-s390.h +++ b/deps/v8/src/wasm/baseline/s390/liftoff-assembler-s390.h @@ -277,11 +277,17 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { UseScratchRegisterScope temps(this); if (!is_int20(offset_imm)) { mov(ip, Operand(offset_imm)); if (offset_reg != no_reg) { + if (!i64_offset) { + // Clear the upper 32 bits of the 64 bit offset register. + llgfr(r0, offset_reg); + offset_reg = r0; + } AddS64(ip, offset_reg); } offset_reg = ip; From 64491124a276094abf38f7dd5cbd77d72f3adea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 13:11:11 +0200 Subject: [PATCH 6/7] deps: V8: cherry-pick 359d44df4cdd Original commit message: [riscv64] Fix build failed Port 2b77ca200c56667c68895e49c96c10ff77834f09 Change-Id: Ie953a1d54f5529423ae35d1b1cd3ca25e8101c6e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2931577 Auto-Submit: Yahan Lu Commit-Queue: Brice Dobry Reviewed-by: Brice Dobry Cr-Commit-Position: refs/heads/master@{#74937} Refs: https://github.com/v8/v8/commit/359d44df4cdd9cdb40cb683b47c2bea6bb7609f6 --- common.gypi | 2 +- deps/v8/src/wasm/baseline/riscv64/liftoff-assembler-riscv64.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/common.gypi b/common.gypi index 88764c8f6b75a8..d4f1d425f83958 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.18', + 'v8_embedder_string': '-node.19', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/riscv64/liftoff-assembler-riscv64.h b/deps/v8/src/wasm/baseline/riscv64/liftoff-assembler-riscv64.h index 47f8ce2125d439..bb6c3bcad886a1 100644 --- a/deps/v8/src/wasm/baseline/riscv64/liftoff-assembler-riscv64.h +++ b/deps/v8/src/wasm/baseline/riscv64/liftoff-assembler-riscv64.h @@ -446,7 +446,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { MemOperand src_op = liftoff::GetMemOp(this, src_addr, offset_reg, offset_imm); if (protected_load_pc) *protected_load_pc = pc_offset(); From fad023dcdd7968cb8fbe9dc7120dfe0e6f5c1423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 10 Jul 2021 13:15:42 +0200 Subject: [PATCH 7/7] deps: V8: backport 5c76da8ddcf8 Original commit message: [mips][wasm][liftoff] Fix compile failed Port 2b77ca200c56667c68895e49c96c10ff77834f09 Bug: v8:11809 Change-Id: Idbbbc10d1339d6c8463686b6e701fb601a217cab Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2931557 Reviewed-by: Zhao Jiazhong Commit-Queue: Zhao Jiazhong Auto-Submit: Liu yu Cr-Commit-Position: refs/heads/master@{#74934} Refs: https://github.com/v8/v8/commit/5c76da8ddcf89297a8dc2606b68da97d7a5329cb --- common.gypi | 2 +- deps/v8/src/wasm/baseline/mips/liftoff-assembler-mips.h | 3 ++- deps/v8/src/wasm/baseline/mips64/liftoff-assembler-mips64.h | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/common.gypi b/common.gypi index d4f1d425f83958..71862791dae3be 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.19', + 'v8_embedder_string': '-node.20', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/wasm/baseline/mips/liftoff-assembler-mips.h b/deps/v8/src/wasm/baseline/mips/liftoff-assembler-mips.h index ca715a8a328114..d078fd5e429429 100644 --- a/deps/v8/src/wasm/baseline/mips/liftoff-assembler-mips.h +++ b/deps/v8/src/wasm/baseline/mips/liftoff-assembler-mips.h @@ -491,7 +491,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uint32_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { Register src = no_reg; if (offset_reg != no_reg) { src = GetUnusedRegister(kGpReg, pinned).gp(); diff --git a/deps/v8/src/wasm/baseline/mips64/liftoff-assembler-mips64.h b/deps/v8/src/wasm/baseline/mips64/liftoff-assembler-mips64.h index a5a9f8ce231b46..dfbd8d6a752ee1 100644 --- a/deps/v8/src/wasm/baseline/mips64/liftoff-assembler-mips64.h +++ b/deps/v8/src/wasm/baseline/mips64/liftoff-assembler-mips64.h @@ -470,7 +470,8 @@ void LiftoffAssembler::StoreTaggedPointer(Register dst_addr, void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, Register offset_reg, uintptr_t offset_imm, LoadType type, LiftoffRegList pinned, - uint32_t* protected_load_pc, bool is_load_mem) { + uint32_t* protected_load_pc, bool is_load_mem, + bool i64_offset) { MemOperand src_op = liftoff::GetMemOp(this, src_addr, offset_reg, offset_imm); if (protected_load_pc) *protected_load_pc = pc_offset();