Skip to content

Commit 662835a

Browse files
kripkenkripken
authored
RemoveUnusedModuleElements: Do not remove unused-but-trapping segments (#6242)
An out of bounds active segment traps during startup, which is an effect we must preserve. To avoid a regression here, ignore this in TNH mode (where the user assures us nothing will trap), and also check if a segment will trivially be in bounds and not trap (if so, it can be removed). Fixes the remove-unused-module-elements part of #6230 The small change to an existing testcase made a segment there be in bounds, to avoid this affecting it. Tests for this are in a new file.
1 parent 0375d95 commit 662835a

File tree

3 files changed

+297
-7
lines changed

3 files changed

+297
-7
lines changed

‎src/passes/RemoveUnusedModuleElements.cpp‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include "ir/subtypes.h"
4646
#include "ir/utils.h"
4747
#include "pass.h"
48+
#include "support/stdckdint.h"
4849
#include "wasm-builder.h"
4950
#include "wasm.h"
5051

@@ -635,17 +636,57 @@ struct RemoveUnusedModuleElements : public Pass {
635636
// Active segments that write to imported tables and memories are roots
636637
// because those writes are externally observable even if the module does
637638
// not otherwise use the tables or memories.
639+
//
640+
// Likewise, if traps are possible during startup then just trapping is an
641+
// effect (which can happen if the offset is out of bounds).
642+
auto maybeRootSegment = [&](ModuleElementKind kind,
643+
Name segmentName,
644+
Index segmentSize,
645+
Expression* offset,
646+
Importable* parent,
647+
Index parentSize) {
648+
auto writesToVisible = parent->imported() && segmentSize;
649+
auto mayTrap = false;
650+
if (!getPassOptions().trapsNeverHappen) {
651+
// Check if this might trap. If it is obviously in bounds then it
652+
// cannot.
653+
auto* c = offset->dynCast<Const>();
654+
// Check for overflow in the largest possible space of addresses.
655+
using AddressType = Address::address64_t;
656+
AddressType maxWritten;
657+
// If there is no integer, or if there is and the addition overflows, or
658+
// if the addition leads to a too-large value, then we may trap.
659+
mayTrap = !c ||
660+
std::ckd_add(&maxWritten,
661+
(AddressType)segmentSize,
662+
(AddressType)c->value.getInteger()) ||
663+
maxWritten > parentSize;
664+
}
665+
if (writesToVisible || mayTrap) {
666+
roots.emplace_back(kind, segmentName);
667+
}
668+
};
638669
ModuleUtils::iterActiveDataSegments(*module, [&](DataSegment* segment) {
639-
if (module->getMemory(segment->memory)->imported() &&
640-
!segment->data.empty()) {
641-
roots.emplace_back(ModuleElementKind::DataSegment, segment->name);
670+
if (segment->memory.is()) {
671+
auto* memory = module->getMemory(segment->memory);
672+
maybeRootSegment(ModuleElementKind::DataSegment,
673+
segment->name,
674+
segment->data.size(),
675+
segment->offset,
676+
memory,
677+
memory->initial * Memory::kPageSize);
642678
}
643679
});
644680
ModuleUtils::iterActiveElementSegments(
645681
*module, [&](ElementSegment* segment) {
646-
if (module->getTable(segment->table)->imported() &&
647-
!segment->data.empty()) {
648-
roots.emplace_back(ModuleElementKind::ElementSegment, segment->name);
682+
if (segment->table.is()) {
683+
auto* table = module->getTable(segment->table);
684+
maybeRootSegment(ModuleElementKind::ElementSegment,
685+
segment->name,
686+
segment->data.size(),
687+
segment->offset,
688+
table,
689+
table->initial * Table::kPageSize);
649690
}
650691
});
651692

‎test/lit/passes/remove-unused-module-elements_all-features.wast‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@
170170
(module ;; remove all tables and the memory
171171
(import "env" "memory" (memory $0 256))
172172
(import "env" "table" (table 0 funcref))
173-
(import "env" "table2" (table $1 1 2 funcref))
173+
(import "env" "table2" (table $1 2 2 funcref))
174174
(elem (table $1) (offset (i32.const 0)) func)
175175
(elem (table $1) (offset (i32.const 1)) func)
176176
)

‎test/lit/passes/remove-unused-module-elements_tnh.wast‎

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up.
3+
4+
;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements -all -S -o - | filecheck %s
5+
;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements -tnh -all -S -o - | filecheck %s --check-prefix=T_N_H
6+
7+
;; The segments here will trap during startup as they are out of bounds. We
8+
;; can only remove such segments if we assume TrapsNeverHappen.
9+
;;
10+
;; The passive segments, however, can be removed: they do nothing during
11+
;; startup, and have no uses.
12+
(module
13+
;; CHECK: (memory $0 16 17 shared)
14+
(memory $0 16 17 shared)
15+
16+
;; CHECK: (data $0 (i32.const -1) "")
17+
(data $0 (i32.const -1) "")
18+
19+
(data $1 "")
20+
21+
;; CHECK: (table $0 1 1 funcref)
22+
(table $0 1 1 funcref)
23+
24+
;; CHECK: (elem $0 (i32.const -1))
25+
(elem $0 (i32.const -1))
26+
27+
(elem $1 func)
28+
)
29+
30+
;; Some segments can be removed: any segment that writes to address 131072 or
31+
;; higher will trap, and must be kept (unless TNH). Only the $bad segment
32+
;; should remain for that reason, however, it keeps the memory alive which
33+
;; keeps the $ok* segments alive too.
34+
(module
35+
;; CHECK: (memory $0 2 2)
36+
(memory $0 2 2)
37+
38+
;; CHECK: (data $ok1 (i32.const 0) "a")
39+
(data $ok1 (i32.const 0) "a")
40+
;; CHECK: (data $ok2 (i32.const 1000) "a")
41+
(data $ok2 (i32.const 1000) "a")
42+
;; CHECK: (data $ok3 (i32.const 131071) "a")
43+
(data $ok3 (i32.const 131071) "a")
44+
;; CHECK: (data $bad (i32.const 131071) "ab")
45+
(data $bad (i32.const 131071) "ab")
46+
)
47+
48+
;; The following modules have variations on the bad segment.
49+
(module
50+
;; CHECK: (memory $0 2 2)
51+
(memory $0 2 2)
52+
53+
;; CHECK: (data $ok1 (i32.const 0) "a")
54+
(data $ok1 (i32.const 0) "a")
55+
;; CHECK: (data $ok2 (i32.const 1000) "a")
56+
(data $ok2 (i32.const 1000) "a")
57+
;; CHECK: (data $ok3 (i32.const 131071) "a")
58+
(data $ok3 (i32.const 131071) "a")
59+
;; CHECK: (data $bad (i32.const 131072) "a")
60+
(data $bad (i32.const 131072) "a")
61+
)
62+
63+
(module
64+
;; CHECK: (memory $0 2 2)
65+
(memory $0 2 2)
66+
67+
;; CHECK: (data $ok1 (i32.const 0) "a")
68+
(data $ok1 (i32.const 0) "a")
69+
;; CHECK: (data $ok2 (i32.const 1000) "a")
70+
(data $ok2 (i32.const 1000) "a")
71+
;; CHECK: (data $ok3 (i32.const 131071) "a")
72+
(data $ok3 (i32.const 131071) "a")
73+
;; CHECK: (data $bad (i32.const 9999999) "a")
74+
(data $bad (i32.const 9999999) "a")
75+
)
76+
77+
(module
78+
;; CHECK: (memory $0 2 2)
79+
(memory $0 2 2)
80+
81+
;; CHECK: (data $ok1 (i32.const 0) "a")
82+
(data $ok1 (i32.const 0) "a")
83+
;; CHECK: (data $ok2 (i32.const 1000) "a")
84+
(data $ok2 (i32.const 1000) "a")
85+
;; CHECK: (data $ok3 (i32.const 131071) "a")
86+
(data $ok3 (i32.const 131071) "a")
87+
;; CHECK: (data $bad (i32.const -2) "a")
88+
(data $bad (i32.const 4294967294) "a")
89+
)
90+
91+
(module
92+
;; CHECK: (memory $0 2 2)
93+
(memory $0 2 2)
94+
95+
;; CHECK: (data $ok1 (i32.const 0) "a")
96+
(data $ok1 (i32.const 0) "a")
97+
;; CHECK: (data $ok2 (i32.const 1000) "a")
98+
(data $ok2 (i32.const 1000) "a")
99+
;; CHECK: (data $ok3 (i32.const 131071) "a")
100+
(data $ok3 (i32.const 131071) "a")
101+
;; CHECK: (data $bad (i32.const -6) "abcdefghijklmnop_overflow")
102+
(data $bad (i32.const 4294967290) "abcdefghijklmnop_overflow")
103+
)
104+
105+
(module
106+
;; CHECK: (memory $0 2 2)
107+
(memory $0 2 2)
108+
109+
;; CHECK: (data $ok1 (i32.const 0) "a")
110+
(data $ok1 (i32.const 0) "a")
111+
;; CHECK: (data $ok2 (i32.const 1000) "a")
112+
(data $ok2 (i32.const 1000) "a")
113+
;; CHECK: (data $ok3 (i32.const 131071) "a")
114+
(data $ok3 (i32.const 131071) "a")
115+
;; CHECK: (data $bad (i32.const -2) "a")
116+
(data $bad (i32.const -2) "a")
117+
)
118+
119+
;; An imported global is an unknown offset, so it might trap.
120+
(module
121+
;; CHECK: (import "a" "b" (global $imported i32))
122+
(import "a" "b" (global $imported i32))
123+
124+
;; CHECK: (memory $0 2 2)
125+
(memory $0 2 2)
126+
127+
;; CHECK: (data $ok1 (i32.const 0) "a")
128+
(data $ok1 (i32.const 0) "a")
129+
;; CHECK: (data $ok2 (i32.const 1000) "a")
130+
(data $ok2 (i32.const 1000) "a")
131+
;; CHECK: (data $ok3 (i32.const 131071) "a")
132+
(data $ok3 (i32.const 131071) "a")
133+
;; CHECK: (data $bad (global.get $imported) "a")
134+
(data $bad (global.get $imported) "a")
135+
)
136+
137+
;; Finally, a module with no bad segments. We can remove all the contents.
138+
(module
139+
(memory $0 2 2)
140+
141+
(data $ok1 (i32.const 0) "a")
142+
(data $ok2 (i32.const 1000) "a")
143+
(data $ok3 (i32.const 131071) "a")
144+
)
145+
146+
;; Similar testing for element segments. One bad segment keeps it all alive
147+
;; here.
148+
(module
149+
(table 10 10 funcref)
150+
151+
;; CHECK: (type $0 (func))
152+
153+
;; CHECK: (table $0 10 10 funcref)
154+
155+
;; CHECK: (elem $ok1 (i32.const 0) $func)
156+
(elem $ok1 (i32.const 0) $func)
157+
;; CHECK: (elem $ok2 (i32.const 8) $func $func)
158+
(elem $ok2 (i32.const 8) $func $func)
159+
;; CHECK: (elem $ok3 (i32.const 9) $func)
160+
(elem $ok3 (i32.const 9) $func)
161+
;; CHECK: (elem $bad (i32.const 10) $func)
162+
(elem $bad (i32.const 10) $func)
163+
164+
;; CHECK: (func $func (type $0)
165+
;; CHECK-NEXT: (nop)
166+
;; CHECK-NEXT: )
167+
;; T_N_H: (type $0 (func))
168+
169+
;; T_N_H: (func $func (type $0)
170+
;; T_N_H-NEXT: (nop)
171+
;; T_N_H-NEXT: )
172+
(func $func)
173+
)
174+
175+
;; A different bad segment.
176+
(module
177+
(table 10 10 funcref)
178+
179+
;; CHECK: (type $0 (func))
180+
181+
;; CHECK: (table $0 10 10 funcref)
182+
183+
;; CHECK: (elem $ok1 (i32.const 0) $func)
184+
(elem $ok1 (i32.const 0) $func)
185+
;; CHECK: (elem $ok2 (i32.const 8) $func $func)
186+
(elem $ok2 (i32.const 8) $func $func)
187+
;; CHECK: (elem $ok3 (i32.const 9) $func)
188+
(elem $ok3 (i32.const 9) $func)
189+
;; CHECK: (elem $bad (i32.const 9) $func $func)
190+
(elem $bad (i32.const 9) $func $func)
191+
192+
;; CHECK: (func $func (type $0)
193+
;; CHECK-NEXT: (nop)
194+
;; CHECK-NEXT: )
195+
;; T_N_H: (type $0 (func))
196+
197+
;; T_N_H: (func $func (type $0)
198+
;; T_N_H-NEXT: (nop)
199+
;; T_N_H-NEXT: )
200+
(func $func)
201+
)
202+
203+
;; No bad segments: all element segments vanish. TODO: the function could too
204+
(module
205+
(table 10 10 funcref)
206+
207+
(elem $ok1 (i32.const 0) $func)
208+
(elem $ok2 (i32.const 8) $func $func)
209+
(elem $ok3 (i32.const 9) $func)
210+
211+
;; CHECK: (type $0 (func))
212+
213+
;; CHECK: (func $func (type $0)
214+
;; CHECK-NEXT: (nop)
215+
;; CHECK-NEXT: )
216+
;; T_N_H: (type $0 (func))
217+
218+
;; T_N_H: (func $func (type $0)
219+
;; T_N_H-NEXT: (nop)
220+
;; T_N_H-NEXT: )
221+
(func $func)
222+
)
223+
224+
;; Multiple memories. One can be removed, the other remains due to a trapping
225+
;; segment.
226+
(module
227+
;; CHECK: (memory $small 1 1)
228+
(memory $small 1 1)
229+
230+
(memory $big 2 2)
231+
232+
;; CHECK: (data $a (i32.const 100000) "ab")
233+
(data $a (memory $small) (i32.const 100000) "ab") ;; fits in $big; not $small
234+
235+
(data $b (memory $big) (i32.const 100000) "cd")
236+
)
237+
238+
;; Reverse order of memories.
239+
(module
240+
(memory $big 2 2)
241+
242+
;; CHECK: (memory $small 1 1)
243+
(memory $small 1 1)
244+
245+
;; CHECK: (data $a (i32.const 100000) "ab")
246+
(data $a (memory $small) (i32.const 100000) "ab") ;; fits in $big; not $small
247+
248+
(data $b (memory $big) (i32.const 100000) "cd")
249+
)

0 commit comments

Comments
 (0)