Skip to content

Commit 63d308f

Browse files
kripkenkripken
authored
[Strings] Work around ref.cast not working on string views, and add fuzzing (#6549)
As suggested in #6434 (comment) , lower ref.cast of string views to ref.as_non_null in binary writing. It is a simple hack that avoids the problem of V8 not allowing them to be cast. Add fuzzing support for the last three core string operations, after which that problem becomes very frequent. Also add yet another makeTrappingRefUse that was missing in that fuzzer code.
1 parent 4e4cb62 commit 63d308f

File tree

6 files changed

+163
-39
lines changed

6 files changed

+163
-39
lines changed

‎README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,14 @@ There are a few differences between Binaryen IR and the WebAssembly language:
147147
much about this when writing Binaryen passes. For more details see the
148148
`requiresNonNullableLocalFixups()` hook in `pass.h` and the
149149
`LocalStructuralDominance` class.
150+
* Strings
151+
* Binaryen allows string views (`stringview_wtf16` etc.) to be cast using
152+
`ref.cast`. This simplifies the IR, as it allows `ref.cast` to always be
153+
used in all places (and it is lowered to `ref.as_non_null` where possible
154+
in the optimizer). The stringref spec does not seem to allow this though,
155+
and to fix that the binary writer will replace `ref.cast` that casts a
156+
string view to a non-nullable type to `ref.as_non_null`. A `ref.cast` of a
157+
string view that is a no-op is skipped entirely.
150158

151159
As a result, you might notice that round-trip conversions (wasm => Binaryen IR
152160
=> wasm) change code a little in some corner cases.

‎src/tools/fuzzing.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,10 @@ class TranslateToFuzzReader {
327327
Expression* makeStringNewArray();
328328
Expression* makeStringNewCodePoint();
329329
Expression* makeStringConcat();
330+
Expression* makeStringSlice();
330331
Expression* makeStringEq(Type type);
332+
Expression* makeStringMeasure(Type type);
333+
Expression* makeStringGet(Type type);
331334
Expression* makeStringEncode(Type type);
332335

333336
// Similar to makeBasic/CompoundRef, but indicates that this value will be

‎src/tools/fuzzing/fuzzing.cpp‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,9 @@ Expression* TranslateToFuzzReader::_makeConcrete(Type type) {
13691369
options.add(FeatureSet::ReferenceTypes | FeatureSet::GC |
13701370
FeatureSet::Strings,
13711371
&Self::makeStringEncode,
1372-
&Self::makeStringEq);
1372+
&Self::makeStringEq,
1373+
&Self::makeStringMeasure,
1374+
&Self::makeStringGet);
13731375
}
13741376
if (type.isTuple()) {
13751377
options.add(FeatureSet::Multivalue, &Self::makeTupleMake);
@@ -2620,7 +2622,7 @@ Expression* TranslateToFuzzReader::makeBasicRef(Type type) {
26202622
if (!funcContext) {
26212623
return makeStringConst();
26222624
}
2623-
switch (upTo(9)) {
2625+
switch (upTo(11)) {
26242626
case 0:
26252627
case 1:
26262628
case 2:
@@ -2639,13 +2641,16 @@ Expression* TranslateToFuzzReader::makeBasicRef(Type type) {
26392641
// generate two string children, i.e., it can lead to exponential
26402642
// growth.
26412643
return makeStringConcat();
2644+
case 9:
2645+
case 10:
2646+
return makeStringSlice();
26422647
}
26432648
WASM_UNREACHABLE("bad switch");
26442649
}
26452650
case HeapType::stringview_wtf16:
26462651
// We fully support wtf16 strings.
2647-
return builder.makeStringAs(
2648-
StringAsWTF16, makeBasicRef(Type(HeapType::string, NonNullable)));
2652+
return builder.makeStringAs(StringAsWTF16,
2653+
makeTrappingRefUse(HeapType::string));
26492654
case HeapType::stringview_wtf8:
26502655
case HeapType::stringview_iter:
26512656
// We do not have interpreter support for wtf8 and iter, so emit something
@@ -2818,6 +2823,13 @@ Expression* TranslateToFuzzReader::makeStringConcat() {
28182823
return builder.makeStringConcat(left, right);
28192824
}
28202825

2826+
Expression* TranslateToFuzzReader::makeStringSlice() {
2827+
auto* ref = makeTrappingRefUse(HeapType::stringview_wtf16);
2828+
auto* start = make(Type::i32);
2829+
auto* end = make(Type::i32);
2830+
return builder.makeStringSliceWTF(StringSliceWTF16, ref, start, end);
2831+
}
2832+
28212833
Expression* TranslateToFuzzReader::makeStringEq(Type type) {
28222834
assert(type == Type::i32);
28232835

@@ -2833,6 +2845,21 @@ Expression* TranslateToFuzzReader::makeStringEq(Type type) {
28332845
return builder.makeStringEq(StringEqCompare, left, right);
28342846
}
28352847

2848+
Expression* TranslateToFuzzReader::makeStringMeasure(Type type) {
2849+
assert(type == Type::i32);
2850+
2851+
auto* ref = makeTrappingRefUse(HeapType::string);
2852+
return builder.makeStringMeasure(StringMeasureWTF16, ref);
2853+
}
2854+
2855+
Expression* TranslateToFuzzReader::makeStringGet(Type type) {
2856+
assert(type == Type::i32);
2857+
2858+
auto* ref = makeTrappingRefUse(HeapType::stringview_wtf16);
2859+
auto* pos = make(Type::i32);
2860+
return builder.makeStringWTF16Get(ref, pos);
2861+
}
2862+
28362863
Expression* TranslateToFuzzReader::makeTrappingRefUse(HeapType type) {
28372864
auto percent = upTo(100);
28382865
// Only give a low probability to emit a nullable reference.

‎src/wasm/wasm-stack.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,6 +2089,24 @@ void BinaryInstWriter::visitRefTest(RefTest* curr) {
20892089
}
20902090

20912091
void BinaryInstWriter::visitRefCast(RefCast* curr) {
2092+
// We allow ref.cast of string views, but V8 does not. Work around that by
2093+
// emitting a ref.as_non_null (or nothing).
2094+
auto type = curr->type;
2095+
if (type.isRef()) {
2096+
auto heapType = type.getHeapType();
2097+
if (heapType == HeapType::stringview_wtf8 ||
2098+
heapType == HeapType::stringview_wtf16 ||
2099+
heapType == HeapType::stringview_iter) {
2100+
// We cannot cast string views to/from anything, so the input must also
2101+
// be a view.
2102+
assert(curr->ref->type.getHeapType() == heapType);
2103+
if (type.isNonNullable() && curr->ref->type.isNullable()) {
2104+
o << int8_t(BinaryConsts::RefAsNonNull);
2105+
}
2106+
return;
2107+
}
2108+
}
2109+
20922110
o << int8_t(BinaryConsts::GCPrefix);
20932111
if (curr->type.isNullable()) {
20942112
o << U32LEB(BinaryConsts::RefCastNull);

‎test/lit/passes/roundtrip.wast‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,70 @@
4242
)
4343
)
4444
)
45+
46+
;; CHECK: (func $string_view_casts (type $2) (param $x stringview_wtf8) (param $y stringview_wtf16) (param $z stringview_iter)
47+
;; CHECK-NEXT: (drop
48+
;; CHECK-NEXT: (ref.as_non_null
49+
;; CHECK-NEXT: (local.get $x)
50+
;; CHECK-NEXT: )
51+
;; CHECK-NEXT: )
52+
;; CHECK-NEXT: (drop
53+
;; CHECK-NEXT: (ref.as_non_null
54+
;; CHECK-NEXT: (local.get $y)
55+
;; CHECK-NEXT: )
56+
;; CHECK-NEXT: )
57+
;; CHECK-NEXT: (drop
58+
;; CHECK-NEXT: (ref.as_non_null
59+
;; CHECK-NEXT: (local.get $z)
60+
;; CHECK-NEXT: )
61+
;; CHECK-NEXT: )
62+
;; CHECK-NEXT: (drop
63+
;; CHECK-NEXT: (local.get $x)
64+
;; CHECK-NEXT: )
65+
;; CHECK-NEXT: (drop
66+
;; CHECK-NEXT: (local.get $y)
67+
;; CHECK-NEXT: )
68+
;; CHECK-NEXT: (drop
69+
;; CHECK-NEXT: (local.get $z)
70+
;; CHECK-NEXT: )
71+
;; CHECK-NEXT: )
72+
(func $string_view_casts
73+
;; ref.cast of string views is not allowed in binaries: replace with
74+
;; ref.as_non_null, or remove if it is a no-op.
75+
(param $x (ref null stringview_wtf8))
76+
(param $y (ref null stringview_wtf16))
77+
(param $z (ref null stringview_iter))
78+
;; Here we still need a cast to non-null.
79+
(drop
80+
(ref.cast (ref stringview_wtf8)
81+
(local.get $x)
82+
)
83+
)
84+
(drop
85+
(ref.cast (ref stringview_wtf16)
86+
(local.get $y)
87+
)
88+
)
89+
(drop
90+
(ref.cast (ref stringview_iter)
91+
(local.get $z)
92+
)
93+
)
94+
;; Here we do not need the cast.
95+
(drop
96+
(ref.cast (ref null stringview_wtf8)
97+
(local.get $x)
98+
)
99+
)
100+
(drop
101+
(ref.cast (ref null stringview_wtf16)
102+
(local.get $y)
103+
)
104+
)
105+
(drop
106+
(ref.cast (ref null stringview_iter)
107+
(local.get $z)
108+
)
109+
)
110+
)
45111
)

‎test/passes/translate-to-fuzz_all-features_metrics_noprint.txt‎

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,47 @@
11
total
2-
[exports] : 4
3-
[funcs] : 6
2+
[exports] : 5
3+
[funcs] : 7
44
[globals] : 14
55
[imports] : 5
66
[memories] : 1
77
[memory-data] : 20
8-
[table-data] : 1
8+
[table-data] : 2
99
[tables] : 1
1010
[tags] : 1
11-
[total] : 533
12-
[vars] : 24
13-
ArrayNew : 11
14-
ArrayNewFixed : 2
15-
AtomicFence : 1
16-
Binary : 79
17-
Block : 57
18-
Break : 6
19-
Call : 8
20-
Const : 137
21-
Drop : 1
22-
GlobalGet : 28
23-
GlobalSet : 28
24-
If : 15
11+
[total] : 467
12+
[vars] : 40
13+
ArrayGet : 1
14+
ArrayLen : 1
15+
ArrayNew : 6
16+
ArrayNewFixed : 1
17+
Binary : 67
18+
Block : 44
19+
Break : 5
20+
Call : 21
21+
Const : 106
22+
Drop : 7
23+
GlobalGet : 20
24+
GlobalSet : 18
25+
If : 14
2526
Load : 17
26-
LocalGet : 40
27-
LocalSet : 23
28-
Loop : 9
29-
Nop : 4
27+
LocalGet : 45
28+
LocalSet : 28
29+
Loop : 3
30+
MemoryFill : 1
31+
Nop : 5
3032
RefAs : 1
31-
RefFunc : 2
32-
RefI31 : 2
33-
RefIsNull : 1
34-
RefNull : 5
35-
Return : 2
36-
SIMDExtract : 2
37-
Select : 1
38-
Store : 2
39-
StringConst : 4
33+
RefCast : 1
34+
RefEq : 1
35+
RefFunc : 3
36+
RefI31 : 4
37+
RefNull : 3
38+
Return : 3
39+
Select : 2
40+
Store : 1
41+
StringConst : 3
42+
StringEncode : 1
4043
StringEq : 1
41-
StructGet : 1
42-
StructNew : 9
43-
TupleMake : 4
44-
Unary : 15
45-
Unreachable : 15
44+
StructNew : 5
45+
TupleMake : 5
46+
Unary : 13
47+
Unreachable : 10

0 commit comments

Comments
 (0)