Fix binaryen.js to include allocate() explicitly

[kripken]

binaryen.js uses allocate, which is no longer exported by default in emscripten.

[sbc100]

Did you see a meaningful error to help you debug this?

[kripken]

I saw an error at compile time due to closure, which is just ALLOC_NORMAL is not defined:

https://github.com/WebAssembly/binaryen/runs/7261780292?check_suite_focus=true

Without closure I guess it would compile ok and then error at runtime in a clear way?

[sbc100]

As a side note we should probably remove the use of allocate here in favor of just using malloc.

We should also probably build with -sSTRICT..

[kripken]

I can't find a way to make this actually work... due to escaping 😢

$allocate looks like a macro, I guess, both in CMake and in Ninja. I tried the following:

• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\\\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\\\\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\\\\\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\\\\\\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE='\${allocate}'
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\${allocate}
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=\$\$allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[$]allocate
• -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[$$]allocate

Any ideas? The only other thing I can think of is to use a response file...

[sbc100]

Does it help if you use target_link_options instead? (technically this is an option and not a library)

[sbc100]

You could also used EXPORTED_RUNTIME_METHODS= which takes unmangled symbols (without the leading $)

[kripken]

target_link_options doesn't seem to help. The escaping seems the same (though it does put this earlier on the commandline).

EXPORTED_RUNTIME_METHODS seems to work.. it will increase code size with the extra export, though, but I guess that's not really significant.

I am worried about the general issue here though, since CMake is pretty popular. Requiring people to export $-prefixed names seems dangerous.

[sbc100]

target_link_options doesn't seem to help. The escaping seems the same (though it does put this earlier on the commandline).

EXPORTED_RUNTIME_METHODS seems to work.. it will increase code size with the extra export, though, but I guess that's not really significant.

I am worried about the general issue here though, since CMake is pretty popular. Requiring people to export $-prefixed names seems dangerous.

I agree, I'll take a look at how to deal with $ in cmake options. This is not a new issue though, since requiring folks to include the $ has always been true for DEFAULT_LIBRARY_FUNCS_TO_INCLUDE..

The real fix here is have binaryen stop using allocate at all I think, since its deprecated. If it really needs it we should probably look into undeprecating it.

[kripken]

Yeah, we should probably not use that API. We use it as kind of a shorthand for "allocate this array on the stack", but maybe there's a better way to do that. For strings at least I know we have helper functions for stack allocation - maybe they work on arrays too?

[sbc100]

Yeah, we should probably not use that API. We use it as kind of a shorthand for "allocate this array on the stack", but maybe there's a better way to do that. For strings at least I know we have helper functions for stack allocation - maybe they work on arrays too?

I had a go a that patch: #4795