[Webkit-unassigned] [Bug 234116] [WebAssembly][Modules] Unify memory import handling code in both module loader and JS cases

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 20 09:10:00 PST 2021


--- Comment #7 from Asumu Takikawa <asumu at igalia.com> ---
> This condition is saying, if calleeGroup exists but if it is not (!calleeGroup->compilationFinished() || calleeGroup->runnable()), then we override with the new one.
Is it right behavior? Destroying already-existing calleeGroup sounds incorrect to me.

I think the condition is saying something a bit different. It should be a negation of the related condition earlier in WasmModule.cpp in getOrCreateCalleeGroup:

> (!calleeGroup || (calleeGroup->compilationFinished() && !calleeGroup->runnable()))


> Source/JavaScriptCore/wasm/WasmModule.cpp:126
> +        if (calleeGroup && (!calleeGroup->compilationFinished() || calleeGroup->runnable()))
> +            continue;

And it also continues when the condition is true.

So this should be saying, continue and skip the `createFromExisting` call if the calleeGroup exists already and unless the compilation is finished but it's still not runnable. Would it be clearer if the second half was written as the following?

> !(calleeGroup->compilationFinished() && !calleeGroup->runnable())

I'll upload a patch with this last refactoring in case that's better.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211220/07a688e6/attachment.htm>

More information about the webkit-unassigned mailing list