Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-sUSE_PTHREADS is simultaneously required and deprecated #19471

Closed
dhdaines opened this issue May 30, 2023 · 5 comments · Fixed by #19474
Closed

-sUSE_PTHREADS is simultaneously required and deprecated #19471

dhdaines opened this issue May 30, 2023 · 5 comments · Fixed by #19474

Comments

@dhdaines
Copy link

dhdaines commented May 30, 2023

Hi, I don't use -sUSE_PTHREADS anywhere in my configuration, yet emcc 3.1.39 crashes my build because it expects it to be defined somewhere.

If I define it, I don't just get a warning (as per the changelog) but actually failure, unless I also disable -sUSE_STRICT. According to the documentation:

The -sUSE_PTHREADS setting still works but is marked as legacy and will generate a warning in -sSTRICT mode.

This is false. It generates a warning without -sSTRICT but actually fails with it.

My linker options are (were):

-Oz
-sFILESYSTEM=0
-sALLOW_MEMORY_GROWTH=1
-sINITIAL_MEMORY=33554432
-sSUPPORT_BIG_ENDIAN=1
-sSTRICT=1
-sSTRICT_JS=0
-sSUPPORT_LONGJMP=0
--memory-init-file=0
-sENVIRONMENT=web
-sWASM=0

Here is the error:

Traceback (most recent call last):
  File "/home/dhd/work/emsdk/upstream/emscripten/emcc.py", line 4398, in <module>
    sys.exit(main(sys.argv))
  File "/usr/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/home/dhd/work/emsdk/upstream/emscripten/emcc.py", line 4391, in main
    ret = run(args)
  File "/home/dhd/work/emsdk/upstream/emscripten/emcc.py", line 1336, in run
    phase_post_link(options, state, wasm_target, wasm_target, target, js_syms)
  File "/usr/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/home/dhd/work/emsdk/upstream/emscripten/emcc.py", line 3115, in phase_post_link
    phase_binaryen(target, options, wasm_target)
  File "/usr/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/home/dhd/work/emsdk/upstream/emscripten/emcc.py", line 3761, in phase_binaryen
    wasm2js = building.wasm2js(wasm2js_template,
  File "/home/dhd/work/emsdk/upstream/emscripten/tools/building.py", line 891, in wasm2js
    if not debug_info and not settings.USE_PTHREADS:
  File "/home/dhd/work/emsdk/upstream/emscripten/tools/settings.py", line 184, in __getattr__
    raise AttributeError(f"no such setting: '{attr}'")
AttributeError: no such setting: 'USE_PTHREADS'
ninja: build stopped: subcommand failed.

Please include the following in your bug report:

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.39 (36f871819b566281d160470a1ec4515f379e9f77)
clang version 17.0.0 (https://github.com/llvm/llvm-project c672c3fe05adbb590abc99da39143b55ad510538)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /home/dhd/work/emsdk/upstream/bin

Failing command line in full:

see above

@dhdaines
Copy link
Author

Seems like the offending option here is -sWASM=0. Probably pretty easy to fix!

@sbc100
Copy link
Collaborator

sbc100 commented May 30, 2023

I think this is due a combination of -sWASM=0 and -sSTRICT=1.

We do have a pretty huge test suite which should prevent this kind of breaking in most cases, but testing every possible combination of settings is hard. In order to improve things we are constantly looking ways to pair down the number of settings (and/or valid combinations or settings) while also growing our test suite.

@sbc100
Copy link
Collaborator

sbc100 commented May 30, 2023

Actually it looks like -sWASM=0 + -STRICT + -O2 (or above) is needed to repro. Working on a fix now.

sbc100 added a commit that referenced this issue May 30, 2023
… NFC

These were not showing up in tests because they only show up with
`-sSTRICT` + `-O2` (or above).

I'm not sure how I missed them in #18923.  My guess is that they might
have shown up as part of later commits.

Fixes: #19471
sbc100 added a commit that referenced this issue May 30, 2023
… NFC

These were not showing up in tests because they only show up with
`-sSTRICT` + `-O2` (or above).

I'm not sure how I missed them in #18923.  My guess is that they might
have shown up as part of later commits.

Fixes: #19471
@dhdaines
Copy link
Author

Thanks! I erased my snarky original comment, sorry about that, I realize it isn't easy to test everything...

@sbc100
Copy link
Collaborator

sbc100 commented May 30, 2023

Thanks! I erased my snarky original comment, sorry about that, I realize it isn't easy to test everything...

Thanks for being understanding.. I can also understand your frustration when a version upgrade breaks your project!

sbc100 added a commit that referenced this issue May 30, 2023
… NFC (#19474)

These were not showing up in tests because they only show up with
`-sSTRICT` + `-O2` (or above).

I'm not sure how I missed them in #18923.  My guess is that they might
have shown up as part of later commits.

Fixes: #19471
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants