Validate natural alignment for atomic memory operations

CHANGELOG.md

@@ -15,6 +15,7 @@ full changeset diff at the end of each section.
 Current Trunk
 -------------
 
+- Reject non-natural alignment for atomic memory operations (#8962)
 - [JS API] support `BinaryenStringConst` (#8951)
 - [JS API] Replace `Module['readBinaryWithFeatures']` with an
   optional `features` parameter to `Module['readBinary']` (#8954)

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 = [
 ]

src/parser/contexts.h

@@ -2394,8 +2394,9 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, 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<ParseDefsCtx>, 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<ParseDefsCtx>, 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<ParseDefsCtx>, 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<ParseDefsCtx>, 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<ParseDefsCtx>, 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,

src/passes/DeAlign.cpp

@@ -31,9 +31,19 @@ struct DeAlign : public WalkerPass<PostWalker<DeAlign>> {
     return std::make_unique<DeAlign>();
   }
 
-  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; }
 

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,13 +645,20 @@ 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");
     printMemoryName(curr->memory, o, wasm);
     if (curr->offset) {
       o << " offset=" << curr->offset;
     }
+    if (curr->align != 4) {
+      o << " align=" << curr->align;
+    }
   }
   void visitAtomicFence(AtomicFence* curr) {
     printMedium(o, "atomic.fence");

src/wasm-builder.h

@@ -389,25 +389,36 @@ class Builder {
   }
   Load* makeAtomicLoad(unsigned bytes,
                        Address offset,
+                       Address align,
                        Expression* ptr,
                        Type type,
                        Name memory,
                        MemoryOrder order) {
     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<AtomicWait>();
     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<AtomicNotify>();
     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<AtomicFence>();
     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<AtomicCmpxchg>();
     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<SIMDExtract>();

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)