Update fuzzer to newer GC spec regarding JS interop #4965
Conversation
| bool validExportResults = | ||
| std::all_of(resultType.begin(), resultType.end(), validExportType); |
There was a problem hiding this comment.
I don't think non-nullable values are allowed in the return position, either.
There was a problem hiding this comment.
Hmm, they do work in the fuzzer itself (we don't mind receiving such values, the problem is manufacturing them in order to send them), and I don't see errors in V8? It accepts e.g. (result (ref func)) on an export, and I'm not sure why it shouldn't?
There was a problem hiding this comment.
This contradicts the draft document, which states, "Function parameters and results: only externref + funcref allowed." @jakobkummerow am I reading the intention there incorrectly?
There was a problem hiding this comment.
Ha, good question!
We've only partially implemented this (because we've allowed more so far, dating back to older guesses about what the JS API might allow, and wanted to be careful about causing breakage), so the fact that you're not getting errors currently isn't a reliable datapoint.
The quoted phrasing disallows non-nullable refs. The intention is that there will be no or absolutely minimal implicit checking at the boundary (for funcref we can't get around checking whether it's a function, but that ship has sailed; stringref can't get around a string check but is considered important enough to allow). To achieve that goal, it's sufficient if the result types of exported Wasm functions are restricted to being subtypes of funcref or externref or stringref, which in particular includes the non-nullable variants of these types. I don't feel strongly about how to resolve this:
(1) most strict option: require exactly funcref or externref or stringref, nothing else, for both parameters and results.
(2) slightly relaxed: only funcref or externref or stringref for parameters, optionally non-nullable results.
(3) maximally relaxed: parameters must be exactly funcref or externref or stringref, results must be subtypes of funcref or externref or stringref (which for the latter two is almost the same as option (2): it would also allow the respective none-type).
Notably none of these options has an impact on run-time performance, they're just minor details of static type checking behavior. I assume that for module producers it doesn't make a practical difference, in particular because of implicit upcasting for any return values (e.g. a function's return type could be relaxed from (result (ref $my_custom_func)) to (result funcref) without changing anything in the function's body).
I think I'm weakly leaning towards option (1) because I don't yet see compelling reasons for anything else, but I could be convinced otherwise. WDYT?
Note 1: option (3) has some analogy to function subtyping: parameters must be "contravariant", i.e. must be supertypes of funcref or externref. Since these are top types, their only supertypes are themselves. Results must be "covariant", i.e. subtypes of F/E. Stringrefs break this analogy though: we don't want to allow anyref parameters, even though that's (most likely) a supertype of stringref.
Note 2: in all statements about required types, there's an implicit "...or one of the traditional Wasm types i32 etc." that's left out for brevity.
|
There may be more to discuss here, but what do you think about landing this to fix fuzzing @tlively ? |
Do not export functions that have types not allowed in the rules for
JS interop. Only very few GC types can be on the JS boundary atm.