Ensure LibPNG compiles with pthreads support when necessary

[rsielken]

Please include the following in your bug report:

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.8 (3ff7eff373d31d8c0179895165462019221f192e)
clang version 15.0.0 (https://github.com/llvm/llvm-project 80ec0ebfdc5692a58e0832125f2c6a991df9d63f)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /local/emsdk/upstream/bin

Failing command line in full:

wasm-ld: error: --shared-memory is disallowed by png.c.o because it was not compiled with 'atomics' or 'bulk-memory' features.
em++: error: '/local/emsdk/upstream/bin/wasm-ld -o /vhome/builder/sandboxes/notes/bin/wasm/bin/nomad.wasm --whole-archive -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libcrypto.so nomad.wasm.o /vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o /vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libnotes.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libnoteswasm.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libmarvelclient.a --no-whole-archive -L/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libpng.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/crtbegin.o --whole-archive -lfetch-mt --no-whole-archive -lGL-mt-ofb -lal -lhtml5 -lstubs-debug -lnoexit -lc-mt-debug -ldlmalloc-mt -lcompiler_rt-mt -lc++-mt -lc++abi-mt -lsockets-mt -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --import-undefined --import-memory --shared-memory --export-if-defined=main --export-if-defined=_emscripten_thread_init --export-if-defined=_emscripten_thread_exit --export-if-defined=_emscripten_thread_crashed --export-if-defined=emscripten_tls_init --export-if-defined=pthread_self --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__stdio_exit --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_get_base --export=emscripten_stack_init --export=__cxa_demangle --export=stackSave --export=stackRestore --export=stackAlloc --export=__wasm_call_ctors --export=__errno_location --export=emscripten_dispatch_to_thread_ --export=_emscripten_thread_free_data --export=_emscripten_allow_main_runtime_queued_calls --export=emscripten_main_browser_thread_id --export=emscripten_main_thread_process_queued_calls --export=emscripten_run_in_main_runtime_thread_js --export=emscripten_stack_set_limits --export=emscripten_builtin_memalign --export=emscripten_proxy_main --export=malloc --export=free --export=__cxa_is_pointer_type --export=__cxa_can_catch --export=setThrew --export=ntohs --export=htons --export=htonl --export=__dl_seterr --export=saveSetjmp --export=emscripten_webgl_make_context_current --export=emscripten_webgl_get_current_context --export-table -z stack-size=5242880 --initial-memory=268435456 --no-entry --max-memory=2147483648 --global-base=1024' failed (returned 1)
profiler:DEBUG: block "link" raised an exception after 1.398 seconds
not cleaning up temp files since in debug-save mode, see them in /tmp/emscripten_temp
tools.filelock:DEBUG: Attempting to release lock 140422459190048 on /tmp/emscripten_temp/emscripten.lock
tools.filelock:DEBUG: Lock 140422459190048 released on /tmp/emscripten_temp/emscripten.lock

### MAKE: ABNORMAL STATUS (256) FROM 'em++ -DEMSDK -DMOBILE -DOPENGL -DNO_DYNAMIC_LOADING -DWASM_NSL -DWASM_NFL -g2 -pthread -s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1 -s PTHREAD_POOL_SIZE=8 -s SHARED_MEMORY=1 -s FETCH=1 -s FETCH_SUPPORT_INDEXEDDB=0 -s FULL_ES2=1 -s USE_FREETYPE=1 -s USE_LIBPNG=1 -s DISABLE_EXCEPTION_CATCHING=0 -s INITIAL_MEMORY=268435456 -s ALLOW_MEMORY_GROWTH=1 -s FORCE_FILESYSTEM=1 -s ERROR_ON_UNDEFINED_SYMBOLS=1 -s OFFSCREEN_FRAMEBUFFER=1 -s DEMANGLE_SUPPORT=1 -s ABORT_ON_WASM_EXCEPTIONS=1  -DGCC3 -DGCC4 -fno-strict-aliasing -DGCC_LBLB_NOT_SUPPORTED  -felide-constructors -Wl,--whole-archive -pthread -s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1 -s PTHREAD_POOL_SIZE=8 -s SHARED_MEMORY=1 -s FETCH=1 -s FETCH_SUPPORT_INDEXEDDB=0 -s FULL_ES2=1 -s USE_FREETYPE=1 -s USE_LIBPNG=1 -s DISABLE_EXCEPTION_CATCHING=0 -s INITIAL_MEMORY=268435456 -s ALLOW_MEMORY_GROWTH=1 -s FORCE_FILESYSTEM=1 -s ERROR_ON_UNDEFINED_SYMBOLS=1 -s OFFSCREEN_FRAMEBUFFER=1 -s DEMANGLE_SUPPORT=1 -s ABORT_ON_WASM_EXCEPTIONS=1 -O0 --closure 0 --memory-init-file 1 --js-library $EMSDK/upstream/emscripten/src/library_websocket.js -lidbfs.js -L/vhome/builder/sandboxes/notes/bin/wasm/bin -lcrypto -o $NOTESBIN/ -o /vhome/builder/sandboxes/notes/bin/wasm/bin/nomad.html `build %1 @obj.nomad.wasm.tmp -d -s` /vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o /vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o -Wl -L/vhome/builder/sandboxes/notes/bin/wasm/bin -lnotes -lnoteswasm -lmarvelclient -Wl,--no-whole-archive -lm --shell-file ./nomadshell.html '

Full link command and output with -v appended:

emcc:DEBUG: linking: ['--whole-archive', '-L/vhome/builder/sandboxes/notes/bin/wasm/bin', '/vhome/builder/sandboxes/notes/bin/wasm/bin/libcrypto.so', 'nomad.wasm.o', '/vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o', '/vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o', '-L/vhome/builder/sandboxes/notes/bin/wasm/bin', '/vhome/builder/sandboxes/notes/bin/wasm/bin/libnotes.a', '/vhome/builder/sandboxes/notes/bin/wasm/bin/libnoteswasm.a', '/vhome/builder/sandboxes/notes/bin/wasm/bin/libmarvelclient.a', '--no-whole-archive', '-L/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten', '/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libpng.a', '/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a', '/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a', '/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/crtbegin.o', '--whole-archive', '-lfetch-mt', '--no-whole-archive', '-lGL-mt-ofb', '-lal', '-lhtml5', '-lstubs-debug', '-lnoexit', '-lc-mt-debug', '-ldlmalloc-mt', '-lcompiler_rt-mt', '-lc++-mt', '-lc++abi-mt', '-lsockets-mt']
 "/local/emsdk/upstream/bin/wasm-ld" -o /vhome/builder/sandboxes/notes/bin/wasm/bin/nomad.wasm --whole-archive -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libcrypto.so nomad.wasm.o /vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o /vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libnotes.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libnoteswasm.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libmarvelclient.a --no-whole-archive -L/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libpng.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/crtbegin.o --whole-archive -lfetch-mt --no-whole-archive -lGL-mt-ofb -lal -lhtml5 -lstubs-debug -lnoexit -lc-mt-debug -ldlmalloc-mt -lcompiler_rt-mt -lc++-mt -lc++abi-mt -lsockets-mt -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --import-undefined --import-memory --shared-memory --export-if-defined=main --export-if-defined=_emscripten_thread_init --export-if-defined=_emscripten_thread_exit --export-if-defined=_emscripten_thread_crashed --export-if-defined=emscripten_tls_init --export-if-defined=pthread_self --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__stdio_exit --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_get_base --export=emscripten_stack_init --export=__cxa_demangle --export=stackSave --export=stackRestore --export=stackAlloc --export=__wasm_call_ctors --export=__errno_location --export=emscripten_dispatch_to_thread_ --export=_emscripten_thread_free_data --export=_emscripten_allow_main_runtime_queued_calls --export=emscripten_main_browser_thread_id --export=emscripten_main_thread_process_queued_calls --export=emscripten_run_in_main_runtime_thread_js --export=emscripten_stack_set_limits --export=emscripten_builtin_memalign --export=emscripten_proxy_main --export=malloc --export=free --export=__cxa_is_pointer_type --export=__cxa_can_catch --export=setThrew --export=ntohs --export=htons --export=htonl --export=__dl_seterr --export=saveSetjmp --export=emscripten_webgl_make_context_current --export=emscripten_webgl_get_current_context --export-table -z stack-size=5242880 --initial-memory=268435456 --no-entry --max-memory=2147483648 --global-base=1024
wasm-ld: error: --shared-memory is disallowed by png.c.o because it was not compiled with 'atomics' or 'bulk-memory' features.
em++: error: '/local/emsdk/upstream/bin/wasm-ld -o /vhome/builder/sandboxes/notes/bin/wasm/bin/nomad.wasm --whole-archive -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libcrypto.so nomad.wasm.o /vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o /vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o -L/vhome/builder/sandboxes/notes/bin/wasm/bin /vhome/builder/sandboxes/notes/bin/wasm/bin/libnotes.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libnoteswasm.a /vhome/builder/sandboxes/notes/bin/wasm/bin/libmarvelclient.a --no-whole-archive -L/local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libpng.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libz.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/libfreetype.a /local/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/crtbegin.o --whole-archive -lfetch-mt --no-whole-archive -lGL-mt-ofb -lal -lhtml5 -lstubs-debug -lnoexit -lc-mt-debug -ldlmalloc-mt -lcompiler_rt-mt -lc++-mt -lc++abi-mt -lsockets-mt -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --import-undefined --import-memory --shared-memory --export-if-defined=main --export-if-defined=_emscripten_thread_init --export-if-defined=_emscripten_thread_exit --export-if-defined=_emscripten_thread_crashed --export-if-defined=emscripten_tls_init --export-if-defined=pthread_self --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__stdio_exit --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_get_base --export=emscripten_stack_init --export=__cxa_demangle --export=stackSave --export=stackRestore --export=stackAlloc --export=__wasm_call_ctors --export=__errno_location --export=emscripten_dispatch_to_thread_ --export=_emscripten_thread_free_data --export=_emscripten_allow_main_runtime_queued_calls --export=emscripten_main_browser_thread_id --export=emscripten_main_thread_process_queued_calls --export=emscripten_run_in_main_runtime_thread_js --export=emscripten_stack_set_limits --export=emscripten_builtin_memalign --export=emscripten_proxy_main --export=malloc --export=free --export=__cxa_is_pointer_type --export=__cxa_can_catch --export=setThrew --export=ntohs --export=htons --export=htonl --export=__dl_seterr --export=saveSetjmp --export=emscripten_webgl_make_context_current --export=emscripten_webgl_get_current_context --export-table -z stack-size=5242880 --initial-memory=268435456 --no-entry --max-memory=2147483648 --global-base=1024' failed (returned 1)
profiler:DEBUG: block "link" raised an exception after 1.275 seconds
not cleaning up temp files since in debug-save mode, see them in /tmp/emscripten_temp
tools.filelock:DEBUG: Attempting to release lock 139682552224544 on /tmp/emscripten_temp/emscripten.lock
tools.filelock:DEBUG: Lock 139682552224544 released on /tmp/emscripten_temp/emscripten.lock

### MAKE: ABNORMAL STATUS (256) FROM 'em++ -DEMSDK -DMOBILE -DOPENGL -DNO_DYNAMIC_LOADING -DWASM_NSL -DWASM_NFL -g2 -pthread -s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1 -s PTHREAD_POOL_SIZE=8 -s SHARED_MEMORY=1 -s FETCH=1 -s FETCH_SUPPORT_INDEXEDDB=0 -s FULL_ES2=1 -s USE_FREETYPE=1 -s USE_LIBPNG=1 -s DISABLE_EXCEPTION_CATCHING=0 -s INITIAL_MEMORY=268435456 -s ALLOW_MEMORY_GROWTH=1 -s FORCE_FILESYSTEM=1 -s ERROR_ON_UNDEFINED_SYMBOLS=1 -s OFFSCREEN_FRAMEBUFFER=1 -s DEMANGLE_SUPPORT=1 -s ABORT_ON_WASM_EXCEPTIONS=1  -DGCC3 -DGCC4 -fno-strict-aliasing -DGCC_LBLB_NOT_SUPPORTED  -felide-constructors -Wl,--whole-archive -v -pthread -s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1 -s PTHREAD_POOL_SIZE=8 -s SHARED_MEMORY=1 -s FETCH=1 -s FETCH_SUPPORT_INDEXEDDB=0 -s FULL_ES2=1 -s USE_FREETYPE=1 -s USE_LIBPNG=1 -s DISABLE_EXCEPTION_CATCHING=0 -s INITIAL_MEMORY=268435456 -s ALLOW_MEMORY_GROWTH=1 -s FORCE_FILESYSTEM=1 -s ERROR_ON_UNDEFINED_SYMBOLS=1 -s OFFSCREEN_FRAMEBUFFER=1 -s DEMANGLE_SUPPORT=1 -s ABORT_ON_WASM_EXCEPTIONS=1 -O0 --closure 0 --memory-init-file 1 --js-library $EMSDK/upstream/emscripten/src/library_websocket.js -lidbfs.js -L/vhome/builder/sandboxes/notes/bin/wasm/bin -lcrypto -o $NOTESBIN/ -o /vhome/builder/sandboxes/notes/bin/wasm/bin/nomad.html `build %1 @obj.nomad.wasm.tmp -d -s` /vhome/builder/sandboxes/notes/bin/wasm/lib/mc_main.o /vhome/builder/sandboxes/notes/bin/wasm/lib/notes0.o -Wl -L/vhome/builder/sandboxes/notes/bin/wasm/bin -lnotes -lnoteswasm -lmarvelclient -Wl,--no-whole-archive -lm --shell-file ./nomadshell.html '

Similar to #16546 which fixed this issue for FreeType, shouldn't
./upstream/emscripten/tools/ports/libpng.py
be changed to add -pthread like
flags=['-sUSE_ZLIB=1', '-pthread']

Given that this is happening to at least 2 ports (we use 3 and it hit 2/3 when we tried to upgrade to EMSDK 3.1.7 or 3.1.8 from 3.1.2), is there some bigger issue here than simply changing the .py files for the ports?

[kripken]

The python files for those ports definitely should be updated. I marked this as good first bug / help wanted and hopefully someone will want to open a PR.

But if this only started happening somewhere between 3.1.2 and 3.1.7, that is odd - it should always have been an issue. So there might be a larger issue, as you said. Bisecting to find out exactly where this regressed is probably the best thing:

https://emscripten.org/docs/contributing/developers_guide.html#bisecting

[connorjclark]

@rsielken bisecting is a pretty involved process but I've recently learned how to do it for emscripten. This bug also affects me so I'm happy to handle the bisect this evening or tomorrow, unless you would prefer to investigate yourself.

[rsielken]

@connorjclark go ahead. We have worked around it by modifying the py file, so it isn't really blocking us at this point such that I'm not in a hurry to go digging too much. We have done bisecting for other issues before, so I'm capable as well, but I'll let you take the first run at it.

[connorjclark]

Bisected to: https://chromium.googlesource.com/emscripten-releases/+/3b7283668de383068a6b426727ff7c466afb48a1

It's an LLVM roll. The relevant commit is most certainly: https://chromium.googlesource.com/external/github.com/llvm/llvm-project.git/+/4f9b8397725c2f57fa671e70f23ac48e7d47fe36

[WebAssembly] Make EH/SjLj vars unconditionally thread local

This makes three thread local variables (`__THREW__`, `__threwValue`,
and `__wasm_lpad_context`) unconditionally thread local. If the target
doesn't support TLS, they will be downgraded to normal variables in
`stripThreadLocals`. This makes the object not linkable with other
objects using shared memory, which is what we intend here; these
variables should be thread local when used with shared memory. This is
what we initially tried in D88262.

But D88323 changed this: It only created these variables when threads
were supported, because `__THREW__` and `__threwValue` were always
generated even if Emscripten EH/SjLj was not used, making all objects
built without threads not linkable with shared memory, which was too
restrictive. But sometimes this is not safe. If we build an object using
variables such as `__THREW__` without threads, it can be linked to other
objects using shared memory, because the original object's `__THREW__`
was not created thread local to begin with.

So this CL basically reverts D88323 with some additional improvements:
- This checks each of the functions and global variables created within
  `LowerEmscriptenEHSjLj` pass and removes it if it's not used at the
  end of the pass. So only modules using those variables will be
  affected.
- Moves `CoalesceFeaturesAndStripAtomics` and `AtomicExpand` passes
  after all other IR pasess that can create thread local variables. It
  is not sufficient to move them to the end of `addIRPasses`, because
  `__wasm_lpad_context` is created in `WasmEHPrepare`, which runs inside
  `addPassesToHandleExceptions`, which runs before `addISelPrepare`. So
  we override `addISelPrepare` and move atomic/TLS stripping and
  expanding passes there.

This also removes merges `TLS` and `NO-TLS` FileCheck lines into one
`CHECK` line, because in the bitcode level we always create them as
thread local. Also some function declarations are deleted `CHECK` lines
because they are unused.

[kleisauke]

The root cause of this is probably because libpng uses setjmp/longjmp internally, see:
https://github.com/glennrp/libpng/blob/a37d4836519517bdce6cb9d956092321eca3e73b/png.c#L301-L303

After https://reviews.llvm.org/D120013 (Emscripten v3.1.6), setjmp/longjmp code must be compiled with -pthread, -sSUPPORT_LONGJMP=wasm or -sSHARED_MEMORY if the final link also contains one of these options.

I wonder whether compiling the libpng port with the new -sSHARED_MEMORY option would also fix this. This should probably be preferred over -pthread if the library itself does not use any pthreads functionally.

[rsielken]

Using -sSHARED_MEMORY=1 instead of -pthread at least built libpng port and our project cleanly and runs in a brief sniff test. I didn't look at the libpng code to see which would make more sense, but a sniff test indicates both work.

Also, I have not seen the issue but a colleague who was assisting the build issue that led to this issue was seeing the __THREW__ issue. I haven't been able to recreate that, but I wanted to at least mention that he saw that __THREW__ error given the results above.

[chenzx]

Is this fixed in latest 3.1.10?

i use the follow cmd to install libpng ports:

emcc -s USE_SDL=2 -s USE_FREETYPE=1 -s USE_HARFBUZZ=1 -s USE_ZLIB=1 -s USE_LIBPNG=1 -s USE_LIBJPEG=1 -s USE_GIFLIB=1 -s USE_ICU=1 -s USE_PTHREADS=1 test.cpp -o test.html

but there is only libpng.a, no libpng-mt.a

[sbc100]

Yup, this change made it into 3.1.10:

$ ./emcc -sUSE_LIBPNG -sUSE_PTHREADS tests/hello_world.c -o test.html
...
ports:INFO: unpacking port: libpng
cache:INFO: generating port: sysroot/lib/wasm32-emscripten/libpng-mt.a... (this will be cached in "/usr/local/google/home/sbc/dev/wasm/emscripten/cache/sysroot/lib/wasm32-emscripten/libpng-mt.a" for subsequent builds)
...

Are you sure you are using 3.1.10?

[chenzx]

@sbc100 3.1.10 is ok, i previously failed on old 3.1.6.

I'm now using: emcc -s ENVIRONMENT=web,worker --no-entry -s LLD_REPORT_UNDEFINED -s EXPORT_ES6=1 -s MODULARIZE=1 -s USE_ES6_IMPORT_META=0 -s USE_SDL=2 -s USE_FREETYPE=1 -s USE_HARFBUZZ=1 -s USE_ZLIB=1 -s USE_LIBPNG=1 -s USE_LIBJPEG=1 -s USE_ICU=1 -s USE_PTHREADS=1 -sSHARED_MEMORY -sFETCH -lembind ...

[sbc100]

Sound like it is fixed for you then?

[chenzx]

@sbc100 yes, it's fixed