Fix Wasm2JSBuilder leaking temporary Function #3098
Conversation
|
The alternative here is to actually add the function to the module, which might potentially be useful if someone wants to print the module after conversion? Is that a valid scenario? |
src/wasm2js.h
Outdated
| @@ -424,13 +424,13 @@ Ref Wasm2JSBuilder::processWasm(Module* wasm, Name funcName) { | |||
| }); | |||
| if (generateFetchHighBits) { | |||
| Builder builder(allocator); | |||
| asmFunc[3]->push_back(processFunction( | |||
| wasm, | |||
| std::unique_ptr<Function> tempFunc( | |||
There was a problem hiding this comment.
Since this Function only lives for a very short scope, I think we should be able to just stack allocate it, avoiding the unique_ptr (and even the builder) entirely.
There was a problem hiding this comment.
Opted for adding the function to the module now. generateFetchHighBits should already ensure that it isn't added twice iiuc. Also, there are other functions already being added if I'm not mistaken, like from i64 lowering, so this should be fine and one can inspect the module afterwards to see exactly what became converted if needed.
|
Adding the function to the modules sounds ok, too, as long as it doesn't get processed twice or something. |
Fixes
Wasm2JSBuilderleaking a temporaryFunction(WASM_FETCH_HIGH_BITS) inWasm2JSBuilder::processWasm. The function is created to be converted to JS, but is not actually part of the module, so it either needs to be cleaned up separately or be added to the module. This PR does the latter in case it is useful.