Skip to content

Commit c1409f9

Browse files
kripkenradekdoulik
kripken
authored and
radekdoulik
committed
StringLowering: Modify string=>extern also in public types (WebAssembly#6301)
We want to actually remove all stringref appearances, in both public and private types.
1 parent 50c4d7b commit c1409f9

File tree

4 files changed

+104
-36
lines changed

4 files changed

+104
-36
lines changed

‎src/ir/type-updating.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ GlobalTypeRewriter::GlobalTypeRewriter(Module& wasm) : wasm(wasm) {}
3030

3131
void GlobalTypeRewriter::update() { mapTypes(rebuildTypes()); }
3232

33-
GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes() {
33+
GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes(
34+
const std::vector<HeapType>& additionalPrivateTypes) {
3435
// Find the heap types that are not publicly observable. Even in a closed
3536
// world scenario, don't modify public types because we assume that they may
3637
// be reflected on or used for linking. Figure out where each private type
@@ -39,6 +40,10 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes() {
3940
Index i = 0;
4041
auto privateTypes = ModuleUtils::getPrivateHeapTypes(wasm);
4142

43+
for (auto t : additionalPrivateTypes) {
44+
privateTypes.push_back(t);
45+
}
46+
4247
// Topological sort to have supertypes first, but we have to account for the
4348
// fact that we may be replacing the supertypes to get the order correct.
4449
struct SupertypesFirst

‎src/ir/type-updating.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,12 @@ class GlobalTypeRewriter {
426426
// Builds new types after updating their contents using the hooks below and
427427
// returns a map from the old types to the modified types. Used internally in
428428
// update().
429-
TypeMap rebuildTypes();
429+
//
430+
// This only operates on private types (so as not to modify the module's
431+
// external ABI). It takes as a parameter a list of public types to consider
432+
// private, which allows more flexibility.
433+
TypeMap
434+
rebuildTypes(const std::vector<HeapType>& additionalPrivateTypes = {});
430435

431436
private:
432437
TypeBuilder typeBuilder;
@@ -446,10 +451,12 @@ class TypeMapper : public GlobalTypeRewriter {
446451
TypeMapper(Module& wasm, const TypeUpdates& mapping)
447452
: GlobalTypeRewriter(wasm), mapping(mapping) {}
448453

449-
void map() {
454+
// As rebuildTypes, this can take an optional set of additional types to
455+
// consider private (and therefore to modify).
456+
void map(const std::vector<HeapType>& additionalPrivateTypes = {}) {
450457
// Update the internals of types (struct fields, signatures, etc.) to
451458
// refer to the merged types.
452-
auto newMapping = rebuildTypes();
459+
auto newMapping = rebuildTypes(additionalPrivateTypes);
453460

454461
// Compose the user-provided mapping from old types to other old types with
455462
// the new mapping from old types to new types. `newMapping` will become

‎src/passes/StringLowering.cpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,13 +232,27 @@ struct StringLowering : public StringGathering {
232232

233233
void updateTypes(Module* module) {
234234
TypeMapper::TypeUpdates updates;
235+
235236
// There is no difference between strings and views with imported strings:
236237
// they are all just JS strings, so they all turn into externref.
237238
updates[HeapType::string] = HeapType::ext;
238239
updates[HeapType::stringview_wtf8] = HeapType::ext;
239240
updates[HeapType::stringview_wtf16] = HeapType::ext;
240241
updates[HeapType::stringview_iter] = HeapType::ext;
241-
TypeMapper(*module, updates).map();
242+
243+
// We consider all types that use strings as modifiable, which means we
244+
// mark them as non-private. That is, we are doing something TypeMapper
245+
// normally does not, as we are changing the external interface/ABI of the
246+
// module: we are changing that ABI from using strings to externs.
247+
auto publicTypes = ModuleUtils::getPublicHeapTypes(*module);
248+
std::vector<HeapType> stringUsers;
249+
for (auto t : publicTypes) {
250+
if (Type(t, Nullable).getFeatures().hasStrings()) {
251+
stringUsers.push_back(t);
252+
}
253+
}
254+
255+
TypeMapper(*module, updates).map(stringUsers);
242256
}
243257

244258
// Imported string functions.

‎test/lit/passes/string-lowering-instructions.wast‎

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,54 +9,62 @@
99
;; CHECK: (type $1 (func (param externref externref) (result i32)))
1010

1111
;; CHECK: (rec
12-
;; CHECK-NEXT: (type $2 (func (param externref) (result externref)))
12+
;; CHECK-NEXT: (type $2 (func (param externref i32 externref)))
1313

14-
;; CHECK: (type $3 (func (param externref) (result i32)))
14+
;; CHECK: (type $3 (func (result externref)))
1515

16-
;; CHECK: (type $4 (func (param externref externref) (result i32)))
16+
;; CHECK: (type $4 (func (param externref) (result externref)))
1717

18-
;; CHECK: (type $5 (func (param externref (ref $array16)) (result i32)))
18+
;; CHECK: (type $5 (func (param externref) (result i32)))
1919

20-
;; CHECK: (type $6 (func (result externref)))
20+
;; CHECK: (type $6 (func (param externref externref) (result i32)))
2121

22-
;; CHECK: (type $7 (func (param (ref $array16))))
22+
;; CHECK: (type $7 (func (param externref (ref $array16)) (result i32)))
2323

24-
;; CHECK: (type $8 (func (param externref externref externref externref)))
24+
;; CHECK: (type $8 (func (param (ref $array16))))
2525

26-
;; CHECK: (type $9 (func))
26+
;; CHECK: (type $9 (func (param externref externref externref externref)))
2727

28-
;; CHECK: (type $10 (func (param (ref null $array16) i32 i32) (result (ref extern))))
28+
;; CHECK: (type $10 (func))
2929

30-
;; CHECK: (type $11 (func (param i32) (result (ref extern))))
30+
;; CHECK: (type $11 (func (param (ref null $array16) i32 i32) (result (ref extern))))
3131

32-
;; CHECK: (type $12 (func (param externref (ref null $array16) i32) (result i32)))
32+
;; CHECK: (type $12 (func (param i32) (result (ref extern))))
3333

34-
;; CHECK: (type $13 (func (param externref) (result i32)))
34+
;; CHECK: (type $13 (func (param externref (ref null $array16) i32) (result i32)))
3535

36-
;; CHECK: (type $14 (func (param externref i32) (result i32)))
36+
;; CHECK: (type $14 (func (param externref) (result i32)))
3737

38-
;; CHECK: (type $15 (func (param externref i32 i32) (result (ref extern))))
38+
;; CHECK: (type $15 (func (param externref i32) (result i32)))
3939

40-
;; CHECK: (import "colliding" "name" (func $fromCodePoint (type $9)))
40+
;; CHECK: (type $16 (func (param externref i32 i32) (result (ref extern))))
41+
42+
;; CHECK: (import "string.const" "0" (global $string.const_exported (ref extern)))
43+
44+
;; CHECK: (import "colliding" "name" (func $fromCodePoint (type $10)))
4145
(import "colliding" "name" (func $fromCodePoint))
4246

43-
;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $10) (param (ref null $array16) i32 i32) (result (ref extern))))
47+
;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $11) (param (ref null $array16) i32 i32) (result (ref extern))))
4448

45-
;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint_11 (type $11) (param i32) (result (ref extern))))
49+
;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint_13 (type $12) (param i32) (result (ref extern))))
4650

47-
;; CHECK: (import "wasm:js-string" "intoCharCodeArray" (func $intoCharCodeArray (type $12) (param externref (ref null $array16) i32) (result i32)))
51+
;; CHECK: (import "wasm:js-string" "intoCharCodeArray" (func $intoCharCodeArray (type $13) (param externref (ref null $array16) i32) (result i32)))
4852

4953
;; CHECK: (import "wasm:js-string" "equals" (func $equals (type $1) (param externref externref) (result i32)))
5054

5155
;; CHECK: (import "wasm:js-string" "compare" (func $compare (type $1) (param externref externref) (result i32)))
5256

53-
;; CHECK: (import "wasm:js-string" "length" (func $length (type $13) (param externref) (result i32)))
57+
;; CHECK: (import "wasm:js-string" "length" (func $length (type $14) (param externref) (result i32)))
58+
59+
;; CHECK: (import "wasm:js-string" "codePointAt" (func $codePointAt (type $15) (param externref i32) (result i32)))
5460

55-
;; CHECK: (import "wasm:js-string" "codePointAt" (func $codePointAt (type $14) (param externref i32) (result i32)))
61+
;; CHECK: (import "wasm:js-string" "substring" (func $substring (type $16) (param externref i32 i32) (result (ref extern))))
5662

57-
;; CHECK: (import "wasm:js-string" "substring" (func $substring (type $15) (param externref i32 i32) (result (ref extern))))
63+
;; CHECK: (export "export.1" (func $exported-string-returner))
5864

59-
;; CHECK: (func $string.as (type $8) (param $a externref) (param $b externref) (param $c externref) (param $d externref)
65+
;; CHECK: (export "export.2" (func $exported-string-receiver))
66+
67+
;; CHECK: (func $string.as (type $9) (param $a externref) (param $b externref) (param $c externref) (param $d externref)
6068
;; CHECK-NEXT: (local.set $b
6169
;; CHECK-NEXT: (local.get $a)
6270
;; CHECK-NEXT: )
@@ -91,7 +99,7 @@
9199
)
92100
)
93101

94-
;; CHECK: (func $string.new.gc (type $7) (param $array16 (ref $array16))
102+
;; CHECK: (func $string.new.gc (type $8) (param $array16 (ref $array16))
95103
;; CHECK-NEXT: (drop
96104
;; CHECK-NEXT: (call $fromCharCodeArray
97105
;; CHECK-NEXT: (local.get $array16)
@@ -110,8 +118,8 @@
110118
)
111119
)
112120

113-
;; CHECK: (func $string.from_code_point (type $6) (result externref)
114-
;; CHECK-NEXT: (call $fromCodePoint_11
121+
;; CHECK: (func $string.from_code_point (type $3) (result externref)
122+
;; CHECK-NEXT: (call $fromCodePoint_13
115123
;; CHECK-NEXT: (i32.const 1)
116124
;; CHECK-NEXT: )
117125
;; CHECK-NEXT: )
@@ -121,7 +129,7 @@
121129
)
122130
)
123131

124-
;; CHECK: (func $string.encode (type $5) (param $ref externref) (param $array16 (ref $array16)) (result i32)
132+
;; CHECK: (func $string.encode (type $7) (param $ref externref) (param $array16 (ref $array16)) (result i32)
125133
;; CHECK-NEXT: (call $intoCharCodeArray
126134
;; CHECK-NEXT: (local.get $ref)
127135
;; CHECK-NEXT: (local.get $array16)
@@ -136,7 +144,7 @@
136144
)
137145
)
138146

139-
;; CHECK: (func $string.eq (type $4) (param $a externref) (param $b externref) (result i32)
147+
;; CHECK: (func $string.eq (type $6) (param $a externref) (param $b externref) (result i32)
140148
;; CHECK-NEXT: (call $equals
141149
;; CHECK-NEXT: (local.get $a)
142150
;; CHECK-NEXT: (local.get $b)
@@ -149,7 +157,7 @@
149157
)
150158
)
151159

152-
;; CHECK: (func $string.compare (type $4) (param $a externref) (param $b externref) (result i32)
160+
;; CHECK: (func $string.compare (type $6) (param $a externref) (param $b externref) (result i32)
153161
;; CHECK-NEXT: (call $compare
154162
;; CHECK-NEXT: (local.get $a)
155163
;; CHECK-NEXT: (local.get $b)
@@ -162,7 +170,7 @@
162170
)
163171
)
164172

165-
;; CHECK: (func $string.length (type $3) (param $ref externref) (result i32)
173+
;; CHECK: (func $string.length (type $5) (param $ref externref) (result i32)
166174
;; CHECK-NEXT: (call $length
167175
;; CHECK-NEXT: (local.get $ref)
168176
;; CHECK-NEXT: )
@@ -173,7 +181,7 @@
173181
)
174182
)
175183

176-
;; CHECK: (func $string.get_codeunit (type $3) (param $ref externref) (result i32)
184+
;; CHECK: (func $string.get_codeunit (type $5) (param $ref externref) (result i32)
177185
;; CHECK-NEXT: (call $codePointAt
178186
;; CHECK-NEXT: (local.get $ref)
179187
;; CHECK-NEXT: (i32.const 2)
@@ -186,7 +194,7 @@
186194
)
187195
)
188196

189-
;; CHECK: (func $string.slice (type $2) (param $ref externref) (result externref)
197+
;; CHECK: (func $string.slice (type $4) (param $ref externref) (result externref)
190198
;; CHECK-NEXT: (call $substring
191199
;; CHECK-NEXT: (local.get $ref)
192200
;; CHECK-NEXT: (i32.const 2)
@@ -200,4 +208,38 @@
200208
(i32.const 3)
201209
)
202210
)
211+
212+
;; CHECK: (func $exported-string-returner (type $3) (result externref)
213+
;; CHECK-NEXT: (global.get $string.const_exported)
214+
;; CHECK-NEXT: )
215+
(func $exported-string-returner (export "export.1") (result stringref)
216+
;; We should update the signature of this function even though it is public
217+
;; (exported).
218+
(string.const "exported")
219+
)
220+
221+
;; CHECK: (func $exported-string-receiver (type $2) (param $x externref) (param $y i32) (param $z externref)
222+
;; CHECK-NEXT: (drop
223+
;; CHECK-NEXT: (local.get $x)
224+
;; CHECK-NEXT: )
225+
;; CHECK-NEXT: (drop
226+
;; CHECK-NEXT: (local.get $y)
227+
;; CHECK-NEXT: )
228+
;; CHECK-NEXT: (drop
229+
;; CHECK-NEXT: (local.get $z)
230+
;; CHECK-NEXT: )
231+
;; CHECK-NEXT: )
232+
(func $exported-string-receiver (export "export.2") (param $x stringref) (param $y i32) (param $z stringref)
233+
;; We should update the signature of this function even though it is public
234+
;; (exported).
235+
(drop
236+
(local.get $x)
237+
)
238+
(drop
239+
(local.get $y)
240+
)
241+
(drop
242+
(local.get $z)
243+
)
244+
)
203245
)

0 commit comments

Comments
 (0)