[JSC] wasm unaligned atomic accesses should trap with "Unaligned" rat…

…her than "Out of bounds" https://bugs.webkit.org/show_bug.cgi?id=278412 rdar://103442167 Reviewed by Yusuke Suzuki. Introduce a new wasm trap for "Unaligned memory access" and use it when atomic alignment is checked. Previously, unaligned atomic accesses would produce an "Out of bounds memory access" trap. * JSTests/wasm/stress/atomic-unaligned-traps.js: Added. (genAtomicInstr): (genWat): (async test): * Source/JavaScriptCore/llint/InPlaceInterpreter64.asm: * Source/JavaScriptCore/llint/WebAssembly32_64.asm: * Source/JavaScriptCore/llint/WebAssembly64.asm: * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicLoadOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicStoreOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicBinaryRMWOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicCompareExchange): * Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicLoadOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicStoreOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicBinaryRMWOp): (JSC::Wasm::BBQJITImpl::BBQJIT::emitAtomicCompareExchange): * Source/JavaScriptCore/wasm/WasmExceptionType.h: (JSC::Wasm::isTypeErrorExceptionType): * Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp: (JSC::Wasm::OMGIRGenerator::fixupPointerPlusOffsetForAtomicOps): * Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp: (JSC::Wasm::OMGIRGenerator::fixupPointerPlusOffsetForAtomicOps): Canonical link: https://commits.webkit.org/282561@main
dhecht · Aug 21, 2024 · 8e565ca · 8e565ca
1 parent ae24b30
commit 8e565ca
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 24 deletions.
diff --git a/JSTests/wasm/stress/atomic-unaligned-traps.js b/JSTests/wasm/stress/atomic-unaligned-traps.js
@@ -0,0 +1,69 @@
+import { instantiate } from "../wabt-wrapper.js";
+import * as assert from "../assert.js";
+
+const verbose = false;
+
+function genAtomicInstr(op, subOp, typeSz, accessSz) {
+    let opSz = '';
+
+    if (subOp != '')
+        subOp = '.' + subOp; // rmw ops
+
+    if (typeSz != accessSz) {
+        opSz = `${accessSz}`;
+        if (op != 'store')
+            subOp += '_u';
+        }
+    return `i${typeSz}.atomic.${op}${opSz}${subOp}`
+}
+
+function genWat(op, subOp, typeSz, accessSz) {
+    const instr = genAtomicInstr(op, subOp, typeSz, accessSz);
+    let expected = '', operand = '', retVal = '';
+    if (op == 'store') {
+        operand = `i${typeSz}.const 42`;
+        retVal = `i${typeSz}.const 7`;
+    } else if (op == 'rmw') {
+        operand = `i${typeSz}.const 80`;
+        if (subOp == 'cmpxchg')
+            expected = `i${typeSz}.const 13`;
+    }
+    let wat = `
+    (module
+      (func (export "test") (param $addr i32) (result i${typeSz})
+        local.get $addr
+        ${expected}
+        ${operand}
+        ${instr}
+        ${retVal}
+      )
+      (memory 1)
+    )
+    `;
+    if (verbose)
+        print(wat + '\n');
+    return wat;
+}
+
+async function test(op, subOp, typeSz, accessSz) {
+    const instance = await instantiate(genWat(op, subOp, typeSz, accessSz), {}, {threads: true});
+    const {test} = instance.exports;
+    assert.throws(() => {
+        test(1);
+    }, WebAssembly.RuntimeError, `Unaligned memory access`);
+}
+
+for (const op of ['load', 'store']) {
+    for (const typeSz of [32, 64]) {
+        for (let accessSz = typeSz; accessSz >= 16; accessSz /= 2)
+            await assert.asyncTest(test(op, '', typeSz, accessSz));
+    }
+}
+
+// RMW operators
+for (const subOp of ['add', 'sub', 'and', 'or', 'xor', 'xchg', 'cmpxchg']) {
+    for (const typeSz of [32, 64]) {
+        for (let accessSz = typeSz; accessSz >= 16; accessSz /= 2)
+            await assert.asyncTest(test('rmw', subOp, typeSz, accessSz));
+    }
+}
diff --git a/Source/JavaScriptCore/llint/InPlaceInterpreter64.asm b/Source/JavaScriptCore/llint/InPlaceInterpreter64.asm
@@ -3656,11 +3656,14 @@ unimplementedInstruction(_simd_f64x2_convert_low_i32x4_u)
 
 macro ipintCheckMemoryBoundWithAlignmentCheck(mem, scratch, size)
     leap size - 1[mem], scratch
-    bpb scratch, boundsCheckingSize, .continuation
-.throw:
+    bpb scratch, boundsCheckingSize, .continuationInBounds
+.throwOOB:
     ipintException(OutOfBoundsMemoryAccess)
-.continuation:
-    btpnz mem, (size - 1), .throw
+.continuationInBounds:
+    btpz mem, (size - 1), .continuationAligned
+.throwUnaligned:
+    throwException(UnalignedMemoryAccess)
+.continuationAligned:
 end
 
 macro ipintCheckMemoryBoundWithAlignmentCheck1(mem, scratch)

diff --git a/Source/JavaScriptCore/llint/WebAssembly32_64.asm b/Source/JavaScriptCore/llint/WebAssembly32_64.asm
@@ -61,15 +61,18 @@ if ARMv7
     # Not enough registers on arm to keep the memory base and size in pinned
     # registers, so load them on each access instead. FIXME: improve this.
     addps offset, pointer
-    bcs .throw
+    bcs .throwOOB
     addps size - 1, pointer, offset # Use offset as scratch register
-    bcs .throw
-    bpb offset, JSWebAssemblyInstance::m_cachedBoundsCheckingSize[wasmInstance], .continuation
-.throw:
+    bcs .throwOOB
+    bpb offset, JSWebAssemblyInstance::m_cachedBoundsCheckingSize[wasmInstance], .continuationInBounds
+.throwOOB:
     throwException(OutOfBoundsMemoryAccess)
-.continuation:
+.continuationInBounds:
     addp JSWebAssemblyInstance::m_cachedMemory[wasmInstance], pointer
-    btpnz pointer, (size - 1), .throw
+    btpz pointer, (size - 1), .continuationAligned
+.throwUnaligned:
+    throwException(UnalignedMemoryAccess)
+.continuationAligned:
 else
     crash()
 end

diff --git a/Source/JavaScriptCore/llint/WebAssembly64.asm b/Source/JavaScriptCore/llint/WebAssembly64.asm
@@ -51,13 +51,16 @@ end
 
 macro emitCheckAndPreparePointerAddingOffsetWithAlignmentCheck(ctx, pointer, offset, size)
     leap size - 1[pointer, offset], t5
-    bpb t5, boundsCheckingSize, .continuation
-.throw:
+    bpb t5, boundsCheckingSize, .continuationInBounds
+.throwOOB:
     throwException(OutOfBoundsMemoryAccess)
-.continuation:
+.continuationInBounds:
     addp memoryBase, pointer
     addp offset, pointer
-    btpnz pointer, (size - 1), .throw
+    btpz pointer, (size - 1), .continuationAligned
+.throwUnaligned:
+    throwException(UnalignedMemoryAccess)
+.continuationAligned:
 end
 
 

diff --git a/Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp b/Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp
@@ -767,7 +767,7 @@ Value WARN_UNUSED_RETURN BBQJIT::emitAtomicLoadOp(ExtAtomicOpType loadOp, Type v
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(loadOp) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(loadOp) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(loadOp) - 1)));
 
     Value result = topValue(valueType.kind);
     Location resultLocation;
@@ -842,7 +842,7 @@ void BBQJIT::emitAtomicStoreOp(ExtAtomicOpType storeOp, Type, Location pointer,
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(storeOp) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(storeOp) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(storeOp) - 1)));
 
     Location source, old, cur;
     switch (canonicalWidth(accessWidth(storeOp))) {
@@ -890,7 +890,7 @@ Value BBQJIT::emitAtomicBinaryRMWOp(ExtAtomicOpType op, Type valueType, Location
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(op) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(op) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(op) - 1)));
 
     Value result = topValue(valueType.kind);
     Location resultLocation;
@@ -1056,7 +1056,7 @@ Value WARN_UNUSED_RETURN BBQJIT::emitAtomicCompareExchange(ExtAtomicOpType op, T
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(op) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(op) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest32(ResultCondition::NonZero, pointer.asGPR(), TrustedImm32(sizeOfAtomicOpMemoryAccess(op) - 1)));
 
     Value result = topValue(valueType.kind);
     Location resultLocation;

diff --git a/Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp b/Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp
@@ -639,7 +639,7 @@ Value WARN_UNUSED_RETURN BBQJIT::emitAtomicLoadOp(ExtAtomicOpType loadOp, Type v
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(loadOp) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(loadOp) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(loadOp) - 1)));
 
     Value result = topValue(valueType.kind);
     Location resultLocation = allocate(result);
@@ -731,7 +731,7 @@ void BBQJIT::emitAtomicStoreOp(ExtAtomicOpType storeOp, Type, Location pointer,
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(storeOp) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(storeOp) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(storeOp) - 1)));
 
     GPRReg scratch1GPR = InvalidGPRReg;
     GPRReg scratch2GPR = InvalidGPRReg;
@@ -832,7 +832,7 @@ Value BBQJIT::emitAtomicBinaryRMWOp(ExtAtomicOpType op, Type valueType, Location
     Address address = Address(pointer.asGPR());
 
     if (accessWidth(op) != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(op) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(op) - 1)));
 
     Value result = topValue(valueType.kind);
     Location resultLocation = allocate(result);
@@ -1196,7 +1196,7 @@ Value WARN_UNUSED_RETURN BBQJIT::emitAtomicCompareExchange(ExtAtomicOpType op, T
     Width accessWidth = this->accessWidth(op);
 
     if (accessWidth != Width8)
-        throwExceptionIf(ExceptionType::OutOfBoundsMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(op) - 1)));
+        throwExceptionIf(ExceptionType::UnalignedMemoryAccess, m_jit.branchTest64(ResultCondition::NonZero, pointer.asGPR(), TrustedImm64(sizeOfAtomicOpMemoryAccess(op) - 1)));
 
     Value result = topValue(expected.type());
     Location resultLocation = allocate(result);

diff --git a/Source/JavaScriptCore/wasm/WasmExceptionType.h b/Source/JavaScriptCore/wasm/WasmExceptionType.h
@@ -35,6 +35,7 @@ namespace Wasm {
 
 #define FOR_EACH_EXCEPTION(macro) \
     macro(OutOfBoundsMemoryAccess,  "Out of bounds memory access"_s) \
+    macro(UnalignedMemoryAccess, "Unaligned memory access"_s) \
     macro(OutOfBoundsTableAccess, "Out of bounds table access"_s) \
     macro(OutOfBoundsCallIndirect, "Out of bounds call_indirect"_s) \
     macro(NullTableEntry,  "call_indirect to a null table entry"_s) \
@@ -101,6 +102,7 @@ ALWAYS_INLINE bool isTypeErrorExceptionType(ExceptionType type)
 {
     switch (type) {
     case ExceptionType::OutOfBoundsMemoryAccess:
+    case ExceptionType::UnalignedMemoryAccess:
     case ExceptionType::OutOfBoundsTableAccess:
     case ExceptionType::OutOfBoundsDataSegmentAccess:
     case ExceptionType::OutOfBoundsElementSegmentAccess:

diff --git a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp
@@ -2695,7 +2695,7 @@ Value* OMGIRGenerator::fixupPointerPlusOffsetForAtomicOps(ExtAtomicOpType op, Va
         CheckValue* check = append<CheckValue>(m_proc, Check, origin(),
             append<Value>(m_proc, BitAnd, origin(), pointer, constant(pointerType(), sizeOfAtomicOpMemoryAccess(op) - 1)));
         check->setGenerator([=, this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
-            this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsMemoryAccess);
+            this->emitExceptionCheck(jit, ExceptionType::UnalignedMemoryAccess);
         });
     }
     return pointer;

diff --git a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp
@@ -2424,7 +2424,7 @@ Value* OMGIRGenerator::fixupPointerPlusOffsetForAtomicOps(ExtAtomicOpType op, Va
         CheckValue* check = m_currentBlock->appendNew<CheckValue>(m_proc, Check, origin(),
             m_currentBlock->appendNew<Value>(m_proc, BitAnd, origin(), pointer, constant(pointerType(), sizeOfAtomicOpMemoryAccess(op) - 1)));
         check->setGenerator([=, this] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
-            this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsMemoryAccess);
+            this->emitExceptionCheck(jit, ExceptionType::UnalignedMemoryAccess);
         });
     }
     return pointer;