Repository navigation
Make WASM Function Signature Support ref concrete heap type #525
Description
Activity
Hi,
Thanks for opening this issue and looking into this.
Let me start with answering your main questions:1.What are your thoughts on this proposed change?
2.Are you already working on this issue?
3. If I proceed with implementing this change, would you be open to accepting a Pull Request (PR) for it?- This is very close to my views on this. The signature ILType should move out of the
BeginWasmFunction,WasmBeginBlock,WasmBeginIfand all the other blocks that have an input signature and similarly for the operations closing the block. On top of that, there are more instructions that would need to be updated, likeWasmDefineTag,WasmDefineGlobal, ... Instead, we'll have aWasmDefineSignatureTypeoperation to define signatures inside recursive type groups which would then be passed as an input into the listed instructions. - Yes, kind of. This is the next thing I'm going to look into once I have time to continue working on the wasm-gc implementation. (Until a few weeks ago (38b76d7) we didn't even have abstract heap types in signatures, yet.)
- Due to the size of this feature, it being very intrusive and having large implications, this might not be very suitable for an external contribution. There are some design constraints that we'll need to be careful about and we're currently performing some large-scale refactoring as Fuzzilli's current code generation doesn't work well for wasm-gc in general. 12
A few more in-depth responses to your issue description:
I believe the core issue is that WasmOperation objects directly reference ILType objects. This violates the intended FuzzIL design, where ILType should be the result of the JSTyper's analysis of the FuzzIL code, not a property stored within a WasmOperation. Therefore, WasmOperation should not hold references to ILType.
This isn't a simple yes/no question. There are things that just fundamentally need some static type annotations, e.g. the
WasmDefineGlobalneeds to be able to annotate that the global should be ani32or anf64. If we didn't useILTypefor that, we'd need a separate enum or we'd need aWasmDefineGlobalI32etc., which wouldn't scale. The separate enum would be a "cleaner" solution but it would require extra conversions. So instead we have someILTypes flowing into wasm operations however, only for "built-in types", i.e. anything that is not "user"-defined / an index type in wasm-gc. Index-types (as you point out as well), require additional inputs, they depend on states in the IL program.Better aligns with the WASM GC Proposal, where struct, array, and func should be parallel types. The func type shouldn't be treated as a special case.
Yes, we'll need a
WasmDefineSignatureTypeoperation and the corresponding extensions to theILTypemechanism. The main reason why this is inconsistent right now is thatarrayandstructtypes are simpler because they didn't interfere with the existing operations for functions and blocks and the functions and blocks simply weren't built with wasm-gc in mind (as the initial goal was to cover wasm MVP and add other proposals later on.)So unfortunately, Fuzzilli is still in an intermediate stage where a lot of wasm-gc support is missing and a lot of that is blocked on having the proper signature support. Similarly, we don't have inheritance of any wasm-gc-type-definitions and we are still missing a large amount of operations (e.g. casts). On top of that, there are also other proposals that depend on wasm-gc like string-builtins, custom descriptors, ..., that therefore are also not covered by Fuzzilli, yet.
I look forward to your feedback and am willing to contribute the necessary implementation work.
I hope this answers most of the raised questions. As described, due to the scope and relevance for our progress on wasm-gc, currently this doesn't seem to be a great issue for an external contribution. If you're in general interested in contributing, I can see if I can (a) document a few more of the outstanding tasks in our issue-tracker and (b) see if we can make these issues publicly visible (right now most of the Fuzzilli project work tracking issues are marked as non-public.)
Footnotes
-
The main issue being input constraints: If the
BeginWasmFunctionsimply requires a signature type definition, we can't create this ad-hoc: When we are in the.wasmcontext, we can't emit a recursive type group definition (which we embed into the.javascriptcontext for better sharing between modules.) I'm currently leaning towards having a signature type definition statement in the.wasmcontext that is then not emedded into a recursive type group. But we'll have the same issue then in the.wasmfunctioncontext, so we'd prefer having that operation available in both of these contexts which also isn't a supported feature for operations as of now as far as I'm aware of. ↩ -
If you run Fuzzilli with default weights (or increased weights only for e.g.
WasmStructSetGenerator, it gets obvious: We will almost never emit astruct.setsuccessfully because it is very unlikely to have both a struct and a matching value to store, the struct being the more difficult part as it requires to have (1) a typegroup, (2) aWasmDefineStructTypeinside and (3) an operation that instantiates a struct (right now onlyWasmStructNewDefault). As said, this is hopefully going to be addressed soon but it's still WIP. ↩
- This is very close to my views on this. The signature ILType should move out of the
Hello,
I recently attempted to add support for custom concrete heap types in WebAssembly function signatures but discovered that Fuzzilli currently does not support this, leading to a Use-After-Free (UAF) crash. Below is a test case that reproduces the issue:
Running this test causes Fuzzilli to crash with a UAF error:
Root Cause Analysis
The issue occurs because for values of type
ref concrete heap type, theWasmReferenceTypedoes not directly hold a strong reference. Instead, it holds an unowned weak reference viaUnownedWasmTypeDescription, while theJSTyperis the actual owner of theWasmStructTypeDescriptionreference.The
getReferenceTypeTomethod creates anILTypeobject that also holds a weak reference to theWasmStructTypeDescriptionobject. Subsequently,addWasmFunction()places thisILTypeinto aWasmSignatureobject.The
WasmSignatureis itself a property of variousWasmOperationclasses (likeBeginWasmFunction).Consequently, while the
WasmStructTypeDescriptionobject is logically owned by theJSTyper, theBeginWasmFunctionoperation also maintains a weak reference to it. When theProgramBuilderfinishes and theJSTyperis deallocated, theWasmStructTypeDescriptionobject is also freed. However, duringWasmLifting, theBeginWasmFunctionoperation attempts to access theILTypeobjects within itsWasmSignature, triggering the UAF crash.Summary: The UAF occurs because the
WasmStructTypeDescriptionobject is leaked intoWasmOperationobjects which have a longer lifecycle than theJSTyper.This bug prevents function signatures from supporting concrete heap types and blocks support for additional instructions like
ref.func.Proposed Solution & Design Discussion
I believe the core issue is that
WasmOperationobjects directly referenceILTypeobjects. This violates the intended FuzzIL design, whereILTypeshould be the result of theJSTyper's analysis of the FuzzIL code, not a property stored within aWasmOperation. Therefore,WasmOperationshould not hold references toILType.Proposed Improvement:
The solution is to support defining function signatures within a Rec Group, similar to how
WasmStructTypeDescriptionandWasmArrayTypeDescriptioncurrently work. This would involve introducing a newWasmFuncSigDescriptionobject to describe function signatures.Benefits of this approach:
struct,array, andfuncshould be parallel types. Thefunctype shouldn't be treated as a special case.Variable.ref.funcandcall_ref.WasmSignaturefields inWasmOperationclasses. Instead, a new InputVariablecan be used to represent the function signature.Example refactor for
BeginWasmFunction:The primary role of
WasmSignatureduring lifting (generating the correcttypeidx) can be fulfilled by querying theJSTyperfor the type information associated with the inputVariable, as all necessary type information is already embedded within the FuzzIL code.Questions for the Maintainers
Before proceeding with the changes, I would like to understand your opinions on this matter.
I look forward to your feedback and am willing to contribute the necessary implementation work.