Skip to content

Commit 7855ff4

Browse files
tlivelytlively
authored
Do not create invalid public types in MergeSimilarFunctions (#9126)
When similar functions differ only in the targets of their respective Call expressions, MergeSimilarFunctions can merge the functions and pass the correct call target in as a function reference. If the call targets were not previously referenced, they might previously have had private types. In an open world where `func` is exposed on the boundary, this transformation will make the private types public. Since public types can have stricter validation rules than private types (e.g. public types may not contain exact references when custom descriptors are disallowed), this could previously create invalid public types. Fix the bug by adding a check that the call target has a valid public type before doing the optimization.
1 parent aaffe18 commit 7855ff4

3 files changed

Lines changed: 238 additions & 0 deletions

File tree

‎src/passes/MergeSimilarFunctions.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
#include "ir/manipulation.h"
8080
#include "ir/module-utils.h"
8181
#include "ir/names.h"
82+
#include "ir/public-type-validator.h"
8283
#include "ir/utils.h"
8384
#include "opt-utils.h"
8485
#include "pass.h"
@@ -248,6 +249,18 @@ bool MergeSimilarFunctions::areInEquvalentClass(Function* lhs,
248249
if (lhsCallee->type != rhsCallee->type) {
249250
return false;
250251
}
252+
// Parameterizing direct calls to different functions requires creating
253+
// `ref.func` and `call_ref` instructions for them. In an open world, this
254+
// can cause a previously private function signature to become public (for
255+
// instance, if `funcref` is publicly exposed). Do not parameterize the
256+
// call if the callee's signature is not a valid public type (e.g., if it
257+
// contains an exact reference when custom descriptors are disabled).
258+
if (lhsCallee != rhsCallee &&
259+
getPassOptions().worldMode == WorldMode::Open &&
260+
!PublicTypeValidator(module->features)
261+
.isValidPublicType(lhsCallee->type.getHeapType())) {
262+
return false;
263+
}
251264

252265
// Arguments operands should be also equivalent ignoring constants.
253266
for (Index i = 0; i < lhsCast->operands.size(); i++) {

‎src/wasm/wasm-validator.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ void validateExactReferences(Module& module, ValidationInfo& info) {
250250
return;
251251
}
252252

253+
// TODO: This only checks directly exposed root types. To catch all invalid
254+
// public exact references (such as types reachable from exposed types or
255+
// subtypes of exposed `funcref` in open-world mode), we should check all
256+
// public heap types if we can do so without making validation too expensive.
253257
for (auto& [type, _] : ModuleUtils::getExposedPublicHeapTypes(module)) {
254258
for (auto child : type.getTypeChildren()) {
255259
if (child.isExact()) {

‎test/lit/passes/dae-merge-similar-functions-exact.wast‎

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: wasm-opt %s --dae --merge-similar-functions --minimize-rec-groups \
4+
;; RUN: -all --disable-custom-descriptors -S -o - | filecheck %s
5+
6+
;; RUN: wasm-opt %s --dae --merge-similar-functions --minimize-rec-groups \
7+
;; RUN: -all --disable-custom-descriptors --closed-world -S -o - \
8+
;; RUN: | filecheck %s --check-prefix=CLOSD
9+
10+
;; Regression test for a bug where MergeSimilarFunctions parameterized direct
11+
;; calls to unreferenced functions ($callee1 and $callee2) whose private
12+
;; signature ($priv) contained an exact reference, making $priv public when
13+
;; custom descriptors were disabled and causing it to collide with the existing
14+
;; inexact public signature ($pub) in MinimizeRecGroups.
15+
16+
(module
17+
;; CHECK: (type $0 (func))
18+
;; CLOSD: (type $0 (func))
19+
(type $0 (func))
20+
;; CHECK: (type $pub (func (result (ref $0))))
21+
;; CLOSD: (type $pub (func (result (ref $0))))
22+
(type $pub (func (result (ref $0))))
23+
24+
;; Exporting a funcref table makes all referenced (non-private) function
25+
;; signatures public in open-world mode.
26+
;; CHECK: (rec
27+
;; CHECK-NEXT: (type $2 (struct))
28+
29+
;; CHECK: (type $3 (func (result (ref (exact $0)))))
30+
31+
;; CHECK: (table $t 1 1 funcref)
32+
;; CLOSD: (rec
33+
;; CLOSD-NEXT: (type $2 (struct))
34+
35+
;; CLOSD: (type $3 (func (result (ref (exact $0)))))
36+
37+
;; CLOSD: (type $4 (func (param (ref $3)) (result (ref $0))))
38+
39+
;; CLOSD: (table $t 1 1 funcref)
40+
(table $t (export "t") 1 1 funcref)
41+
;; Referencing $pub-fn in an element segment makes $pub a public signature.
42+
;; CHECK: (elem $e (i32.const 0) $pub-fn)
43+
;; CLOSD: (elem $e (i32.const 0) $pub-fn)
44+
(elem $e (i32.const 0) $pub-fn)
45+
46+
;; CHECK: (elem declare func $target)
47+
48+
;; CHECK: (export "t" (table $t))
49+
50+
;; CHECK: (export "caller1" (func $caller1))
51+
52+
;; CHECK: (export "caller2" (func $caller2))
53+
54+
;; CHECK: (func $target (type $0)
55+
;; CHECK-NEXT: )
56+
;; CLOSD: (elem declare func $callee1 $callee2 $target)
57+
58+
;; CLOSD: (export "t" (table $t))
59+
60+
;; CLOSD: (export "caller1" (func $caller1))
61+
62+
;; CLOSD: (export "caller2" (func $caller2))
63+
64+
;; CLOSD: (func $target (type $0)
65+
;; CLOSD-NEXT: )
66+
(func $target (type $0))
67+
68+
;; CHECK: (func $pub-fn (type $pub) (result (ref $0))
69+
;; CHECK-NEXT: (ref.func $target)
70+
;; CHECK-NEXT: )
71+
;; CLOSD: (func $pub-fn (type $pub) (result (ref $0))
72+
;; CLOSD-NEXT: (ref.func $target)
73+
;; CLOSD-NEXT: )
74+
(func $pub-fn (type $pub) (result (ref $0))
75+
;; Referenced in $e, so its signature $pub is public from the start.
76+
(ref.func $target)
77+
)
78+
79+
;; CHECK: (func $callee1 (type $3) (result (ref (exact $0)))
80+
;; CHECK-NEXT: (ref.func $target)
81+
;; CHECK-NEXT: )
82+
;; CLOSD: (func $callee1 (type $3) (result (ref (exact $0)))
83+
;; CLOSD-NEXT: (ref.func $target)
84+
;; CLOSD-NEXT: )
85+
(func $callee1 (result (ref $0))
86+
;; Unreferenced helper. DAE refines its return type to `(ref (exact $0))`.
87+
;; Without the fix in MergeSimilarFunctions, MergeSimilarFunctions would
88+
;; take `(ref.func $callee1)` in a thunk for $caller1 and call it via
89+
;; `call_ref` in a shared helper, promoting its refined signature to a
90+
;; public type that collides with $pub in MinimizeRecGroups once
91+
;; --disable-custom-descriptors erases `exact`.
92+
(ref.func $target)
93+
)
94+
95+
;; CHECK: (func $callee2 (type $3) (result (ref (exact $0)))
96+
;; CHECK-NEXT: (ref.func $target)
97+
;; CHECK-NEXT: )
98+
;; CLOSD: (func $callee2 (type $3) (result (ref (exact $0)))
99+
;; CLOSD-NEXT: (ref.func $target)
100+
;; CLOSD-NEXT: )
101+
(func $callee2 (result (ref $0))
102+
;; Second unreferenced helper whose return type is refined to
103+
;; `(ref (exact $0))` by DAE.
104+
(ref.func $target)
105+
)
106+
107+
;; CHECK: (func $caller1 (type $pub) (result (ref $0))
108+
;; CHECK-NEXT: (drop
109+
;; CHECK-NEXT: (ref.as_non_null
110+
;; CHECK-NEXT: (call $callee1)
111+
;; CHECK-NEXT: )
112+
;; CHECK-NEXT: )
113+
;; CHECK-NEXT: (nop)
114+
;; CHECK-NEXT: (nop)
115+
;; CHECK-NEXT: (nop)
116+
;; CHECK-NEXT: (nop)
117+
;; CHECK-NEXT: (nop)
118+
;; CHECK-NEXT: (nop)
119+
;; CHECK-NEXT: (nop)
120+
;; CHECK-NEXT: (nop)
121+
;; CHECK-NEXT: (nop)
122+
;; CHECK-NEXT: (nop)
123+
;; CHECK-NEXT: (nop)
124+
;; CHECK-NEXT: (nop)
125+
;; CHECK-NEXT: (nop)
126+
;; CHECK-NEXT: (nop)
127+
;; CHECK-NEXT: (nop)
128+
;; CHECK-NEXT: (nop)
129+
;; CHECK-NEXT: (call $callee1)
130+
;; CHECK-NEXT: )
131+
;; CLOSD: (func $caller1 (type $pub) (result (ref $0))
132+
;; CLOSD-NEXT: (return_call $byn$mgfn-shared$caller1
133+
;; CLOSD-NEXT: (ref.func $callee1)
134+
;; CLOSD-NEXT: )
135+
;; CLOSD-NEXT: )
136+
(func $caller1 (export "caller1") (result (ref $0))
137+
;; Wrapping the first call in `ref.as_non_null` ensures DAE does not treat
138+
;; the call as dropped and instead refines $callee1's return type.
139+
;; Without the fix, MergeSimilarFunctions would merge $caller1 and $caller2
140+
;; into a shared function taking a parameter of $callee1's refined signature
141+
;; and replace $caller1's body with a thunk passing `(ref.func $callee1)`.
142+
;; With the fix, MergeSimilarFunctions sees that the refined signature is
143+
;; not a valid public type without custom descriptors and leaves $caller1
144+
;; unmerged. The nops are so MergeSimilarFunctions would think this function
145+
;; and $caller2 are otherwise profitable to merge.
146+
;;
147+
;; We can still optimize with --closed-world.
148+
(drop (ref.as_non_null (call $callee1)))
149+
(nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
150+
(nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
151+
(call $callee1)
152+
)
153+
154+
;; CHECK: (func $caller2 (type $pub) (result (ref $0))
155+
;; CHECK-NEXT: (drop
156+
;; CHECK-NEXT: (ref.as_non_null
157+
;; CHECK-NEXT: (call $callee2)
158+
;; CHECK-NEXT: )
159+
;; CHECK-NEXT: )
160+
;; CHECK-NEXT: (nop)
161+
;; CHECK-NEXT: (nop)
162+
;; CHECK-NEXT: (nop)
163+
;; CHECK-NEXT: (nop)
164+
;; CHECK-NEXT: (nop)
165+
;; CHECK-NEXT: (nop)
166+
;; CHECK-NEXT: (nop)
167+
;; CHECK-NEXT: (nop)
168+
;; CHECK-NEXT: (nop)
169+
;; CHECK-NEXT: (nop)
170+
;; CHECK-NEXT: (nop)
171+
;; CHECK-NEXT: (nop)
172+
;; CHECK-NEXT: (nop)
173+
;; CHECK-NEXT: (nop)
174+
;; CHECK-NEXT: (nop)
175+
;; CHECK-NEXT: (nop)
176+
;; CHECK-NEXT: (call $callee2)
177+
;; CHECK-NEXT: )
178+
;; CLOSD: (func $caller2 (type $pub) (result (ref $0))
179+
;; CLOSD-NEXT: (return_call $byn$mgfn-shared$caller1
180+
;; CLOSD-NEXT: (ref.func $callee2)
181+
;; CLOSD-NEXT: )
182+
;; CLOSD-NEXT: )
183+
(func $caller2 (export "caller2") (result (ref $0))
184+
;; Identical to $caller1 except for calling $callee2 instead of $callee1.
185+
;; Without the fix, MergeSimilarFunctions would replace $caller2's body
186+
;; with a thunk passing `(ref.func $callee2)`. With the fix, $caller2 is
187+
;; left unmerged so the refined signature remains private.
188+
(drop (ref.as_non_null (call $callee2)))
189+
(nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
190+
(nop) (nop) (nop) (nop) (nop) (nop) (nop) (nop)
191+
(call $callee2)
192+
)
193+
)
194+
;; CLOSD: (func $byn$mgfn-shared$caller1 (type $4) (param $0 (ref $3)) (result (ref $0))
195+
;; CLOSD-NEXT: (drop
196+
;; CLOSD-NEXT: (ref.as_non_null
197+
;; CLOSD-NEXT: (call_ref $3
198+
;; CLOSD-NEXT: (local.get $0)
199+
;; CLOSD-NEXT: )
200+
;; CLOSD-NEXT: )
201+
;; CLOSD-NEXT: )
202+
;; CLOSD-NEXT: (nop)
203+
;; CLOSD-NEXT: (nop)
204+
;; CLOSD-NEXT: (nop)
205+
;; CLOSD-NEXT: (nop)
206+
;; CLOSD-NEXT: (nop)
207+
;; CLOSD-NEXT: (nop)
208+
;; CLOSD-NEXT: (nop)
209+
;; CLOSD-NEXT: (nop)
210+
;; CLOSD-NEXT: (nop)
211+
;; CLOSD-NEXT: (nop)
212+
;; CLOSD-NEXT: (nop)
213+
;; CLOSD-NEXT: (nop)
214+
;; CLOSD-NEXT: (nop)
215+
;; CLOSD-NEXT: (nop)
216+
;; CLOSD-NEXT: (nop)
217+
;; CLOSD-NEXT: (nop)
218+
;; CLOSD-NEXT: (call_ref $3
219+
;; CLOSD-NEXT: (local.get $0)
220+
;; CLOSD-NEXT: )
221+
;; CLOSD-NEXT: )

0 commit comments

Comments
 (0)