Emscripten doesn't produce error in case of names collision added with --js-library and mergeInto

[Stuonts]

In case if names collide for functions added with --js-library and mergeInto, no error is shown and latest function is injected to resulting .js

Version of emscripten/emsdk:
3.1.1

Failing command line in full:
emcc tst.cpp -o tst.html --js-library lib.js --js-library lib1.js

Full link command and output with -v appended:
"D:/emsdk/upstream/bin\clang++.exe" -target wasm32-unknown-emscripten -DEMSCRIPTEN -fignore-exceptions -fvisibility=default -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -D__EMSCRIPTEN_major__=3 -D__EMSCRIPTEN_minor__=1 -D__EMSCRIPTEN_tiny__=1 -D_LIBCPP_ABI_VERSION=2 -Werror=implicit-function-declaration -Xclang -iwithsysroot/include/SDL --sysroot=D:\emsdk\upstream\emscripten\cache\sysroot -Xclang -iwithsysroot/include\compat -v tst.cpp -c -o C:\Users\ligol\AppData\Local\Temp\emscripten_temp_83wluooq\tst_0.o
clang version 14.0.0 (https://github.com/llvm/llvm-project f142c45f1e494f8dbdcc1bcf14122d128ac8f3fe)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: D:\emsdk\upstream\bin
(in-process)
"D:\emsdk\upstream\bin\clang++.exe" -cc1 -triple wasm32-unknown-emscripten -emit-obj -mrelax-all --mrelax-relocations -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name tst.cpp -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -debugger-tuning=gdb -v "-fcoverage-compilation-dir=e:\tst\side" -resource-dir "D:\emsdk\upstream\lib\clang\14.0.0" -D EMSCRIPTEN -D EMSCRIPTEN_major=3 -D EMSCRIPTEN_minor=1 -D EMSCRIPTEN_tiny=1 -D _LIBCPP_ABI_VERSION=2 -isysroot "D:\emsdk\upstream\emscripten\cache\sysroot" -internal-isystem "D:\emsdk\upstream\emscripten\cache\sysroot/include/wasm32-emscripten/c++/v1" -internal-isystem "D:\emsdk\upstream\emscripten\cache\sysroot/include/c++/v1" -internal-isystem "D:\emsdk\upstream\lib\clang\14.0.0\include" -internal-isystem "D:\emsdk\upstream\emscripten\cache\sysroot/include/wasm32-emscripten" -internal-isystem "D:\emsdk\upstream\emscripten\cache\sysroot/include" -Werror=implicit-function-declaration -fdeprecated-macro "-fdebug-compilation-dir=e:\tst\side" -ferror-limit 19 -fmessage-length=120 -fvisibility default -fgnuc-version=4.2.1 -fcxx-exceptions -fignore-exceptions -fexceptions -fcolor-diagnostics -iwithsysroot/include/SDL "-iwithsysroot/include\compat" -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -o "C:\Users\ligol\AppData\Local\Temp\emscripten_temp_83wluooq\tst_0.o" -x c++ tst.cpp
clang -cc1 version 14.0.0 based upon LLVM 14.0.0git default target x86_64-pc-windows-msvc
ignoring nonexistent directory "D:\emsdk\upstream\emscripten\cache\sysroot/include/wasm32-emscripten/c++/v1"
ignoring nonexistent directory "D:\emsdk\upstream\emscripten\cache\sysroot/include/wasm32-emscripten"
#include "..." search starts here:
#include <...> search starts here:
D:\emsdk\upstream\emscripten\cache\sysroot/include/SDL
D:\emsdk\upstream\emscripten\cache\sysroot/include\compat
D:\emsdk\upstream\emscripten\cache\sysroot/include/c++/v1
D:\emsdk\upstream\lib\clang\14.0.0\include
D:\emsdk\upstream\emscripten\cache\sysroot/include
End of search list.
"D:/emsdk/upstream/bin\wasm-ld.exe" -o tst.wasm C:\Users\ligol\AppData\Local\Temp\emscripten_temp_83wluooq\tst_0.o -LD:\emsdk\upstream\emscripten\cache\sysroot\lib\wasm32-emscripten -lGL -lal -lhtml5 -lstubs-debug -lnoexit -lc-debug -lcompiler_rt -lc++-noexcept -lc++abi-noexcept -ldlmalloc -lc_rt -lsockets -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --import-undefined --strip-debug --export-if-defined=main --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=fflush --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_init --export=stackSave --export=stackRestore --export=stackAlloc --export=__wasm_call_ctors --export=__errno_location --export-table -z stack-size=5242880 --initial-memory=16777216 --no-entry --max-memory=16777216 --global-base=1024
"D:/emsdk/upstream\bin\wasm-emscripten-finalize" --minimize-wasm-changes --dyncalls-i64 tst.wasm -o tst.wasm --detect-features
"D:/emsdk/node/14.18.2_64bit/bin/node.exe" D:\emsdk\upstream\emscripten\src\compiler.js C:\Users\ligol\AppData\Local\Temp\tmph5p5w1h6.json
"D:/emsdk/upstream/bin\llvm-objcopy.exe" tst.wasm tst.wasm --remove-section=.debug* --remove-section=producers
"D:/emsdk/node/14.18.2_64bit/bin/node.exe" D:\emsdk\upstream\emscripten\tools\preprocessor.js C:\Users\ligol\AppData\Local\Temp\emscripten_temp_83wluooq\settings.js shell.html.

side.zip

Have a fix, but can't push branch because have no permissions

[sbc100]

IIRC this is actually a feature. The ideas is that users can override system library JS function by providing their own overrides. Right @juj?

[juj]

This is indeed an intended feature. E.g. Unity uses this routinely to hotpatch functions or to customize/override behavior.

However I do agree that this can produce accidents. I wonder if we'd want to add a new parameter to the mergeInto() function to make it either only expect to append (aborting on replaces), or also allow to replace. And then default to only allowing appending, and individual library files can choose to also allow replacing symbols.

[sbc100]

Sounds like a good idea! @Stuonts if you have to create a PR please go ahead. Otherwise, I'll mark this as help wanted.

[Stuonts]

@sbc100, @juj, would be nice to add additional param to mergeInto, have this change, but can't push because have no permissions

can push when I have permissions

[sbc100]

You would need to create a fork and then create a PR

[Stuonts]

PR updated:
#16364

[Stuonts]

PR is merged, optional param to mergeInto has been added