Bug 202433

Summary: REGRESSION (Safari 13): Accessing WebAssembly.Module creates a global Module function
Product: WebKit Reporter: William Furr <wfurr>
Component: WebAssemblyAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Critical CC: ajuma, alonzakai, keith_miller, saam, ysuzuki
Priority: P2    
Version: Other   
Hardware: All   
OS: macOS 10.14   

Description William Furr 2019-10-01 14:10:38 PDT
Emscripten by default creates a global "Module" object that has all of its exported functions for interacting with the WebAssembly instance from JavaScript (e.g. embind functions, the Emscripten C API, etc.).  When I run in Safari, Emscripten's Module object is stomped on by a global Module function that is the same as WebAssembly.Module.

Here's a minimal example showing that Module is undefined until WebAssembly.Module is accessed: https://jsfiddle.net/vbo4zugk/

Safari 13.0.1 (14608.2.11.11)
MacOS Mojave 10.14.6

This used to work, and I suspect the bug is new in Safari 13.  I don't have Safari 12 to test on, and I'm not sure how to get older versions of Safari.

This bug is breaking any website that uses WebAssembly and the default Emscripten settings.  e.g. https://keep.google.com and https://canvas.apps.chrome.
Comment 1 William Furr 2019-10-01 14:20:19 PDT
Safari 12.1.2 works fine.
Comment 2 William Furr 2019-10-01 14:38:21 PDT
Looks like this may have already been fixed: https://trac.webkit.org/changeset/249538/webkit
Comment 3 Keith Miller 2019-10-01 16:32:44 PDT
(In reply to William Furr from comment #2)
> Looks like this may have already been fixed:
> https://trac.webkit.org/changeset/249538/webkit

Yeah, I think this is a dupe. If you want a workaround, adding a ‘var Module’ before you emscripten code runs should fix the bug. Sorry about the inconvenience... :/
Comment 4 Alon Zakai 2019-10-01 17:04:35 PDT
> If you want a workaround, adding a ‘var Module’ before you emscripten code runs should fix the bug.

Oh, the emscripten code itself will have a "var Module" as the first line, typically. Or does the "var Module" need to be before or after something to do with using "WebAssembly.Module"?
Comment 5 Keith Miller 2019-10-01 20:15:46 PDT
(In reply to Alon Zakai from comment #4)
> > If you want a workaround, adding a ‘var Module’ before you emscripten code runs should fix the bug.
> 
> Oh, the emscripten code itself will have a "var Module" as the first line,
> typically. Or does the "var Module" need to be before or after something to
> do with using "WebAssembly.Module"?

As long as the emscripten module value is assigned to the var I think it should shadow the global property. I’m on my phone right now so I can’t confirm with JSfiddle... it doesn’t seem to work on mobile anyway.
Comment 6 William Furr 2019-10-02 07:27:16 PDT
It depends on how you declare the Module global.

Declaring it as "var Module" will prevent WebAssembly.Module from overwriting it:

> Module
< ReferenceError: Can't find variable: Module
> var Module = {}
< undefined
> WebAssembly.Module
< function Module() {
    [native code]
}
> Module
< {}

Declaring it was window.Module does not.  Accessing WebAssembly.Module overwrites a pre-existing window.Module property, but it can be redeclared after:

> window.Module
< undefined
> window.Module = {}
< {}
> WebAssembly.Module
< function Module() {
    [native code]
}
> window.Module
< function Module() {
    [native code]
}
> window.Module = {}
< {}
> window.Module
< {}

My test app does have a "var Module = { ...etc... }" as the third line of script, before the Emscripten script file that uses WebAssembly.Module is even loaded.  I still end up with "Module" as an alias for "WebAssembly.Module" by the end.  I'm not sure why.
Comment 7 Keith Miller 2019-10-02 07:57:03 PDT
(In reply to William Furr from comment #6)
> 
> My test app does have a "var Module = { ...etc... }" as the third line of
> script, before the Emscripten script file that uses WebAssembly.Module is
> even loaded.  I still end up with "Module" as an alias for
> "WebAssembly.Module" by the end.  I'm not sure why.

Do you have an example? IIRC, global variables should always shadow a global property.

Looking at https://canvas.apps.chrome, I don't see a `var Module` in the source on Mac anyway.

*** This bug has been marked as a duplicate of bug 201484 ***
Comment 8 Alon Zakai 2019-10-02 08:44:22 PDT
I'm not sure which js file to look at on the canvas web site, but in general emscripten output would start with

  var Module=typeof Module!=="undefined"?Module:{};

When minified by closure, that might become

  var b;b||(b=typeof Module !== 'undefined' ? Module : {});

Other minifiers may do slightly different things.

Users can define the Module object before the script, passing in arguments that way, which is why we check if it exists. In that case I'd expect the user code to have "var Module = ".
Comment 9 Keith Miller 2019-10-02 09:05:53 PDT
(In reply to Alon Zakai from comment #8)
> I'm not sure which js file to look at on the canvas web site, but in general
> emscripten output would start with
> 
>   var Module=typeof Module!=="undefined"?Module:{};
> 
> When minified by closure, that might become
> 
>   var b;b||(b=typeof Module !== 'undefined' ? Module : {});
> 
> Other minifiers may do slightly different things.
> 
> Users can define the Module object before the script, passing in arguments
> that way, which is why we check if it exists. In that case I'd expect the
> user code to have "var Module = ".

I just did a regex search across all the js files for /var(.*)[^\w]Module[^\w]/ and didn't see anything. Maybe I missed the var declaration though?
Comment 10 William Furr 2019-10-02 11:20:43 PDT
Looks like Keep, Canvas, etc. are all using the "window.Module" form or an equivalent.

I had an internal demo that I thought had "var Module = { stuff }" at the top, but I was looking at the wrong file.  When I test that version in Safari, the Emscripten Module loads correctly.

I will see about getting workarounds into Keep, Canvas, etc.  Is there a timeline for this fix showing up in a new version of Safari?
Comment 11 William Furr 2019-10-02 11:21:01 PDT
Thank you for your help, Keith and Alon.
Comment 12 Keith Miller 2019-10-02 11:24:32 PDT
(In reply to William Furr from comment #10)
> Looks like Keep, Canvas, etc. are all using the "window.Module" form or an
> equivalent.
> 
> I had an internal demo that I thought had "var Module = { stuff }" at the
> top, but I was looking at the wrong file.  When I test that version in
> Safari, the Emscripten Module loads correctly.
> 
> I will see about getting workarounds into Keep, Canvas, etc.  Is there a
> timeline for this fix showing up in a new version of Safari?

Great, I'm glad this was worked out! Thanks again for working around our bug and again sorry for the inconvenience...

Unfortunately, Apple has a policy prohibiting comments on future product releases, so I can't give you a timeline for when/if a fix will ship. :(
Comment 13 William Furr 2019-10-22 07:57:25 PDT
Still present in Safari Version 13.0.2 (14608.2.40.1.2).