[wasm][atomics] Fix assumption
The assumption in {DecodeAtomicOpcode} (added in https://crrev.com/c/3990654) is only true for valid opcodes. Since Atomic opcodes are variable-length encoded, it's possible to create out-of-bounds atomic opcodes which violate the assumption. This CL fixes that by checking for such out-of-bounds opcodes early in the method. This replaces the assumption, which the compiler can now derive from the if-statement. R=ahaas@chromium.org Bug: chromium:1381330 Change-Id: Ifaaceb0c8a765811fe2f934be1920bcb14675f36 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4008538 Reviewed-by: Andreas Haas <ahaas@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/main@{#84091}
This commit is contained in:
parent
98551cf4a2
commit
75dc4a9cf4
@ -5827,11 +5827,8 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
|
||||
#undef NON_CONST_ONLY
|
||||
|
||||
uint32_t DecodeAtomicOpcode(WasmOpcode opcode, uint32_t opcode_length) {
|
||||
// This assumption avoids a dynamic check in signature lookup, and might
|
||||
// also help the big switch below.
|
||||
V8_ASSUME(opcode >> 8 == kAtomicPrefix);
|
||||
const FunctionSig* sig = WasmOpcodes::Signature(opcode);
|
||||
if (!VALIDATE(sig != nullptr)) {
|
||||
// Fast check for out-of-range opcodes (only allow 0xfeXX).
|
||||
if (!VALIDATE((opcode >> 8) == kAtomicPrefix)) {
|
||||
this->DecodeError("invalid atomic opcode");
|
||||
return 0;
|
||||
}
|
||||
@ -5868,6 +5865,9 @@ class WasmFullDecoder : public WasmDecoder<ValidationTag, decoding_mode> {
|
||||
return 0;
|
||||
}
|
||||
|
||||
const FunctionSig* sig = WasmOpcodes::Signature(opcode);
|
||||
V8_ASSUME(sig != nullptr);
|
||||
|
||||
MemoryAccessImmediate imm = MakeMemoryAccessImmediate(
|
||||
opcode_length, ElementSizeLog2Of(memtype.representation()));
|
||||
if (!this->Validate(this->pc_ + opcode_length, imm)) return false;
|
||||
|
@ -468,3 +468,16 @@ function CmpExchgLoop(opcode, alignment) {
|
||||
CmpExchgLoop(kExprI64AtomicCompareExchange16U, 1);
|
||||
CmpExchgLoop(kExprI64AtomicCompareExchange8U, 0);
|
||||
})();
|
||||
|
||||
(function TestIllegalAtomicOp() {
|
||||
// Regression test for https://crbug.com/1381330.
|
||||
print(arguments.callee.name);
|
||||
let builder = new WasmModuleBuilder();
|
||||
builder.addFunction('main', kSig_v_v).addBody([
|
||||
kAtomicPrefix, 0x90, 0x0f
|
||||
]);
|
||||
assertEquals(false, WebAssembly.validate(builder.toBuffer()));
|
||||
assertThrows(
|
||||
() => builder.toModule(), WebAssembly.CompileError,
|
||||
/invalid atomic opcode/);
|
||||
})();
|
||||
|
Loading…
Reference in New Issue
Block a user