AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm] Be more strict about allowed SIMD opcodes

Browse Source 
This makes some checks a bit stricter to avoid accepting illegal relaxed
SIMD opcodes.

1) The default case in the Liftoff compiler should be UNREACHABLE,
   such that the switch case is required to cover all defined opcodes.
2) The {WasmOpcodes::IsRelaxedSimdOpcode} wrongly also returned {true}
   for opcodes like 0xfd300. We should really check nibbles 3-5 for the
   exact value 0xfd1.
3) {WasmOpcodes::Signature} was returning a non-null signatures for
   illegal opcodes like 0xfd200, because {IsRelaxedSimdOpcode} returned
   false, and then we would just use the lower bytes for the lookup in
   the SIMD signature table.

R=thibaudm@chromium.org
CC=gdeepti@chromium.org

Bug: chromium:1324081
Change-Id: Idbfde570ccd782e59b47b96e7ca8cc28fa7fae98
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687309
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80934}
This commit is contained in:
Clemens Backes 2022-06-03 10:40:17 +02:00 committed by V8 LUCI CQ
parent 9dcdfaec7d
commit 40738e6c45
2 changed files with 18 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/wasm/baseline/liftoff-compiler.cc
View File

@ -4119,7 +4119,7 @@ class LiftoffCompiler {
return; return;
} }
default: default:
unsupported(decoder, kSimd, "simd"); UNREACHABLE();
} }
} }

24
src/wasm/wasm-opcodes-inl.h
View File

@ -600,13 +600,15 @@ constexpr bool WasmOpcodes::IsThrowingOpcode(WasmOpcode opcode) {
// static // static
constexpr bool WasmOpcodes::IsRelaxedSimdOpcode(WasmOpcode opcode) { constexpr bool WasmOpcodes::IsRelaxedSimdOpcode(WasmOpcode opcode) {
// Relaxed SIMD opcodes have the SIMD prefix (0xfd) shifted by 12 bits, and
// nibble 3 must be 0x1. I.e. their encoded opcode is in [0xfd100, 0xfd1ff].
static_assert(kSimdPrefix == 0xfd); static_assert(kSimdPrefix == 0xfd);
#define CHECK_OPCODE(name, opcode, _) \ #define CHECK_OPCODE(name, opcode, _) \
static_assert((opcode & 0xfd100) == 0xfd100); static_assert((opcode & 0xfff00) == 0xfd100);
FOREACH_RELAXED_SIMD_OPCODE(CHECK_OPCODE) FOREACH_RELAXED_SIMD_OPCODE(CHECK_OPCODE)
#undef CHECK_OPCODE #undef CHECK_OPCODE
return (opcode & 0xfd100) == 0xfd100; return (opcode & 0xfff00) == 0xfd100;
} }
constexpr byte WasmOpcodes::ExtractPrefix(WasmOpcode opcode) { constexpr byte WasmOpcodes::ExtractPrefix(WasmOpcode opcode) {
@ -695,16 +697,24 @@ constexpr std::array<WasmOpcodeSig, 256> kNumericExprSigTable =
constexpr const FunctionSig* WasmOpcodes::Signature(WasmOpcode opcode) { constexpr const FunctionSig* WasmOpcodes::Signature(WasmOpcode opcode) {
switch (ExtractPrefix(opcode)) { switch (ExtractPrefix(opcode)) {
case 0: case 0:
DCHECK_GT(impl::kShortSigTable.size(), opcode);
return impl::kCachedSigs[impl::kShortSigTable[opcode]]; return impl::kCachedSigs[impl::kShortSigTable[opcode]];
case kSimdPrefix: { case kSimdPrefix: {
if (IsRelaxedSimdOpcode(opcode)) // Handle SIMD MVP opcodes (in [0xfd00, 0xfdff]).
return impl::kCachedSigs[impl::kRelaxedSimdExprSigTable[opcode & 0xFF]]; if (opcode <= 0xfdff) {
return impl::kCachedSigs[impl::kSimdExprSigTable[opcode & 0xFF]]; DCHECK_LE(0xfd00, opcode);
return impl::kCachedSigs[impl::kSimdExprSigTable[opcode & 0xff]];
}
// Handle relaxed SIMD opcodes (in [0xfd100, 0xfd1ff]).
if (IsRelaxedSimdOpcode(opcode)) {
return impl::kCachedSigs[impl::kRelaxedSimdExprSigTable[opcode & 0xff]];
}
return nullptr;
} }
case kAtomicPrefix: case kAtomicPrefix:
return impl::kCachedSigs[impl::kAtomicExprSigTable[opcode & 0xFF]]; return impl::kCachedSigs[impl::kAtomicExprSigTable[opcode & 0xff]];
case kNumericPrefix: case kNumericPrefix:
return impl::kCachedSigs[impl::kNumericExprSigTable[opcode & 0xFF]]; return impl::kCachedSigs[impl::kNumericExprSigTable[opcode & 0xff]];
default: default:
UNREACHABLE(); // invalid prefix. UNREACHABLE(); // invalid prefix.
} }