From ccb7c21a08c25bb08103f309ef569e099ef3943f Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Tue, 4 Aug 2026 13:47:03 +0530 Subject: [PATCH 1/2] Validate natural alignment for atomic memory ops Preserve memarg alignment in IR for atomic load/store/RMW/cmpxchg/wait/notify (including field delegations) so non-natural alignment fails validation. Fixes #8962 Signed-off-by: Gaurav Chaudhary --- CHANGELOG.md | 2 + scripts/test/shared.py | 3 - src/parser/contexts.h | 31 ++++++--- src/passes/DeAlign.cpp | 14 +++- src/passes/Print.cpp | 13 ++++ src/wasm-builder.h | 81 ++++++++++++++++++++++- src/wasm-delegations-fields.def | 4 ++ src/wasm-ir-builder.h | 29 +++++--- src/wasm.h | 4 ++ src/wasm/wasm-binary.cpp | 72 +++++++++++--------- src/wasm/wasm-ir-builder.cpp | 41 ++++++++---- src/wasm/wasm-stack.cpp | 28 +++++--- src/wasm/wasm-validator.cpp | 11 +++ test/lit/passes/dealign-atomics.wast | 19 ++++++ test/lit/validation/atomic-alignment.wast | 39 +++++++++++ 15 files changed, 312 insertions(+), 79 deletions(-) create mode 100644 test/lit/passes/dealign-atomics.wast create mode 100644 test/lit/validation/atomic-alignment.wast diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b28dee98d3..30abb07e039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ full changeset diff at the end of each section. Current Trunk ------------- +- Reject non-natural alignment for atomic memory operations (#8962) + v132 ---- diff --git a/scripts/test/shared.py b/scripts/test/shared.py index cad9ce4b930..77b36105abf 100644 --- a/scripts/test/shared.py +++ b/scripts/test/shared.py @@ -415,9 +415,6 @@ def get_tests(test_dir, extensions=[], recursive=False): # Requires better support for multi-threaded tests 'threads/wait_notify.wast', - - # Non-natural alignment is invalid for atomic operations - 'threads/atomic.wast', ] SPEC_TESTSUITE_PROPOSALS_TO_SKIP = [ ] diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 9602a17ce1f..75ab7cb0637 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -2394,8 +2394,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { auto m = getMemory(pos, mem); CHECK_ERR(m); if (isAtomic) { - return withLoc( - pos, irBuilder.makeAtomicLoad(bytes, memarg.offset, type, *m, order)); + return withLoc(pos, + irBuilder.makeAtomicLoad( + bytes, memarg.offset, memarg.align, type, *m, order)); } return withLoc(pos, irBuilder.makeLoad( @@ -2413,8 +2414,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { auto m = getMemory(pos, mem); CHECK_ERR(m); if (isAtomic) { - return withLoc( - pos, irBuilder.makeAtomicStore(bytes, memarg.offset, type, *m, order)); + return withLoc(pos, + irBuilder.makeAtomicStore( + bytes, memarg.offset, memarg.align, type, *m, order)); } return withLoc( pos, irBuilder.makeStore(bytes, memarg.offset, memarg.align, type, *m)); @@ -2454,8 +2456,14 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { MemoryOrder order) { auto m = getMemory(pos, mem); CHECK_ERR(m); - return withLoc( - pos, irBuilder.makeAtomicRMW(op, bytes, memarg.offset, type, *m, order)); + return withLoc(pos, + irBuilder.makeAtomicRMW(op, + bytes, + memarg.offset, + memarg.align, + type, + *m, + order)); } Result<> makeAtomicCmpxchg(Index pos, @@ -2467,8 +2475,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { MemoryOrder order) { auto m = getMemory(pos, mem); CHECK_ERR(m); - return withLoc( - pos, irBuilder.makeAtomicCmpxchg(bytes, memarg.offset, type, *m, order)); + return withLoc(pos, + irBuilder.makeAtomicCmpxchg( + bytes, memarg.offset, memarg.align, type, *m, order)); } Result<> makeAtomicWait(Index pos, @@ -2478,7 +2487,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Memarg memarg) { auto m = getMemory(pos, mem); CHECK_ERR(m); - return withLoc(pos, irBuilder.makeAtomicWait(type, memarg.offset, *m)); + return withLoc( + pos, irBuilder.makeAtomicWait(type, memarg.offset, memarg.align, *m)); } Result<> makeAtomicNotify(Index pos, @@ -2487,7 +2497,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Memarg memarg) { auto m = getMemory(pos, mem); CHECK_ERR(m); - return withLoc(pos, irBuilder.makeAtomicNotify(memarg.offset, *m)); + return withLoc( + pos, irBuilder.makeAtomicNotify(memarg.offset, memarg.align, *m)); } Result<> makeAtomicFence(Index pos, diff --git a/src/passes/DeAlign.cpp b/src/passes/DeAlign.cpp index 4de36cc88ce..52b20b28634 100644 --- a/src/passes/DeAlign.cpp +++ b/src/passes/DeAlign.cpp @@ -31,9 +31,19 @@ struct DeAlign : public WalkerPass> { return std::make_unique(); } - void visitLoad(Load* curr) { curr->align = 1; } + void visitLoad(Load* curr) { + if (curr->isAtomic()) { + return; + } + curr->align = 1; + } - void visitStore(Store* curr) { curr->align = 1; } + void visitStore(Store* curr) { + if (curr->isAtomic()) { + return; + } + curr->align = 1; + } void visitSIMDLoad(SIMDLoad* curr) { curr->align = 1; } diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 50600001e9a..8eae30e770d 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -613,6 +613,9 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } + if (curr->align != curr->bytes) { + o << " align=" << curr->align; + } } void visitAtomicCmpxchg(AtomicCmpxchg* curr) { prepareColor(o); @@ -628,6 +631,9 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } + if (curr->align != curr->bytes) { + o << " align=" << curr->align; + } } void visitAtomicWait(AtomicWait* curr) { prepareColor(o); @@ -639,6 +645,10 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } + Index natural = type == Type::i32 ? 4 : 8; + if (curr->align != natural) { + o << " align=" << curr->align; + } } void visitAtomicNotify(AtomicNotify* curr) { printMedium(o, "memory.atomic.notify"); @@ -646,6 +656,9 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } + if (curr->align != 4) { + o << " align=" << curr->align; + } } void visitAtomicFence(AtomicFence* curr) { printMedium(o, "atomic.fence"); diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 3b4cbb2d7ef..c3064fc411b 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -389,6 +389,7 @@ class Builder { } Load* makeAtomicLoad(unsigned bytes, Address offset, + Address align, Expression* ptr, Type type, Name memory, @@ -396,18 +397,28 @@ class Builder { assert(order != MemoryOrder::Unordered && "Atomic loads can't be unordered"); - Load* load = makeLoad(bytes, false, offset, bytes, ptr, type, memory); + Load* load = makeLoad(bytes, false, offset, align, ptr, type, memory); load->order = order; return load; } + Load* makeAtomicLoad(unsigned bytes, + Address offset, + Expression* ptr, + Type type, + Name memory, + MemoryOrder order) { + return makeAtomicLoad(bytes, offset, bytes, ptr, type, memory, order); + } AtomicWait* makeAtomicWait(Expression* ptr, Expression* expected, Expression* timeout, Type expectedType, Address offset, + Address align, Name memory) { auto* wait = wasm.allocator.alloc(); wait->offset = offset; + wait->align = align; wait->ptr = ptr; wait->expected = expected; wait->timeout = timeout; @@ -416,18 +427,40 @@ class Builder { wait->memory = memory; return wait; } + AtomicWait* makeAtomicWait(Expression* ptr, + Expression* expected, + Expression* timeout, + Type expectedType, + Address offset, + Name memory) { + return makeAtomicWait(ptr, + expected, + timeout, + expectedType, + offset, + expectedType.getByteSize(), + memory); + } AtomicNotify* makeAtomicNotify(Expression* ptr, Expression* notifyCount, Address offset, + Address align, Name memory) { auto* notify = wasm.allocator.alloc(); notify->offset = offset; + notify->align = align; notify->ptr = ptr; notify->notifyCount = notifyCount; notify->finalize(); notify->memory = memory; return notify; } + AtomicNotify* makeAtomicNotify(Expression* ptr, + Expression* notifyCount, + Address offset, + Name memory) { + return makeAtomicNotify(ptr, notifyCount, offset, 4, memory); + } AtomicFence* makeAtomicFence(MemoryOrder order) { auto* ret = wasm.allocator.alloc(); ret->order = order; @@ -455,6 +488,7 @@ class Builder { } Store* makeAtomicStore(unsigned bytes, Address offset, + Address align, Expression* ptr, Expression* value, Type type, @@ -463,13 +497,24 @@ class Builder { assert(order != MemoryOrder::Unordered && "Atomic stores can't be unordered"); - Store* store = makeStore(bytes, offset, bytes, ptr, value, type, memory); + Store* store = makeStore(bytes, offset, align, ptr, value, type, memory); store->order = order; return store; } + Store* makeAtomicStore(unsigned bytes, + Address offset, + Expression* ptr, + Expression* value, + Type type, + Name memory, + MemoryOrder order) { + return makeAtomicStore( + bytes, offset, bytes, ptr, value, type, memory, order); + } AtomicRMW* makeAtomicRMW(AtomicRMWOp op, unsigned bytes, Address offset, + Address align, Expression* ptr, Expression* value, Type type, @@ -479,6 +524,7 @@ class Builder { ret->op = op; ret->bytes = bytes; ret->offset = offset; + ret->align = align; ret->ptr = ptr; ret->value = value; ret->type = type; @@ -487,8 +533,20 @@ class Builder { ret->finalize(); return ret; } + AtomicRMW* makeAtomicRMW(AtomicRMWOp op, + unsigned bytes, + Address offset, + Expression* ptr, + Expression* value, + Type type, + Name memory, + MemoryOrder order) { + return makeAtomicRMW( + op, bytes, offset, bytes, ptr, value, type, memory, order); + } AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, Address offset, + Address align, Expression* ptr, Expression* expected, Expression* replacement, @@ -498,6 +556,7 @@ class Builder { auto* ret = wasm.allocator.alloc(); ret->bytes = bytes; ret->offset = offset; + ret->align = align; ret->ptr = ptr; ret->expected = expected; ret->replacement = replacement; @@ -507,6 +566,24 @@ class Builder { ret->finalize(); return ret; } + AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, + Address offset, + Expression* ptr, + Expression* expected, + Expression* replacement, + Type type, + Name memory, + MemoryOrder order) { + return makeAtomicCmpxchg(bytes, + offset, + bytes, + ptr, + expected, + replacement, + type, + memory, + order); + } SIMDExtract* makeSIMDExtract(SIMDExtractOp op, Expression* vec, uint8_t index) { auto* ret = wasm.allocator.alloc(); diff --git a/src/wasm-delegations-fields.def b/src/wasm-delegations-fields.def index d2de547c3d3..9455981a53e 100644 --- a/src/wasm-delegations-fields.def +++ b/src/wasm-delegations-fields.def @@ -383,6 +383,7 @@ DELEGATE_FIELD_CHILD(AtomicRMW, ptr) DELEGATE_FIELD_INT(AtomicRMW, op) DELEGATE_FIELD_INT(AtomicRMW, bytes) DELEGATE_FIELD_ADDRESS(AtomicRMW, offset) +DELEGATE_FIELD_ADDRESS(AtomicRMW, align) DELEGATE_FIELD_INT(AtomicRMW, order) DELEGATE_FIELD_NAME_KIND(AtomicRMW, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicRMW) @@ -393,6 +394,7 @@ DELEGATE_FIELD_CHILD(AtomicCmpxchg, expected) DELEGATE_FIELD_CHILD(AtomicCmpxchg, ptr) DELEGATE_FIELD_INT(AtomicCmpxchg, bytes) DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, offset) +DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, align) DELEGATE_FIELD_INT(AtomicCmpxchg, order) DELEGATE_FIELD_NAME_KIND(AtomicCmpxchg, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicCmpxchg) @@ -402,6 +404,7 @@ DELEGATE_FIELD_CHILD(AtomicWait, timeout) DELEGATE_FIELD_CHILD(AtomicWait, expected) DELEGATE_FIELD_CHILD(AtomicWait, ptr) DELEGATE_FIELD_ADDRESS(AtomicWait, offset) +DELEGATE_FIELD_ADDRESS(AtomicWait, align) DELEGATE_FIELD_TYPE(AtomicWait, expectedType) DELEGATE_FIELD_NAME_KIND(AtomicWait, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicWait) @@ -410,6 +413,7 @@ DELEGATE_FIELD_CASE_START(AtomicNotify) DELEGATE_FIELD_CHILD(AtomicNotify, notifyCount) DELEGATE_FIELD_CHILD(AtomicNotify, ptr) DELEGATE_FIELD_ADDRESS(AtomicNotify, offset) +DELEGATE_FIELD_ADDRESS(AtomicNotify, align) DELEGATE_FIELD_NAME_KIND(AtomicNotify, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicNotify) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 5998dc87428..6b68c27b686 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -155,20 +155,33 @@ class IRBuilder : public UnifiedExpressionVisitor> { Name mem); Result<> makeStore( unsigned bytes, Address offset, unsigned align, Type type, Name mem); - Result<> makeAtomicLoad( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); - Result<> makeAtomicStore( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); + Result<> makeAtomicLoad(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order); + Result<> makeAtomicStore(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order); Result<> makeAtomicRMW(AtomicRMWOp op, unsigned bytes, Address offset, + Address align, Type type, Name mem, MemoryOrder order); - Result<> makeAtomicCmpxchg( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order); - Result<> makeAtomicWait(Type type, Address offset, Name mem); - Result<> makeAtomicNotify(Address offset, Name mem); + Result<> makeAtomicCmpxchg(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order); + Result<> makeAtomicWait(Type type, Address offset, Address align, Name mem); + Result<> makeAtomicNotify(Address offset, Address align, Name mem); Result<> makeAtomicFence(MemoryOrder order); Result<> makePause(); Result<> makeSIMDExtract(SIMDExtractOp op, uint8_t lane); diff --git a/src/wasm.h b/src/wasm.h index a94e36cad6d..216d4f6de7a 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -1055,6 +1055,7 @@ class AtomicRMW : public SpecificExpression { AtomicRMWOp op; uint8_t bytes; Address offset; + Address align; Expression* ptr; Expression* value; Name memory; @@ -1070,6 +1071,7 @@ class AtomicCmpxchg : public SpecificExpression { uint8_t bytes; Address offset; + Address align; Expression* ptr; Expression* expected; Expression* replacement; @@ -1085,6 +1087,7 @@ class AtomicWait : public SpecificExpression { AtomicWait(MixedArena& allocator) : AtomicWait() {} Address offset; + Address align; Expression* ptr; Expression* expected; Expression* timeout; @@ -1100,6 +1103,7 @@ class AtomicNotify : public SpecificExpression { AtomicNotify(MixedArena& allocator) : AtomicNotify() {} Address offset; + Address align; Expression* ptr; Expression* notifyCount; Name memory; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index fc88a5dd997..a92dcd047e5 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -3934,105 +3934,111 @@ Result<> WasmBinaryReader::readInst() { auto op = getU32LEB(); switch (op) { case BinaryConsts::I32AtomicLoad8U: { - // TODO: pass align through for validation. auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(1, offset, Type::i32, mem, memoryOrder); + return builder.makeAtomicLoad( + 1, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicLoad16U: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(2, offset, Type::i32, mem, memoryOrder); + return builder.makeAtomicLoad( + 2, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicLoad: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(4, offset, Type::i32, mem, memoryOrder); + return builder.makeAtomicLoad( + 4, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I64AtomicLoad8U: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(1, offset, Type::i64, mem, memoryOrder); + return builder.makeAtomicLoad( + 1, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicLoad16U: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(2, offset, Type::i64, mem, memoryOrder); + return builder.makeAtomicLoad( + 2, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicLoad32U: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(4, offset, Type::i64, mem, memoryOrder); + return builder.makeAtomicLoad( + 4, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicLoad: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicLoad(8, offset, Type::i64, mem, memoryOrder); + return builder.makeAtomicLoad( + 8, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I32AtomicStore8: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 1, offset, Type::i32, mem, memoryOrder); + 1, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicStore16: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 2, offset, Type::i32, mem, memoryOrder); + 2, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicStore: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 4, offset, Type::i32, mem, memoryOrder); + 4, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I64AtomicStore8: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 1, offset, Type::i64, mem, memoryOrder); + 1, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore16: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 2, offset, Type::i64, mem, memoryOrder); + 2, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore32: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 4, offset, Type::i64, mem, memoryOrder); + 4, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicStore: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); return builder.makeAtomicStore( - 8, offset, Type::i64, mem, memoryOrder); + 8, offset, align, Type::i64, mem, memoryOrder); } #define RMW(op) \ case BinaryConsts::I32AtomicRMW##op: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ - return builder.makeAtomicRMW( \ - RMW##op, 4, offset, Type::i32, mem, memoryOrder); \ + return builder.makeAtomicRMW( \ + RMW##op, 4, offset, align, Type::i32, mem, memoryOrder); \ } \ case BinaryConsts::I32AtomicRMW##op##8U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 1, offset, Type::i32, mem, memoryOrder); \ + RMW##op, 1, offset, align, Type::i32, mem, memoryOrder); \ } \ case BinaryConsts::I32AtomicRMW##op##16U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 2, offset, Type::i32, mem, memoryOrder); \ + RMW##op, 2, offset, align, Type::i32, mem, memoryOrder); \ } \ case BinaryConsts::I64AtomicRMW##op: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 8, offset, Type::i64, mem, memoryOrder); \ + RMW##op, 8, offset, align, Type::i64, mem, memoryOrder); \ } \ case BinaryConsts::I64AtomicRMW##op##8U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 1, offset, Type::i64, mem, memoryOrder); \ + RMW##op, 1, offset, align, Type::i64, mem, memoryOrder); \ } \ case BinaryConsts::I64AtomicRMW##op##16U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 2, offset, Type::i64, mem, memoryOrder); \ + RMW##op, 2, offset, align, Type::i64, mem, memoryOrder); \ } \ case BinaryConsts::I64AtomicRMW##op##32U: { \ auto [mem, align, offset, memoryOrder] = getRMWMemarg(); \ return builder.makeAtomicRMW( \ - RMW##op, 4, offset, Type::i64, mem, memoryOrder); \ + RMW##op, 4, offset, align, Type::i64, mem, memoryOrder); \ } RMW(Add); @@ -4045,49 +4051,49 @@ Result<> WasmBinaryReader::readInst() { case BinaryConsts::I32AtomicCmpxchg: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 4, offset, Type::i32, mem, memoryOrder); + 4, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicCmpxchg8U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 1, offset, Type::i32, mem, memoryOrder); + 1, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I32AtomicCmpxchg16U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 2, offset, Type::i32, mem, memoryOrder); + 2, offset, align, Type::i32, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 8, offset, Type::i64, mem, memoryOrder); + 8, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg8U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 1, offset, Type::i64, mem, memoryOrder); + 1, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg16U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 2, offset, Type::i64, mem, memoryOrder); + 2, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I64AtomicCmpxchg32U: { auto [mem, align, offset, memoryOrder] = getRMWMemarg(); return builder.makeAtomicCmpxchg( - 4, offset, Type::i64, mem, memoryOrder); + 4, offset, align, Type::i64, mem, memoryOrder); } case BinaryConsts::I32AtomicWait: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicWait(Type::i32, offset, mem); + return builder.makeAtomicWait(Type::i32, offset, align, mem); } case BinaryConsts::I64AtomicWait: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicWait(Type::i64, offset, mem); + return builder.makeAtomicWait(Type::i64, offset, align, mem); } case BinaryConsts::AtomicNotify: { auto [mem, align, offset, memoryOrder] = getAtomicMemarg(); - return builder.makeAtomicNotify(offset, mem); + return builder.makeAtomicNotify(offset, align, mem); } case BinaryConsts::AtomicFence: { MemoryOrder order = getMemoryOrder(/*isRMW=*/false); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index e4c753fb220..5b4902207b5 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1634,29 +1634,39 @@ Result<> IRBuilder::makeStore( return Ok{}; } -Result<> IRBuilder::makeAtomicLoad( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) { +Result<> IRBuilder::makeAtomicLoad(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order) { Load curr; curr.memory = mem; CHECK_ERR(visitLoad(&curr)); - push(builder.makeAtomicLoad(bytes, offset, curr.ptr, type, mem, order)); + push(builder.makeAtomicLoad( + bytes, offset, align, curr.ptr, type, mem, order)); return Ok{}; } -Result<> IRBuilder::makeAtomicStore( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) { +Result<> IRBuilder::makeAtomicStore(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order) { Store curr; curr.memory = mem; curr.valueType = type; CHECK_ERR(visitStore(&curr)); push(builder.makeAtomicStore( - bytes, offset, curr.ptr, curr.value, type, mem, order)); + bytes, offset, align, curr.ptr, curr.value, type, mem, order)); return Ok{}; } Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op, unsigned bytes, Address offset, + Address align, Type type, Name mem, MemoryOrder order) { @@ -1665,17 +1675,22 @@ Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op, curr.type = type; CHECK_ERR(visitAtomicRMW(&curr)); push(builder.makeAtomicRMW( - op, bytes, offset, curr.ptr, curr.value, type, mem, order)); + op, bytes, offset, align, curr.ptr, curr.value, type, mem, order)); return Ok{}; } -Result<> IRBuilder::makeAtomicCmpxchg( - unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) { +Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes, + Address offset, + Address align, + Type type, + Name mem, + MemoryOrder order) { AtomicCmpxchg curr; curr.memory = mem; CHECK_ERR(ChildPopper{*this}.visitAtomicCmpxchg(&curr, type)); push(builder.makeAtomicCmpxchg(bytes, offset, + align, curr.ptr, curr.expected, curr.replacement, @@ -1685,21 +1700,21 @@ Result<> IRBuilder::makeAtomicCmpxchg( return Ok{}; } -Result<> IRBuilder::makeAtomicWait(Type type, Address offset, Name mem) { +Result<> IRBuilder::makeAtomicWait(Type type, Address offset, Address align, Name mem) { AtomicWait curr; curr.memory = mem; curr.expectedType = type; CHECK_ERR(visitAtomicWait(&curr)); push(builder.makeAtomicWait( - curr.ptr, curr.expected, curr.timeout, type, offset, mem)); + curr.ptr, curr.expected, curr.timeout, type, offset, align, mem)); return Ok{}; } -Result<> IRBuilder::makeAtomicNotify(Address offset, Name mem) { +Result<> IRBuilder::makeAtomicNotify(Address offset, Address align, Name mem) { AtomicNotify curr; curr.memory = mem; CHECK_ERR(visitAtomicNotify(&curr)); - push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, mem)); + push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, align, mem)); return Ok{}; } diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 5f1c6a13e8d..760be6015e4 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -548,7 +548,7 @@ void BinaryInstWriter::visitAtomicRMW(AtomicRMW* curr) { default: WASM_UNREACHABLE("unexpected op"); } - emitMemoryAccess(curr->bytes, + emitMemoryAccess(curr->align, curr->bytes, curr->offset, curr->memory, @@ -595,7 +595,7 @@ void BinaryInstWriter::visitAtomicCmpxchg(AtomicCmpxchg* curr) { default: WASM_UNREACHABLE("unexpected type"); } - emitMemoryAccess(curr->bytes, + emitMemoryAccess(curr->align, curr->bytes, curr->offset, curr->memory, @@ -608,14 +608,22 @@ void BinaryInstWriter::visitAtomicWait(AtomicWait* curr) { switch (curr->expectedType.getBasic()) { case Type::i32: { o << static_cast(BinaryConsts::I32AtomicWait); - emitMemoryAccess( - 4, 4, curr->offset, curr->memory, MemoryOrder::SeqCst, /*isRMW=*/false); + emitMemoryAccess(curr->align, + 4, + curr->offset, + curr->memory, + MemoryOrder::SeqCst, + /*isRMW=*/false); break; } case Type::i64: { o << static_cast(BinaryConsts::I64AtomicWait); - emitMemoryAccess( - 8, 8, curr->offset, curr->memory, MemoryOrder::SeqCst, /*isRMW=*/false); + emitMemoryAccess(curr->align, + 8, + curr->offset, + curr->memory, + MemoryOrder::SeqCst, + /*isRMW=*/false); break; } default: @@ -626,8 +634,12 @@ void BinaryInstWriter::visitAtomicWait(AtomicWait* curr) { void BinaryInstWriter::visitAtomicNotify(AtomicNotify* curr) { o << static_cast(BinaryConsts::AtomicPrefix) << static_cast(BinaryConsts::AtomicNotify); - emitMemoryAccess( - 4, 4, curr->offset, curr->memory, MemoryOrder::SeqCst, /*isRMW=*/false); + emitMemoryAccess(curr->align, + 4, + curr->offset, + curr->memory, + MemoryOrder::SeqCst, + /*isRMW=*/false); } void BinaryInstWriter::visitAtomicFence(AtomicFence* curr) { diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 56237a3d732..aacd7ee5368 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -1361,6 +1361,8 @@ void FunctionValidator::visitAtomicRMW(AtomicRMW* curr) { "AtomicRMW result type must match operand"); shouldBeIntOrUnreachable( curr->type, curr, "Atomic operations are only valid on int types"); + validateAlignment( + curr->align, curr->type, curr->bytes, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) { @@ -1423,6 +1425,8 @@ void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) { shouldBeIntOrUnreachable(curr->expected->type, curr, "Atomic operations are only valid on int types"); + validateAlignment( + curr->align, curr->type, curr->bytes, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicWait(AtomicWait* curr) { @@ -1449,6 +1453,11 @@ void FunctionValidator::visitAtomicWait(AtomicWait* curr) { Type(Type::i64), curr, "AtomicWait timeout type must be i64"); + validateAlignment(curr->align, + curr->expectedType, + curr->expectedType.getByteSize(), + /*isAtomic=*/true, + curr); } void FunctionValidator::visitAtomicNotify(AtomicNotify* curr) { @@ -1469,6 +1478,8 @@ void FunctionValidator::visitAtomicNotify(AtomicNotify* curr) { Type(Type::i32), curr, "AtomicNotify notifyCount type must be i32"); + validateAlignment( + curr->align, Type::i32, 4, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicFence(AtomicFence* curr) { diff --git a/test/lit/passes/dealign-atomics.wast b/test/lit/passes/dealign-atomics.wast new file mode 100644 index 00000000000..3bf612d2f0b --- /dev/null +++ b/test/lit/passes/dealign-atomics.wast @@ -0,0 +1,19 @@ +;; RUN: wasm-opt %s --enable-threads --dealign -S -o - | filecheck %s + +(module + (memory 1 1 shared) + + (func $test + (drop (i32.load align=4 (i32.const 0))) + (drop (i32.atomic.load (i32.const 4))) + (i32.store align=4 (i32.const 8) (i32.const 0)) + (i32.atomic.store (i32.const 12) (i32.const 0)) + ) +) + +;; CHECK: (i32.load align=1 +;; CHECK: (i32.atomic.load +;; CHECK-NOT: i32.atomic.load align=1 +;; CHECK: (i32.store align=1 +;; CHECK: (i32.atomic.store +;; CHECK-NOT: i32.atomic.store align=1 diff --git a/test/lit/validation/atomic-alignment.wast b/test/lit/validation/atomic-alignment.wast new file mode 100644 index 00000000000..2ef5bfbdde7 --- /dev/null +++ b/test/lit/validation/atomic-alignment.wast @@ -0,0 +1,39 @@ +;; RUN: not wasm-opt %s --enable-threads -o /dev/null 2>&1 | filecheck %s --check-prefix=CHECK-VAL +;; RUN: wasm-as %s --enable-threads --validate=none -o %t.wasm +;; RUN: wasm-dis %t.wasm -o - | filecheck %s --check-prefix=CHECK-DIS +;; RUN: not wasm-opt %t.wasm --enable-threads -o /dev/null 2>&1 | filecheck %s --check-prefix=CHECK-VAL + +(module + (memory 1 1 shared) + + (func $bad-load (result i32) + (i32.atomic.load align=1 (i32.const 0)) + ) + (func $bad-store + (i32.atomic.store align=2 (i32.const 0) (i32.const 0)) + ) + (func $bad-rmw (result i32) + (i32.atomic.rmw.add align=1 (i32.const 0) (i32.const 0)) + ) + (func $bad-cmpxchg (result i32) + (i32.atomic.rmw.cmpxchg align=1 (i32.const 0) (i32.const 0) (i32.const 0)) + ) + (func $bad-wait32 (result i32) + (memory.atomic.wait32 align=1 (i32.const 0) (i32.const 0) (i64.const 0)) + ) + (func $bad-wait64 (result i32) + (memory.atomic.wait64 align=4 (i32.const 0) (i64.const 0) (i64.const 0)) + ) + (func $bad-notify (result i32) + (memory.atomic.notify align=1 (i32.const 0) (i32.const 0)) + ) + (func $bad-packed (result i64) + (i64.atomic.load32_u align=8 (i32.const 0)) + ) +) + +;; CHECK-VAL: atomic accesses must have natural alignment + +;; CHECK-DIS: align=1 +;; CHECK-DIS: align=2 +;; CHECK-DIS: align=8 From c2fe05a8a3018596c4a94639786d0fc96571396e Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Sun, 16 Aug 2026 12:21:03 +0530 Subject: [PATCH 2/2] Reject non-natural atomic alignment at parse time Store only natural alignment in IR for RMW/cmpxchg/wait/notify and fail in IRBuilder so WAT and binary both reject invalid memarg align without extra AST fields. Fixes #8962. Signed-off-by: Gaurav Chaudhary --- CHANGELOG.md | 2 +- src/passes/Print.cpp | 13 --- src/wasm-builder.h | 81 +------------------ src/wasm-delegations-fields.def | 4 - src/wasm.h | 4 - src/wasm/wasm-ir-builder.cpp | 25 ++++-- src/wasm/wasm-stack.cpp | 10 +-- src/wasm/wasm-validator.cpp | 11 --- test/lit/basic/relaxed-atomics.wast | 32 ++++---- test/lit/validation/atomic-alignment.wast | 98 ++++++++++++++--------- test/lit/validation/relaxed-atomics.wast | 2 +- 11 files changed, 102 insertions(+), 180 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30abb07e039..d6b25c71a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ full changeset diff at the end of each section. Current Trunk ------------- -- Reject non-natural alignment for atomic memory operations (#8962) +- Reject non-natural alignment for atomic memory operations at parse time (#8962) v132 ---- diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 8eae30e770d..50600001e9a 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -613,9 +613,6 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } - if (curr->align != curr->bytes) { - o << " align=" << curr->align; - } } void visitAtomicCmpxchg(AtomicCmpxchg* curr) { prepareColor(o); @@ -631,9 +628,6 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } - if (curr->align != curr->bytes) { - o << " align=" << curr->align; - } } void visitAtomicWait(AtomicWait* curr) { prepareColor(o); @@ -645,10 +639,6 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } - Index natural = type == Type::i32 ? 4 : 8; - if (curr->align != natural) { - o << " align=" << curr->align; - } } void visitAtomicNotify(AtomicNotify* curr) { printMedium(o, "memory.atomic.notify"); @@ -656,9 +646,6 @@ struct PrintExpressionContents if (curr->offset) { o << " offset=" << curr->offset; } - if (curr->align != 4) { - o << " align=" << curr->align; - } } void visitAtomicFence(AtomicFence* curr) { printMedium(o, "atomic.fence"); diff --git a/src/wasm-builder.h b/src/wasm-builder.h index c3064fc411b..3b4cbb2d7ef 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -389,7 +389,6 @@ class Builder { } Load* makeAtomicLoad(unsigned bytes, Address offset, - Address align, Expression* ptr, Type type, Name memory, @@ -397,28 +396,18 @@ class Builder { assert(order != MemoryOrder::Unordered && "Atomic loads can't be unordered"); - Load* load = makeLoad(bytes, false, offset, align, ptr, type, memory); + Load* load = makeLoad(bytes, false, offset, bytes, ptr, type, memory); load->order = order; return load; } - Load* makeAtomicLoad(unsigned bytes, - Address offset, - Expression* ptr, - Type type, - Name memory, - MemoryOrder order) { - return makeAtomicLoad(bytes, offset, bytes, ptr, type, memory, order); - } AtomicWait* makeAtomicWait(Expression* ptr, Expression* expected, Expression* timeout, Type expectedType, Address offset, - Address align, Name memory) { auto* wait = wasm.allocator.alloc(); wait->offset = offset; - wait->align = align; wait->ptr = ptr; wait->expected = expected; wait->timeout = timeout; @@ -427,40 +416,18 @@ class Builder { wait->memory = memory; return wait; } - AtomicWait* makeAtomicWait(Expression* ptr, - Expression* expected, - Expression* timeout, - Type expectedType, - Address offset, - Name memory) { - return makeAtomicWait(ptr, - expected, - timeout, - expectedType, - offset, - expectedType.getByteSize(), - memory); - } AtomicNotify* makeAtomicNotify(Expression* ptr, Expression* notifyCount, Address offset, - Address align, Name memory) { auto* notify = wasm.allocator.alloc(); notify->offset = offset; - notify->align = align; notify->ptr = ptr; notify->notifyCount = notifyCount; notify->finalize(); notify->memory = memory; return notify; } - AtomicNotify* makeAtomicNotify(Expression* ptr, - Expression* notifyCount, - Address offset, - Name memory) { - return makeAtomicNotify(ptr, notifyCount, offset, 4, memory); - } AtomicFence* makeAtomicFence(MemoryOrder order) { auto* ret = wasm.allocator.alloc(); ret->order = order; @@ -488,7 +455,6 @@ class Builder { } Store* makeAtomicStore(unsigned bytes, Address offset, - Address align, Expression* ptr, Expression* value, Type type, @@ -497,24 +463,13 @@ class Builder { assert(order != MemoryOrder::Unordered && "Atomic stores can't be unordered"); - Store* store = makeStore(bytes, offset, align, ptr, value, type, memory); + Store* store = makeStore(bytes, offset, bytes, ptr, value, type, memory); store->order = order; return store; } - Store* makeAtomicStore(unsigned bytes, - Address offset, - Expression* ptr, - Expression* value, - Type type, - Name memory, - MemoryOrder order) { - return makeAtomicStore( - bytes, offset, bytes, ptr, value, type, memory, order); - } AtomicRMW* makeAtomicRMW(AtomicRMWOp op, unsigned bytes, Address offset, - Address align, Expression* ptr, Expression* value, Type type, @@ -524,7 +479,6 @@ class Builder { ret->op = op; ret->bytes = bytes; ret->offset = offset; - ret->align = align; ret->ptr = ptr; ret->value = value; ret->type = type; @@ -533,20 +487,8 @@ class Builder { ret->finalize(); return ret; } - AtomicRMW* makeAtomicRMW(AtomicRMWOp op, - unsigned bytes, - Address offset, - Expression* ptr, - Expression* value, - Type type, - Name memory, - MemoryOrder order) { - return makeAtomicRMW( - op, bytes, offset, bytes, ptr, value, type, memory, order); - } AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, Address offset, - Address align, Expression* ptr, Expression* expected, Expression* replacement, @@ -556,7 +498,6 @@ class Builder { auto* ret = wasm.allocator.alloc(); ret->bytes = bytes; ret->offset = offset; - ret->align = align; ret->ptr = ptr; ret->expected = expected; ret->replacement = replacement; @@ -566,24 +507,6 @@ class Builder { ret->finalize(); return ret; } - AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, - Address offset, - Expression* ptr, - Expression* expected, - Expression* replacement, - Type type, - Name memory, - MemoryOrder order) { - return makeAtomicCmpxchg(bytes, - offset, - bytes, - ptr, - expected, - replacement, - type, - memory, - order); - } SIMDExtract* makeSIMDExtract(SIMDExtractOp op, Expression* vec, uint8_t index) { auto* ret = wasm.allocator.alloc(); diff --git a/src/wasm-delegations-fields.def b/src/wasm-delegations-fields.def index 9455981a53e..d2de547c3d3 100644 --- a/src/wasm-delegations-fields.def +++ b/src/wasm-delegations-fields.def @@ -383,7 +383,6 @@ DELEGATE_FIELD_CHILD(AtomicRMW, ptr) DELEGATE_FIELD_INT(AtomicRMW, op) DELEGATE_FIELD_INT(AtomicRMW, bytes) DELEGATE_FIELD_ADDRESS(AtomicRMW, offset) -DELEGATE_FIELD_ADDRESS(AtomicRMW, align) DELEGATE_FIELD_INT(AtomicRMW, order) DELEGATE_FIELD_NAME_KIND(AtomicRMW, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicRMW) @@ -394,7 +393,6 @@ DELEGATE_FIELD_CHILD(AtomicCmpxchg, expected) DELEGATE_FIELD_CHILD(AtomicCmpxchg, ptr) DELEGATE_FIELD_INT(AtomicCmpxchg, bytes) DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, offset) -DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, align) DELEGATE_FIELD_INT(AtomicCmpxchg, order) DELEGATE_FIELD_NAME_KIND(AtomicCmpxchg, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicCmpxchg) @@ -404,7 +402,6 @@ DELEGATE_FIELD_CHILD(AtomicWait, timeout) DELEGATE_FIELD_CHILD(AtomicWait, expected) DELEGATE_FIELD_CHILD(AtomicWait, ptr) DELEGATE_FIELD_ADDRESS(AtomicWait, offset) -DELEGATE_FIELD_ADDRESS(AtomicWait, align) DELEGATE_FIELD_TYPE(AtomicWait, expectedType) DELEGATE_FIELD_NAME_KIND(AtomicWait, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicWait) @@ -413,7 +410,6 @@ DELEGATE_FIELD_CASE_START(AtomicNotify) DELEGATE_FIELD_CHILD(AtomicNotify, notifyCount) DELEGATE_FIELD_CHILD(AtomicNotify, ptr) DELEGATE_FIELD_ADDRESS(AtomicNotify, offset) -DELEGATE_FIELD_ADDRESS(AtomicNotify, align) DELEGATE_FIELD_NAME_KIND(AtomicNotify, memory, ModuleItemKind::Memory) DELEGATE_FIELD_CASE_END(AtomicNotify) diff --git a/src/wasm.h b/src/wasm.h index 216d4f6de7a..a94e36cad6d 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -1055,7 +1055,6 @@ class AtomicRMW : public SpecificExpression { AtomicRMWOp op; uint8_t bytes; Address offset; - Address align; Expression* ptr; Expression* value; Name memory; @@ -1071,7 +1070,6 @@ class AtomicCmpxchg : public SpecificExpression { uint8_t bytes; Address offset; - Address align; Expression* ptr; Expression* expected; Expression* replacement; @@ -1087,7 +1085,6 @@ class AtomicWait : public SpecificExpression { AtomicWait(MixedArena& allocator) : AtomicWait() {} Address offset; - Address align; Expression* ptr; Expression* expected; Expression* timeout; @@ -1103,7 +1100,6 @@ class AtomicNotify : public SpecificExpression { AtomicNotify(MixedArena& allocator) : AtomicNotify() {} Address offset; - Address align; Expression* ptr; Expression* notifyCount; Name memory; diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 5b4902207b5..e559e39a38b 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -92,6 +92,13 @@ Result<> validateTypeAnnotation(HeapType type, Expression* child) { return validateTypeAnnotation(Type(type, Nullable), child); } +Result<> requireNaturalAtomicAlign(Address align, Address natural) { + if (align != natural) { + return Err{"atomic accesses must have natural alignment"}; + } + return Ok{}; +} + } // anonymous namespace Result IRBuilder::addScratchLocal(Type type) { @@ -1640,11 +1647,11 @@ Result<> IRBuilder::makeAtomicLoad(unsigned bytes, Type type, Name mem, MemoryOrder order) { + CHECK_ERR(requireNaturalAtomicAlign(align, bytes)); Load curr; curr.memory = mem; CHECK_ERR(visitLoad(&curr)); - push(builder.makeAtomicLoad( - bytes, offset, align, curr.ptr, type, mem, order)); + push(builder.makeAtomicLoad(bytes, offset, curr.ptr, type, mem, order)); return Ok{}; } @@ -1654,12 +1661,13 @@ Result<> IRBuilder::makeAtomicStore(unsigned bytes, Type type, Name mem, MemoryOrder order) { + CHECK_ERR(requireNaturalAtomicAlign(align, bytes)); Store curr; curr.memory = mem; curr.valueType = type; CHECK_ERR(visitStore(&curr)); push(builder.makeAtomicStore( - bytes, offset, align, curr.ptr, curr.value, type, mem, order)); + bytes, offset, curr.ptr, curr.value, type, mem, order)); return Ok{}; } @@ -1670,12 +1678,13 @@ Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op, Type type, Name mem, MemoryOrder order) { + CHECK_ERR(requireNaturalAtomicAlign(align, bytes)); AtomicRMW curr; curr.memory = mem; curr.type = type; CHECK_ERR(visitAtomicRMW(&curr)); push(builder.makeAtomicRMW( - op, bytes, offset, align, curr.ptr, curr.value, type, mem, order)); + op, bytes, offset, curr.ptr, curr.value, type, mem, order)); return Ok{}; } @@ -1685,12 +1694,12 @@ Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes, Type type, Name mem, MemoryOrder order) { + CHECK_ERR(requireNaturalAtomicAlign(align, bytes)); AtomicCmpxchg curr; curr.memory = mem; CHECK_ERR(ChildPopper{*this}.visitAtomicCmpxchg(&curr, type)); push(builder.makeAtomicCmpxchg(bytes, offset, - align, curr.ptr, curr.expected, curr.replacement, @@ -1701,20 +1710,22 @@ Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes, } Result<> IRBuilder::makeAtomicWait(Type type, Address offset, Address align, Name mem) { + CHECK_ERR(requireNaturalAtomicAlign(align, type == Type::i32 ? 4 : 8)); AtomicWait curr; curr.memory = mem; curr.expectedType = type; CHECK_ERR(visitAtomicWait(&curr)); push(builder.makeAtomicWait( - curr.ptr, curr.expected, curr.timeout, type, offset, align, mem)); + curr.ptr, curr.expected, curr.timeout, type, offset, mem)); return Ok{}; } Result<> IRBuilder::makeAtomicNotify(Address offset, Address align, Name mem) { + CHECK_ERR(requireNaturalAtomicAlign(align, 4)); AtomicNotify curr; curr.memory = mem; CHECK_ERR(visitAtomicNotify(&curr)); - push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, align, mem)); + push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, mem)); return Ok{}; } diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 760be6015e4..29d9a38c15b 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -548,7 +548,7 @@ void BinaryInstWriter::visitAtomicRMW(AtomicRMW* curr) { default: WASM_UNREACHABLE("unexpected op"); } - emitMemoryAccess(curr->align, + emitMemoryAccess(curr->bytes, curr->bytes, curr->offset, curr->memory, @@ -595,7 +595,7 @@ void BinaryInstWriter::visitAtomicCmpxchg(AtomicCmpxchg* curr) { default: WASM_UNREACHABLE("unexpected type"); } - emitMemoryAccess(curr->align, + emitMemoryAccess(curr->bytes, curr->bytes, curr->offset, curr->memory, @@ -608,7 +608,7 @@ void BinaryInstWriter::visitAtomicWait(AtomicWait* curr) { switch (curr->expectedType.getBasic()) { case Type::i32: { o << static_cast(BinaryConsts::I32AtomicWait); - emitMemoryAccess(curr->align, + emitMemoryAccess(4, 4, curr->offset, curr->memory, @@ -618,7 +618,7 @@ void BinaryInstWriter::visitAtomicWait(AtomicWait* curr) { } case Type::i64: { o << static_cast(BinaryConsts::I64AtomicWait); - emitMemoryAccess(curr->align, + emitMemoryAccess(8, 8, curr->offset, curr->memory, @@ -634,7 +634,7 @@ void BinaryInstWriter::visitAtomicWait(AtomicWait* curr) { void BinaryInstWriter::visitAtomicNotify(AtomicNotify* curr) { o << static_cast(BinaryConsts::AtomicPrefix) << static_cast(BinaryConsts::AtomicNotify); - emitMemoryAccess(curr->align, + emitMemoryAccess(4, 4, curr->offset, curr->memory, diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index aacd7ee5368..56237a3d732 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -1361,8 +1361,6 @@ void FunctionValidator::visitAtomicRMW(AtomicRMW* curr) { "AtomicRMW result type must match operand"); shouldBeIntOrUnreachable( curr->type, curr, "Atomic operations are only valid on int types"); - validateAlignment( - curr->align, curr->type, curr->bytes, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) { @@ -1425,8 +1423,6 @@ void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) { shouldBeIntOrUnreachable(curr->expected->type, curr, "Atomic operations are only valid on int types"); - validateAlignment( - curr->align, curr->type, curr->bytes, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicWait(AtomicWait* curr) { @@ -1453,11 +1449,6 @@ void FunctionValidator::visitAtomicWait(AtomicWait* curr) { Type(Type::i64), curr, "AtomicWait timeout type must be i64"); - validateAlignment(curr->align, - curr->expectedType, - curr->expectedType.getByteSize(), - /*isAtomic=*/true, - curr); } void FunctionValidator::visitAtomicNotify(AtomicNotify* curr) { @@ -1478,8 +1469,6 @@ void FunctionValidator::visitAtomicNotify(AtomicNotify* curr) { Type(Type::i32), curr, "AtomicNotify notifyCount type must be i32"); - validateAlignment( - curr->align, Type::i32, 4, /*isAtomic=*/true, curr); } void FunctionValidator::visitAtomicFence(AtomicFence* curr) { diff --git a/test/lit/basic/relaxed-atomics.wast b/test/lit/basic/relaxed-atomics.wast index 0484c8465b8..c9994e43d94 100644 --- a/test/lit/basic/relaxed-atomics.wast +++ b/test/lit/basic/relaxed-atomics.wast @@ -8,21 +8,21 @@ ;; RTRIP: (func $acqrel (type $0) ;; RTRIP-NEXT: (atomic.fence acqrel) ;; RTRIP-NEXT: (i32.atomic.store acqrel - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: (i32.const 1) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: (drop ;; RTRIP-NEXT: (i32.atomic.load acqrel - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) (func $acqrel (atomic.fence acqrel) - (i32.atomic.store acqrel (i32.const 1) (i32.const 1)) + (i32.atomic.store acqrel (i32.const 0) (i32.const 1)) (drop (i32.atomic.load acqrel - (i32.const 1) + (i32.const 0) )) ) @@ -30,56 +30,56 @@ ;; RTRIP: (func $seqcst (type $0) ;; RTRIP-NEXT: (atomic.fence) ;; RTRIP-NEXT: (i32.atomic.store - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: (i32.const 1) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: (i32.atomic.store - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: (i32.const 1) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: (drop ;; RTRIP-NEXT: (i32.atomic.load - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: (drop ;; RTRIP-NEXT: (i32.atomic.load - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) (func $seqcst (atomic.fence seqcst) - (i32.atomic.store seqcst (i32.const 1) (i32.const 1)) - (i32.atomic.store 0 seqcst (i32.const 1) (i32.const 1)) + (i32.atomic.store seqcst (i32.const 0) (i32.const 1)) + (i32.atomic.store 0 seqcst (i32.const 0) (i32.const 1)) (drop (i32.atomic.load seqcst - (i32.const 1) + (i32.const 0) )) ;; allows memory index before memory ordering immediate (drop (i32.atomic.load 0 seqcst - (i32.const 1) + (i32.const 0) )) ) ;; RTRIP: (func $relaxed (type $0) ;; RTRIP-NEXT: (atomic.fence relaxed) ;; RTRIP-NEXT: (i32.atomic.store relaxed - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: (i32.const 1) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: (drop ;; RTRIP-NEXT: (i32.atomic.load relaxed - ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: (i32.const 0) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) ;; RTRIP-NEXT: ) (func $relaxed (atomic.fence relaxed) - (i32.atomic.store relaxed (i32.const 1) (i32.const 1)) + (i32.atomic.store relaxed (i32.const 0) (i32.const 1)) (drop (i32.atomic.load relaxed - (i32.const 1) + (i32.const 0) ) ) ) diff --git a/test/lit/validation/atomic-alignment.wast b/test/lit/validation/atomic-alignment.wast index 2ef5bfbdde7..0b424259282 100644 --- a/test/lit/validation/atomic-alignment.wast +++ b/test/lit/validation/atomic-alignment.wast @@ -1,39 +1,59 @@ -;; RUN: not wasm-opt %s --enable-threads -o /dev/null 2>&1 | filecheck %s --check-prefix=CHECK-VAL -;; RUN: wasm-as %s --enable-threads --validate=none -o %t.wasm -;; RUN: wasm-dis %t.wasm -o - | filecheck %s --check-prefix=CHECK-DIS -;; RUN: not wasm-opt %t.wasm --enable-threads -o /dev/null 2>&1 | filecheck %s --check-prefix=CHECK-VAL - -(module - (memory 1 1 shared) - - (func $bad-load (result i32) - (i32.atomic.load align=1 (i32.const 0)) - ) - (func $bad-store - (i32.atomic.store align=2 (i32.const 0) (i32.const 0)) - ) - (func $bad-rmw (result i32) - (i32.atomic.rmw.add align=1 (i32.const 0) (i32.const 0)) - ) - (func $bad-cmpxchg (result i32) - (i32.atomic.rmw.cmpxchg align=1 (i32.const 0) (i32.const 0) (i32.const 0)) - ) - (func $bad-wait32 (result i32) - (memory.atomic.wait32 align=1 (i32.const 0) (i32.const 0) (i64.const 0)) - ) - (func $bad-wait64 (result i32) - (memory.atomic.wait64 align=4 (i32.const 0) (i64.const 0) (i64.const 0)) - ) - (func $bad-notify (result i32) - (memory.atomic.notify align=1 (i32.const 0) (i32.const 0)) - ) - (func $bad-packed (result i64) - (i64.atomic.load32_u align=8 (i32.const 0)) - ) -) - -;; CHECK-VAL: atomic accesses must have natural alignment - -;; CHECK-DIS: align=1 -;; CHECK-DIS: align=2 -;; CHECK-DIS: align=8 +;; Non-natural alignment on atomic memory ops is a parse error. + +;; RUN: foreach %s %t not wasm-opt --enable-threads -o /dev/null 2>&1 | filecheck %s + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (i32.atomic.load align=1 (i32.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func + (i32.atomic.store align=2 (i32.const 0) (i32.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (i32.atomic.rmw.add align=1 (i32.const 0) (i32.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (i32.atomic.rmw.cmpxchg align=1 (i32.const 0) (i32.const 0) (i32.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (memory.atomic.wait32 align=1 (i32.const 0) (i32.const 0) (i64.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (memory.atomic.wait64 align=4 (i32.const 0) (i64.const 0) (i64.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i32) + (memory.atomic.notify align=1 (i32.const 0) (i32.const 0))) +) + +;; CHECK: Fatal: {{.*}}: error: atomic accesses must have natural alignment +(module + (memory 1 1 shared) + (func (result i64) + (i64.atomic.load32_u align=8 (i32.const 0))) +) diff --git a/test/lit/validation/relaxed-atomics.wast b/test/lit/validation/relaxed-atomics.wast index aa316cda94a..5f419190fe2 100644 --- a/test/lit/validation/relaxed-atomics.wast +++ b/test/lit/validation/relaxed-atomics.wast @@ -14,7 +14,7 @@ (func $relaxed (result i32) ;; CHECK: Relaxed operations require relaxed atomics [--enable-relaxed-atomics] (i32.atomic.load relaxed - (i32.const 1) + (i32.const 0) ) ) )