Skip to content

Commit

Permalink
Merged: [wasm-simd] Fix bounds check for load extends
Browse files Browse the repository at this point in the history
Load extends always load 8 bytes, so the access size does not depend on
MachineType of the load. The MachineType is used for classifying the
lane shape of the 8-byte load.

Also add cctest to load splats and load extends to test OOB. (Note that
load splats access size depends on MachineType).

Add regression test from clusterfuzz, minimized by ahaas@. Remove the
`--no-wasm-trap-handler` flag since we have a no_wasm_traps variant that
should test this flag.

Bug: chromium:1116019
(cherry picked from commit a85b5a6)

Change-Id: I6fc3ef9d7ac8a50037224e7886037fd0d2dcf16e
No-Try: true
No-Presubmit: true
No-Tree-Checks: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2367332
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Zhi An Ng <[email protected]>
Cr-Commit-Position: refs/branch-heads/8.6@{#4}
Cr-Branched-From: a64aed2-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc0-refs/heads/master@{#69472}
  • Loading branch information
ngzhian authored and Commit Bot committed Aug 21, 2020
1 parent f39ac91 commit f9e5c3c
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/compiler/wasm-compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4012,8 +4012,13 @@ Node* WasmGraphBuilder::LoadTransform(wasm::ValueType type, MachineType memtype,
#else
// Wasm semantics throw on OOB. Introduce explicit bounds check and
// conditioning when not using the trap handler.
index = BoundsCheckMem(memtype.MemSize(), index, offset, position,
kCanOmitBoundsCheck);

// Load extends always load 8 bytes.
uint8_t access_size = transform == wasm::LoadTransformationKind::kExtend
? 8
: memtype.MemSize();
index =
BoundsCheckMem(access_size, index, offset, position, kCanOmitBoundsCheck);

LoadTransformation transformation = GetLoadTransformation(memtype, transform);
LoadKind load_kind = GetLoadKind(mcgraph(), memtype, use_trap_handler());
Expand Down
32 changes: 32 additions & 0 deletions test/cctest/wasm/test-run-wasm-simd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3499,6 +3499,22 @@ void RunLoadSplatTest(TestExecutionTier execution_tier, LowerSimd lower_simd,
CHECK_EQ(x, ReadLittleEndianValue<T>(&global[i]));
}
}

// Test for OOB.
{
WasmRunner<int32_t, uint32_t> r(execution_tier, lower_simd);
r.builder().AddMemoryElems<T>(kWasmPageSize / sizeof(T));
r.builder().AddGlobal<T>(kWasmS128);

BUILD(r, WASM_SET_GLOBAL(0, WASM_SIMD_LOAD_OP(op, WASM_GET_LOCAL(0))),
WASM_ONE);

// Load splats load sizeof(T) bytes.
for (uint32_t offset = kWasmPageSize - (sizeof(T) - 1);
offset < kWasmPageSize; ++offset) {
CHECK_TRAP(r.Call(offset));
}
}
}

WASM_SIMD_TEST(S8x16LoadSplat) {
Expand Down Expand Up @@ -3574,6 +3590,22 @@ void RunLoadExtendTest(TestExecutionTier execution_tier, LowerSimd lower_simd,
CHECK_EQ(expected, ReadLittleEndianValue<T>(&global[i]));
}
}

// Test for OOB.
{
WasmRunner<int32_t, uint32_t> r(execution_tier, lower_simd);
r.builder().AddMemoryElems<S>(kWasmPageSize / sizeof(S));
r.builder().AddGlobal<T>(kWasmS128);

BUILD(r, WASM_SET_GLOBAL(0, WASM_SIMD_LOAD_OP(op, WASM_GET_LOCAL(0))),
WASM_ONE);

// Load extends load 8 bytes, so should trap from -7.
for (uint32_t offset = kWasmPageSize - 7; offset < kWasmPageSize;
++offset) {
CHECK_TRAP(r.Call(offset));
}
}
}

WASM_SIMD_TEST(I16x8Load8x8U) {
Expand Down
23 changes: 23 additions & 0 deletions test/mjsunit/regress/wasm/regress-1116019.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2020 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: --experimental-wasm-simd

load('test/mjsunit/wasm/wasm-module-builder.js');

const builder = new WasmModuleBuilder();
builder.addMemory(16, 32, false);
builder.addType(makeSig([], [kWasmI32]));
// Generate function 1 (out of 1).
builder.addFunction(undefined, 0 /* sig */)
.addBodyWithEnd([
// signature: i_v
// body:
kExprI32Const, 0x00, // i32.const
kSimdPrefix, kExprI16x8Load8x8U, 0x03, 0xff, 0xff, 0x3f, // i16x8.load8x8_u
kSimdPrefix, kExprI16x8ExtractLaneS, 0,
kExprEnd, // end @371
]).exportAs('main');
const instance = builder.instantiate();
assertTraps(kTrapMemOutOfBounds, () => instance.exports.main());
1 change: 1 addition & 0 deletions test/mjsunit/wasm/wasm-module-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ let kExprI64x2Splat = 0x12;
let kExprF32x4Splat = 0x13;
let kExprF64x2Splat = 0x14;
let kExprI8x16ReplaceLane = 0x17;
let kExprI16x8ExtractLaneS = 0x18;
let kExprI16x8ReplaceLane = 0x1a;
let kExprI32x4ExtractLane = 0x1b;
let kExprI32x4ReplaceLane = 0x1c;
Expand Down

0 comments on commit f9e5c3c

Please sign in to comment.