Skip to content

Commit 88aa87b

Browse files
sumleosumleo
authored
Fix i32 sign extension in MemoryPacking pass (#8368)
## Summary - Fix sign extension of `geti32()` results when assigned to `uint64_t`/`size_t` in MemoryPacking pass - In `visitMemoryInit` (line 465-466): `offsetVal` and `sizeVal` sign-extend for values >= `0x80000000`, causing valid `memory.init` instructions to be incorrectly replaced with traps - In `createSplitSegments` (line 713-714): same pattern corrupts start/end range calculation for segment splitting The fix casts through `uint32_t` before widening, consistent with the pattern already used at lines 458 and 461 in the same function. ## Test plan - [x] Build succeeds with no warnings - [x] All 309 GTest unit tests pass - [x] All wasm-opt tests pass (MemoryPacking pass is exercised there)
1 parent 4b946ba commit 88aa87b

2 files changed

Lines changed: 145 additions & 6 deletions

File tree

‎src/passes/MemoryPacking.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,15 @@ void MemoryPacking::optimizeSegmentOps(Module* module) {
455455
bool mustTrap = false;
456456
auto* offset = curr->offset->dynCast<Const>();
457457
auto* size = curr->size->dynCast<Const>();
458-
if (offset && uint32_t(offset->value.geti32()) > maxRuntimeSize) {
458+
if (offset && offset->value.getUnsigned() > maxRuntimeSize) {
459459
mustTrap = true;
460460
}
461-
if (size && uint32_t(size->value.geti32()) > maxRuntimeSize) {
461+
if (size && size->value.getUnsigned() > maxRuntimeSize) {
462462
mustTrap = true;
463463
}
464464
if (offset && size) {
465-
uint64_t offsetVal(offset->value.geti32());
466-
uint64_t sizeVal(size->value.geti32());
465+
auto offsetVal = offset->value.getUnsigned();
466+
auto sizeVal = size->value.getUnsigned();
467467
if (offsetVal + sizeVal > maxRuntimeSize) {
468468
mustTrap = true;
469469
} else if (offsetVal == 0 && sizeVal == 0) {
@@ -710,8 +710,8 @@ void MemoryPacking::createReplacements(Module* module,
710710
}
711711

712712
// Nonconstant offsets or sizes will have inhibited splitting
713-
size_t start = init->offset->cast<Const>()->value.geti32();
714-
size_t end = start + init->size->cast<Const>()->value.geti32();
713+
size_t start = init->offset->cast<Const>()->value.getUnsigned();
714+
size_t end = start + init->size->cast<Const>()->value.getUnsigned();
715715

716716
// Index in `segments` of the segment used in emitted memory.init
717717
// instructions

‎test/lit/passes/memory-packing_memory64-high-addr.wast‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
;; RUN: foreach %s %t wasm-opt --memory-packing -all --zero-filled-memory -S -o - | filecheck %s
3+
;; RUN: foreach %s %t wasm-opt --memory-packing -all --zero-filled-memory -tnh -S -o - | filecheck %s --check-prefix=TNH__
4+
5+
;; Test that memory-packing correctly handles memory64 data segment offsets
6+
;; with the high bit set (>= 0x80000000). These must be treated as unsigned
7+
;; values, not sign-extended to 64 bits.
8+
9+
(module
10+
;; An active data segment at offset 0x80000000 on a memory64 with only 2
11+
;; pages. The offset far exceeds the memory size, so this should trap and the
12+
;; segment must be preserved.
13+
;; CHECK: (memory $memory i64 2 2)
14+
;; TNH__: (memory $memory i64 2 2)
15+
(memory $memory i64 2 2)
16+
;; CHECK: (data $data (i64.const 2147483648) "\00")
17+
(data $data (i64.const 0x80000000) "\00")
18+
)
19+
20+
(module
21+
;; Similar to above but with a multi-byte segment that should be shortened to
22+
;; a single byte at the highest address to preserve the trap.
23+
;; CHECK: (memory $memory i64 2 2)
24+
;; TNH__: (memory $memory i64 2 2)
25+
(memory $memory i64 2 2)
26+
;; CHECK: (data $data (i64.const 2147483650) "\00")
27+
(data $data (i64.const 0x80000000) "\00\00\00")
28+
)
29+
30+
(module
31+
;; Offset 0xFFFFFFFF (4GB - 1) on memory64. This is a valid i64 offset that
32+
;; must not be confused with -1 in i32. The segment should trap.
33+
;; CHECK: (memory $memory i64 2 2)
34+
;; TNH__: (memory $memory i64 2 2)
35+
(memory $memory i64 2 2)
36+
;; CHECK: (data $data (i64.const 4294967295) "\00")
37+
(data $data (i64.const 0xFFFFFFFF) "\00")
38+
)
39+
40+
(module
41+
;; A memory64 with enough pages that offset 0x80000000 is in bounds (0x80000000
42+
;; = 2GB, which is 32768 pages). The segment should be removable since it's all
43+
;; zeroes and in bounds.
44+
;; CHECK: (memory $memory i64 32769 32769)
45+
;; TNH__: (memory $memory i64 32769 32769)
46+
(memory $memory i64 32769 32769)
47+
(data $data (i64.const 0x80000000) "\00")
48+
)
49+
50+
(module
51+
;; A passive segment used with memory.init. The memory.init offset into the
52+
;; segment is 0x80000000 (as i32), which exceeds the segment data size and
53+
;; should cause a trap. Ensure the i32 offset is treated as unsigned (2147483648)
54+
;; not sign-extended to a negative value.
55+
;; CHECK: (type $0 (func))
56+
57+
;; CHECK: (memory $memory i64 2 2)
58+
;; TNH__: (type $0 (func))
59+
60+
;; TNH__: (memory $memory i64 2 2)
61+
(memory $memory i64 2 2)
62+
(data $data "hello")
63+
;; CHECK: (func $test (type $0)
64+
;; CHECK-NEXT: (drop
65+
;; CHECK-NEXT: (i64.const 0)
66+
;; CHECK-NEXT: )
67+
;; CHECK-NEXT: (drop
68+
;; CHECK-NEXT: (i32.const -2147483648)
69+
;; CHECK-NEXT: )
70+
;; CHECK-NEXT: (drop
71+
;; CHECK-NEXT: (i32.const 1)
72+
;; CHECK-NEXT: )
73+
;; CHECK-NEXT: (unreachable)
74+
;; CHECK-NEXT: )
75+
;; TNH__: (func $test (type $0)
76+
;; TNH__-NEXT: (drop
77+
;; TNH__-NEXT: (i64.const 0)
78+
;; TNH__-NEXT: )
79+
;; TNH__-NEXT: (drop
80+
;; TNH__-NEXT: (i32.const -2147483648)
81+
;; TNH__-NEXT: )
82+
;; TNH__-NEXT: (drop
83+
;; TNH__-NEXT: (i32.const 1)
84+
;; TNH__-NEXT: )
85+
;; TNH__-NEXT: (unreachable)
86+
;; TNH__-NEXT: )
87+
(func $test
88+
(memory.init $data
89+
(i64.const 0)
90+
(i32.const 0x80000000)
91+
(i32.const 1)
92+
)
93+
)
94+
)
95+
96+
(module
97+
;; A passive segment where memory.init size has the high bit set.
98+
;; Size 0x80000000 (2147483648) exceeds the segment data size (5) and should
99+
;; trap. Ensure the i32 size is treated as unsigned.
100+
;; CHECK: (type $0 (func))
101+
102+
;; CHECK: (memory $memory i64 2 2)
103+
;; TNH__: (type $0 (func))
104+
105+
;; TNH__: (memory $memory i64 2 2)
106+
(memory $memory i64 2 2)
107+
(data $data "hello")
108+
;; CHECK: (func $test (type $0)
109+
;; CHECK-NEXT: (drop
110+
;; CHECK-NEXT: (i64.const 0)
111+
;; CHECK-NEXT: )
112+
;; CHECK-NEXT: (drop
113+
;; CHECK-NEXT: (i32.const 0)
114+
;; CHECK-NEXT: )
115+
;; CHECK-NEXT: (drop
116+
;; CHECK-NEXT: (i32.const -2147483648)
117+
;; CHECK-NEXT: )
118+
;; CHECK-NEXT: (unreachable)
119+
;; CHECK-NEXT: )
120+
;; TNH__: (func $test (type $0)
121+
;; TNH__-NEXT: (drop
122+
;; TNH__-NEXT: (i64.const 0)
123+
;; TNH__-NEXT: )
124+
;; TNH__-NEXT: (drop
125+
;; TNH__-NEXT: (i32.const 0)
126+
;; TNH__-NEXT: )
127+
;; TNH__-NEXT: (drop
128+
;; TNH__-NEXT: (i32.const -2147483648)
129+
;; TNH__-NEXT: )
130+
;; TNH__-NEXT: (unreachable)
131+
;; TNH__-NEXT: )
132+
(func $test
133+
(memory.init $data
134+
(i64.const 0)
135+
(i32.const 0)
136+
(i32.const 0x80000000)
137+
)
138+
)
139+
)

0 commit comments

Comments
 (0)