Skip to content

Commit c2fe05a

Browse files
ANAMASGARDANAMASGARD
committed
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 <chaudharygaurav2004@gmail.com>
1 parent ccb7c21 commit c2fe05a

11 files changed

Lines changed: 102 additions & 180 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ full changeset diff at the end of each section.
1515
Current Trunk
1616
-------------
1717

18-
- Reject non-natural alignment for atomic memory operations (#8962)
18+
- Reject non-natural alignment for atomic memory operations at parse time (#8962)
1919

2020
v132
2121
----

‎src/passes/Print.cpp‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -613,9 +613,6 @@ struct PrintExpressionContents
613613
if (curr->offset) {
614614
o << " offset=" << curr->offset;
615615
}
616-
if (curr->align != curr->bytes) {
617-
o << " align=" << curr->align;
618-
}
619616
}
620617
void visitAtomicCmpxchg(AtomicCmpxchg* curr) {
621618
prepareColor(o);
@@ -631,9 +628,6 @@ struct PrintExpressionContents
631628
if (curr->offset) {
632629
o << " offset=" << curr->offset;
633630
}
634-
if (curr->align != curr->bytes) {
635-
o << " align=" << curr->align;
636-
}
637631
}
638632
void visitAtomicWait(AtomicWait* curr) {
639633
prepareColor(o);
@@ -645,20 +639,13 @@ struct PrintExpressionContents
645639
if (curr->offset) {
646640
o << " offset=" << curr->offset;
647641
}
648-
Index natural = type == Type::i32 ? 4 : 8;
649-
if (curr->align != natural) {
650-
o << " align=" << curr->align;
651-
}
652642
}
653643
void visitAtomicNotify(AtomicNotify* curr) {
654644
printMedium(o, "memory.atomic.notify");
655645
printMemoryName(curr->memory, o, wasm);
656646
if (curr->offset) {
657647
o << " offset=" << curr->offset;
658648
}
659-
if (curr->align != 4) {
660-
o << " align=" << curr->align;
661-
}
662649
}
663650
void visitAtomicFence(AtomicFence* curr) {
664651
printMedium(o, "atomic.fence");

‎src/wasm-builder.h‎

Lines changed: 2 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -389,36 +389,25 @@ class Builder {
389389
}
390390
Load* makeAtomicLoad(unsigned bytes,
391391
Address offset,
392-
Address align,
393392
Expression* ptr,
394393
Type type,
395394
Name memory,
396395
MemoryOrder order) {
397396
assert(order != MemoryOrder::Unordered &&
398397
"Atomic loads can't be unordered");
399398

400-
Load* load = makeLoad(bytes, false, offset, align, ptr, type, memory);
399+
Load* load = makeLoad(bytes, false, offset, bytes, ptr, type, memory);
401400
load->order = order;
402401
return load;
403402
}
404-
Load* makeAtomicLoad(unsigned bytes,
405-
Address offset,
406-
Expression* ptr,
407-
Type type,
408-
Name memory,
409-
MemoryOrder order) {
410-
return makeAtomicLoad(bytes, offset, bytes, ptr, type, memory, order);
411-
}
412403
AtomicWait* makeAtomicWait(Expression* ptr,
413404
Expression* expected,
414405
Expression* timeout,
415406
Type expectedType,
416407
Address offset,
417-
Address align,
418408
Name memory) {
419409
auto* wait = wasm.allocator.alloc<AtomicWait>();
420410
wait->offset = offset;
421-
wait->align = align;
422411
wait->ptr = ptr;
423412
wait->expected = expected;
424413
wait->timeout = timeout;
@@ -427,40 +416,18 @@ class Builder {
427416
wait->memory = memory;
428417
return wait;
429418
}
430-
AtomicWait* makeAtomicWait(Expression* ptr,
431-
Expression* expected,
432-
Expression* timeout,
433-
Type expectedType,
434-
Address offset,
435-
Name memory) {
436-
return makeAtomicWait(ptr,
437-
expected,
438-
timeout,
439-
expectedType,
440-
offset,
441-
expectedType.getByteSize(),
442-
memory);
443-
}
444419
AtomicNotify* makeAtomicNotify(Expression* ptr,
445420
Expression* notifyCount,
446421
Address offset,
447-
Address align,
448422
Name memory) {
449423
auto* notify = wasm.allocator.alloc<AtomicNotify>();
450424
notify->offset = offset;
451-
notify->align = align;
452425
notify->ptr = ptr;
453426
notify->notifyCount = notifyCount;
454427
notify->finalize();
455428
notify->memory = memory;
456429
return notify;
457430
}
458-
AtomicNotify* makeAtomicNotify(Expression* ptr,
459-
Expression* notifyCount,
460-
Address offset,
461-
Name memory) {
462-
return makeAtomicNotify(ptr, notifyCount, offset, 4, memory);
463-
}
464431
AtomicFence* makeAtomicFence(MemoryOrder order) {
465432
auto* ret = wasm.allocator.alloc<AtomicFence>();
466433
ret->order = order;
@@ -488,7 +455,6 @@ class Builder {
488455
}
489456
Store* makeAtomicStore(unsigned bytes,
490457
Address offset,
491-
Address align,
492458
Expression* ptr,
493459
Expression* value,
494460
Type type,
@@ -497,24 +463,13 @@ class Builder {
497463
assert(order != MemoryOrder::Unordered &&
498464
"Atomic stores can't be unordered");
499465

500-
Store* store = makeStore(bytes, offset, align, ptr, value, type, memory);
466+
Store* store = makeStore(bytes, offset, bytes, ptr, value, type, memory);
501467
store->order = order;
502468
return store;
503469
}
504-
Store* makeAtomicStore(unsigned bytes,
505-
Address offset,
506-
Expression* ptr,
507-
Expression* value,
508-
Type type,
509-
Name memory,
510-
MemoryOrder order) {
511-
return makeAtomicStore(
512-
bytes, offset, bytes, ptr, value, type, memory, order);
513-
}
514470
AtomicRMW* makeAtomicRMW(AtomicRMWOp op,
515471
unsigned bytes,
516472
Address offset,
517-
Address align,
518473
Expression* ptr,
519474
Expression* value,
520475
Type type,
@@ -524,7 +479,6 @@ class Builder {
524479
ret->op = op;
525480
ret->bytes = bytes;
526481
ret->offset = offset;
527-
ret->align = align;
528482
ret->ptr = ptr;
529483
ret->value = value;
530484
ret->type = type;
@@ -533,20 +487,8 @@ class Builder {
533487
ret->finalize();
534488
return ret;
535489
}
536-
AtomicRMW* makeAtomicRMW(AtomicRMWOp op,
537-
unsigned bytes,
538-
Address offset,
539-
Expression* ptr,
540-
Expression* value,
541-
Type type,
542-
Name memory,
543-
MemoryOrder order) {
544-
return makeAtomicRMW(
545-
op, bytes, offset, bytes, ptr, value, type, memory, order);
546-
}
547490
AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes,
548491
Address offset,
549-
Address align,
550492
Expression* ptr,
551493
Expression* expected,
552494
Expression* replacement,
@@ -556,7 +498,6 @@ class Builder {
556498
auto* ret = wasm.allocator.alloc<AtomicCmpxchg>();
557499
ret->bytes = bytes;
558500
ret->offset = offset;
559-
ret->align = align;
560501
ret->ptr = ptr;
561502
ret->expected = expected;
562503
ret->replacement = replacement;
@@ -566,24 +507,6 @@ class Builder {
566507
ret->finalize();
567508
return ret;
568509
}
569-
AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes,
570-
Address offset,
571-
Expression* ptr,
572-
Expression* expected,
573-
Expression* replacement,
574-
Type type,
575-
Name memory,
576-
MemoryOrder order) {
577-
return makeAtomicCmpxchg(bytes,
578-
offset,
579-
bytes,
580-
ptr,
581-
expected,
582-
replacement,
583-
type,
584-
memory,
585-
order);
586-
}
587510
SIMDExtract*
588511
makeSIMDExtract(SIMDExtractOp op, Expression* vec, uint8_t index) {
589512
auto* ret = wasm.allocator.alloc<SIMDExtract>();

‎src/wasm-delegations-fields.def‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,6 @@ DELEGATE_FIELD_CHILD(AtomicRMW, ptr)
383383
DELEGATE_FIELD_INT(AtomicRMW, op)
384384
DELEGATE_FIELD_INT(AtomicRMW, bytes)
385385
DELEGATE_FIELD_ADDRESS(AtomicRMW, offset)
386-
DELEGATE_FIELD_ADDRESS(AtomicRMW, align)
387386
DELEGATE_FIELD_INT(AtomicRMW, order)
388387
DELEGATE_FIELD_NAME_KIND(AtomicRMW, memory, ModuleItemKind::Memory)
389388
DELEGATE_FIELD_CASE_END(AtomicRMW)
@@ -394,7 +393,6 @@ DELEGATE_FIELD_CHILD(AtomicCmpxchg, expected)
394393
DELEGATE_FIELD_CHILD(AtomicCmpxchg, ptr)
395394
DELEGATE_FIELD_INT(AtomicCmpxchg, bytes)
396395
DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, offset)
397-
DELEGATE_FIELD_ADDRESS(AtomicCmpxchg, align)
398396
DELEGATE_FIELD_INT(AtomicCmpxchg, order)
399397
DELEGATE_FIELD_NAME_KIND(AtomicCmpxchg, memory, ModuleItemKind::Memory)
400398
DELEGATE_FIELD_CASE_END(AtomicCmpxchg)
@@ -404,7 +402,6 @@ DELEGATE_FIELD_CHILD(AtomicWait, timeout)
404402
DELEGATE_FIELD_CHILD(AtomicWait, expected)
405403
DELEGATE_FIELD_CHILD(AtomicWait, ptr)
406404
DELEGATE_FIELD_ADDRESS(AtomicWait, offset)
407-
DELEGATE_FIELD_ADDRESS(AtomicWait, align)
408405
DELEGATE_FIELD_TYPE(AtomicWait, expectedType)
409406
DELEGATE_FIELD_NAME_KIND(AtomicWait, memory, ModuleItemKind::Memory)
410407
DELEGATE_FIELD_CASE_END(AtomicWait)
@@ -413,7 +410,6 @@ DELEGATE_FIELD_CASE_START(AtomicNotify)
413410
DELEGATE_FIELD_CHILD(AtomicNotify, notifyCount)
414411
DELEGATE_FIELD_CHILD(AtomicNotify, ptr)
415412
DELEGATE_FIELD_ADDRESS(AtomicNotify, offset)
416-
DELEGATE_FIELD_ADDRESS(AtomicNotify, align)
417413
DELEGATE_FIELD_NAME_KIND(AtomicNotify, memory, ModuleItemKind::Memory)
418414
DELEGATE_FIELD_CASE_END(AtomicNotify)
419415

‎src/wasm.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,6 @@ class AtomicRMW : public SpecificExpression<Expression::AtomicRMWId> {
10551055
AtomicRMWOp op;
10561056
uint8_t bytes;
10571057
Address offset;
1058-
Address align;
10591058
Expression* ptr;
10601059
Expression* value;
10611060
Name memory;
@@ -1071,7 +1070,6 @@ class AtomicCmpxchg : public SpecificExpression<Expression::AtomicCmpxchgId> {
10711070

10721071
uint8_t bytes;
10731072
Address offset;
1074-
Address align;
10751073
Expression* ptr;
10761074
Expression* expected;
10771075
Expression* replacement;
@@ -1087,7 +1085,6 @@ class AtomicWait : public SpecificExpression<Expression::AtomicWaitId> {
10871085
AtomicWait(MixedArena& allocator) : AtomicWait() {}
10881086

10891087
Address offset;
1090-
Address align;
10911088
Expression* ptr;
10921089
Expression* expected;
10931090
Expression* timeout;
@@ -1103,7 +1100,6 @@ class AtomicNotify : public SpecificExpression<Expression::AtomicNotifyId> {
11031100
AtomicNotify(MixedArena& allocator) : AtomicNotify() {}
11041101

11051102
Address offset;
1106-
Address align;
11071103
Expression* ptr;
11081104
Expression* notifyCount;
11091105
Name memory;

‎src/wasm/wasm-ir-builder.cpp‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ Result<> validateTypeAnnotation(HeapType type, Expression* child) {
9292
return validateTypeAnnotation(Type(type, Nullable), child);
9393
}
9494

95+
Result<> requireNaturalAtomicAlign(Address align, Address natural) {
96+
if (align != natural) {
97+
return Err{"atomic accesses must have natural alignment"};
98+
}
99+
return Ok{};
100+
}
101+
95102
} // anonymous namespace
96103

97104
Result<Index> IRBuilder::addScratchLocal(Type type) {
@@ -1640,11 +1647,11 @@ Result<> IRBuilder::makeAtomicLoad(unsigned bytes,
16401647
Type type,
16411648
Name mem,
16421649
MemoryOrder order) {
1650+
CHECK_ERR(requireNaturalAtomicAlign(align, bytes));
16431651
Load curr;
16441652
curr.memory = mem;
16451653
CHECK_ERR(visitLoad(&curr));
1646-
push(builder.makeAtomicLoad(
1647-
bytes, offset, align, curr.ptr, type, mem, order));
1654+
push(builder.makeAtomicLoad(bytes, offset, curr.ptr, type, mem, order));
16481655
return Ok{};
16491656
}
16501657

@@ -1654,12 +1661,13 @@ Result<> IRBuilder::makeAtomicStore(unsigned bytes,
16541661
Type type,
16551662
Name mem,
16561663
MemoryOrder order) {
1664+
CHECK_ERR(requireNaturalAtomicAlign(align, bytes));
16571665
Store curr;
16581666
curr.memory = mem;
16591667
curr.valueType = type;
16601668
CHECK_ERR(visitStore(&curr));
16611669
push(builder.makeAtomicStore(
1662-
bytes, offset, align, curr.ptr, curr.value, type, mem, order));
1670+
bytes, offset, curr.ptr, curr.value, type, mem, order));
16631671
return Ok{};
16641672
}
16651673

@@ -1670,12 +1678,13 @@ Result<> IRBuilder::makeAtomicRMW(AtomicRMWOp op,
16701678
Type type,
16711679
Name mem,
16721680
MemoryOrder order) {
1681+
CHECK_ERR(requireNaturalAtomicAlign(align, bytes));
16731682
AtomicRMW curr;
16741683
curr.memory = mem;
16751684
curr.type = type;
16761685
CHECK_ERR(visitAtomicRMW(&curr));
16771686
push(builder.makeAtomicRMW(
1678-
op, bytes, offset, align, curr.ptr, curr.value, type, mem, order));
1687+
op, bytes, offset, curr.ptr, curr.value, type, mem, order));
16791688
return Ok{};
16801689
}
16811690

@@ -1685,12 +1694,12 @@ Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes,
16851694
Type type,
16861695
Name mem,
16871696
MemoryOrder order) {
1697+
CHECK_ERR(requireNaturalAtomicAlign(align, bytes));
16881698
AtomicCmpxchg curr;
16891699
curr.memory = mem;
16901700
CHECK_ERR(ChildPopper{*this}.visitAtomicCmpxchg(&curr, type));
16911701
push(builder.makeAtomicCmpxchg(bytes,
16921702
offset,
1693-
align,
16941703
curr.ptr,
16951704
curr.expected,
16961705
curr.replacement,
@@ -1701,20 +1710,22 @@ Result<> IRBuilder::makeAtomicCmpxchg(unsigned bytes,
17011710
}
17021711

17031712
Result<> IRBuilder::makeAtomicWait(Type type, Address offset, Address align, Name mem) {
1713+
CHECK_ERR(requireNaturalAtomicAlign(align, type == Type::i32 ? 4 : 8));
17041714
AtomicWait curr;
17051715
curr.memory = mem;
17061716
curr.expectedType = type;
17071717
CHECK_ERR(visitAtomicWait(&curr));
17081718
push(builder.makeAtomicWait(
1709-
curr.ptr, curr.expected, curr.timeout, type, offset, align, mem));
1719+
curr.ptr, curr.expected, curr.timeout, type, offset, mem));
17101720
return Ok{};
17111721
}
17121722

17131723
Result<> IRBuilder::makeAtomicNotify(Address offset, Address align, Name mem) {
1724+
CHECK_ERR(requireNaturalAtomicAlign(align, 4));
17141725
AtomicNotify curr;
17151726
curr.memory = mem;
17161727
CHECK_ERR(visitAtomicNotify(&curr));
1717-
push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, align, mem));
1728+
push(builder.makeAtomicNotify(curr.ptr, curr.notifyCount, offset, mem));
17181729
return Ok{};
17191730
}
17201731

0 commit comments

Comments
 (0)