Skip to content

Commit 53f568f

Browse files
stevenfontanellastevenfontanella
committed
Fix alignment validation
1 parent 1dd0e24 commit 53f568f

File tree

8 files changed

+67
-36
lines changed

8 files changed

+67
-36
lines changed

‎src/binaryen-c.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,6 +1365,7 @@ BinaryenExpressionRef BinaryenAtomicLoad(BinaryenModuleRef module,
13651365
Builder(*(Module*)module)
13661366
.makeAtomicLoad(bytes,
13671367
offset,
1368+
bytes,
13681369
(Expression*)ptr,
13691370
Type(type),
13701371
getMemoryName(module, memoryName),
@@ -1382,6 +1383,7 @@ BinaryenExpressionRef BinaryenAtomicStore(BinaryenModuleRef module,
13821383
Builder(*(Module*)module)
13831384
.makeAtomicStore(bytes,
13841385
offset,
1386+
bytes,
13851387
(Expression*)ptr,
13861388
(Expression*)value,
13871389
Type(type),

‎src/parser/contexts.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,8 +2270,9 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
22702270
auto m = getMemory(pos, mem);
22712271
CHECK_ERR(m);
22722272
if (isAtomic) {
2273-
return withLoc(
2274-
pos, irBuilder.makeAtomicLoad(bytes, memarg.offset, type, *m, order));
2273+
return withLoc(pos,
2274+
irBuilder.makeAtomicLoad(
2275+
bytes, memarg.offset, memarg.align, type, *m, order));
22752276
}
22762277
return withLoc(pos,
22772278
irBuilder.makeLoad(
@@ -2289,8 +2290,9 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
22892290
auto m = getMemory(pos, mem);
22902291
CHECK_ERR(m);
22912292
if (isAtomic) {
2292-
return withLoc(
2293-
pos, irBuilder.makeAtomicStore(bytes, memarg.offset, type, *m, order));
2293+
return withLoc(pos,
2294+
irBuilder.makeAtomicStore(
2295+
bytes, memarg.offset, memarg.align, type, *m, order));
22942296
}
22952297
return withLoc(
22962298
pos, irBuilder.makeStore(bytes, memarg.offset, memarg.align, type, *m));

‎src/tools/wasm-split/instrumenter.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,9 @@ void Instrumenter::instrumentFuncs() {
146146
: secondaryMemory;
147147
ModuleUtils::iterDefinedFunctions(*wasm, [&](Function* func) {
148148
func->body = builder.makeSequence(
149-
builder.makeAtomicStore(1,
149+
builder.makeAtomicStore(4,
150150
funcIdx,
151+
/*align=*/4,
151152
builder.makeConstPtr(0, Type::i32),
152153
builder.makeConst(uint32_t(1)),
153154
Type::i32,
@@ -288,8 +289,9 @@ void Instrumenter::addProfileExport(size_t numFuncs) {
288289
getAddr(),
289290
builder.makeBinary(
290291
MulInt32, getFuncIdx(), builder.makeConst(uint32_t(4)))),
291-
builder.makeAtomicLoad(1,
292+
builder.makeAtomicLoad(4,
292293
0,
294+
/*align=*/4,
293295
getFuncIdx(),
294296
Type::i32,
295297
loadMemoryName,

‎src/wasm-builder.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,16 @@ class Builder {
386386
}
387387
Load* makeAtomicLoad(unsigned bytes,
388388
Address offset,
389+
unsigned align,
389390
Expression* ptr,
390391
Type type,
391392
Name memory,
392393
MemoryOrder order) {
393394
assert(order != MemoryOrder::Unordered &&
394395
"Atomic loads can't be unordered");
395396

396-
Load* load = makeLoad(bytes, false, offset, bytes, ptr, type, memory);
397+
// this part is wrong
398+
Load* load = makeLoad(bytes, false, offset, align, ptr, type, memory);
397399
load->order = order;
398400
return load;
399401
}
@@ -448,6 +450,7 @@ class Builder {
448450
}
449451
Store* makeAtomicStore(unsigned bytes,
450452
Address offset,
453+
unsigned align,
451454
Expression* ptr,
452455
Expression* value,
453456
Type type,
@@ -456,7 +459,7 @@ class Builder {
456459
assert(order != MemoryOrder::Unordered &&
457460
"Atomic stores can't be unordered");
458461

459-
Store* store = makeStore(bytes, offset, bytes, ptr, value, type, memory);
462+
Store* store = makeStore(bytes, offset, align, ptr, value, type, memory);
460463
store->order = order;
461464
return store;
462465
}

‎src/wasm-ir-builder.h‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,18 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
155155
Name mem);
156156
Result<> makeStore(
157157
unsigned bytes, Address offset, unsigned align, Type type, Name mem);
158-
Result<> makeAtomicLoad(
159-
unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order);
160-
Result<> makeAtomicStore(
161-
unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order);
158+
Result<> makeAtomicLoad(unsigned bytes,
159+
Address offset,
160+
unsigned align,
161+
Type type,
162+
Name mem,
163+
MemoryOrder order);
164+
Result<> makeAtomicStore(unsigned bytes,
165+
Address offset,
166+
unsigned align,
167+
Type type,
168+
Name mem,
169+
MemoryOrder order);
162170
Result<> makeAtomicRMW(AtomicRMWOp op,
163171
unsigned bytes,
164172
Address offset,

‎src/wasm/wasm-binary.cpp‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3641,66 +3641,73 @@ Result<> WasmBinaryReader::readInst() {
36413641
case BinaryConsts::I32AtomicLoad8U: {
36423642
// TODO: pass align through for validation.
36433643
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3644-
return builder.makeAtomicLoad(1, offset, Type::i32, mem, memoryOrder);
3644+
return builder.makeAtomicLoad(
3645+
1, offset, align, Type::i32, mem, memoryOrder);
36453646
}
36463647
case BinaryConsts::I32AtomicLoad16U: {
36473648
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3648-
return builder.makeAtomicLoad(2, offset, Type::i32, mem, memoryOrder);
3649+
return builder.makeAtomicLoad(
3650+
2, offset, align, Type::i32, mem, memoryOrder);
36493651
}
36503652
case BinaryConsts::I32AtomicLoad: {
36513653
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3652-
return builder.makeAtomicLoad(4, offset, Type::i32, mem, memoryOrder);
3654+
return builder.makeAtomicLoad(
3655+
4, offset, align, Type::i32, mem, memoryOrder);
36533656
}
36543657
case BinaryConsts::I64AtomicLoad8U: {
36553658
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3656-
return builder.makeAtomicLoad(1, offset, Type::i64, mem, memoryOrder);
3659+
return builder.makeAtomicLoad(
3660+
1, offset, align, Type::i64, mem, memoryOrder);
36573661
}
36583662
case BinaryConsts::I64AtomicLoad16U: {
36593663
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3660-
return builder.makeAtomicLoad(2, offset, Type::i64, mem, memoryOrder);
3664+
return builder.makeAtomicLoad(
3665+
2, offset, align, Type::i64, mem, memoryOrder);
36613666
}
36623667
case BinaryConsts::I64AtomicLoad32U: {
36633668
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3664-
return builder.makeAtomicLoad(4, offset, Type::i64, mem, memoryOrder);
3669+
return builder.makeAtomicLoad(
3670+
4, offset, align, Type::i64, mem, memoryOrder);
36653671
}
36663672
case BinaryConsts::I64AtomicLoad: {
36673673
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
3668-
return builder.makeAtomicLoad(8, offset, Type::i64, mem, memoryOrder);
3674+
return builder.makeAtomicLoad(
3675+
8, offset, align, Type::i64, mem, memoryOrder);
36693676
}
36703677
case BinaryConsts::I32AtomicStore8: {
36713678
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36723679
return builder.makeAtomicStore(
3673-
1, offset, Type::i32, mem, memoryOrder);
3680+
1, offset, align, Type::i32, mem, memoryOrder);
36743681
}
36753682
case BinaryConsts::I32AtomicStore16: {
36763683
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36773684
return builder.makeAtomicStore(
3678-
2, offset, Type::i32, mem, memoryOrder);
3685+
2, offset, align, Type::i32, mem, memoryOrder);
36793686
}
36803687
case BinaryConsts::I32AtomicStore: {
36813688
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36823689
return builder.makeAtomicStore(
3683-
4, offset, Type::i32, mem, memoryOrder);
3690+
4, offset, align, Type::i32, mem, memoryOrder);
36843691
}
36853692
case BinaryConsts::I64AtomicStore8: {
36863693
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36873694
return builder.makeAtomicStore(
3688-
1, offset, Type::i64, mem, memoryOrder);
3695+
1, offset, align, Type::i64, mem, memoryOrder);
36893696
}
36903697
case BinaryConsts::I64AtomicStore16: {
36913698
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36923699
return builder.makeAtomicStore(
3693-
2, offset, Type::i64, mem, memoryOrder);
3700+
2, offset, align, Type::i64, mem, memoryOrder);
36943701
}
36953702
case BinaryConsts::I64AtomicStore32: {
36963703
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
36973704
return builder.makeAtomicStore(
3698-
4, offset, Type::i64, mem, memoryOrder);
3705+
4, offset, align, Type::i64, mem, memoryOrder);
36993706
}
37003707
case BinaryConsts::I64AtomicStore: {
37013708
auto [mem, align, offset, memoryOrder] = getAtomicMemarg();
37023709
return builder.makeAtomicStore(
3703-
8, offset, Type::i64, mem, memoryOrder);
3710+
8, offset, align, Type::i64, mem, memoryOrder);
37043711
}
37053712

37063713
#define RMW(op) \

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,23 +1484,32 @@ Result<> IRBuilder::makeStore(
14841484
return Ok{};
14851485
}
14861486

1487-
Result<> IRBuilder::makeAtomicLoad(
1488-
unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) {
1487+
Result<> IRBuilder::makeAtomicLoad(unsigned bytes,
1488+
Address offset,
1489+
unsigned align,
1490+
Type type,
1491+
Name mem,
1492+
MemoryOrder order) {
14891493
Load curr;
14901494
curr.memory = mem;
14911495
CHECK_ERR(visitLoad(&curr));
1492-
push(builder.makeAtomicLoad(bytes, offset, curr.ptr, type, mem, order));
1496+
push(
1497+
builder.makeAtomicLoad(bytes, offset, align, curr.ptr, type, mem, order));
14931498
return Ok{};
14941499
}
14951500

1496-
Result<> IRBuilder::makeAtomicStore(
1497-
unsigned bytes, Address offset, Type type, Name mem, MemoryOrder order) {
1501+
Result<> IRBuilder::makeAtomicStore(unsigned bytes,
1502+
Address offset,
1503+
unsigned align,
1504+
Type type,
1505+
Name mem,
1506+
MemoryOrder order) {
14981507
Store curr;
14991508
curr.memory = mem;
15001509
curr.valueType = type;
15011510
CHECK_ERR(visitStore(&curr));
15021511
push(builder.makeAtomicStore(
1503-
bytes, offset, curr.ptr, curr.value, type, mem, order));
1512+
bytes, offset, align, curr.ptr, curr.value, type, mem, order));
15041513
return Ok{};
15051514
}
15061515

‎src/wasm/wasm-validator.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4319,10 +4319,8 @@ void FunctionValidator::validateOffset(Address offset,
43194319
void FunctionValidator::validateAlignment(
43204320
size_t align, Type type, Index bytes, bool isAtomic, Expression* curr) {
43214321
if (isAtomic) {
4322-
shouldBeEqual(align,
4323-
(size_t)bytes,
4324-
curr,
4325-
"atomic accesses must have natural alignment");
4322+
shouldBeEqual(
4323+
align, (size_t)bytes, curr, "atomic alignment must be natural");
43264324
return;
43274325
}
43284326
switch (align) {

0 commit comments

Comments
 (0)