Skip to content

Upgrade to emsdk v3.1.48 breaking build #20721

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

Closed
oboukli opened this issue Nov 15, 2023 · 4 comments · Fixed by #20722
Closed

Upgrade to emsdk v3.1.48 breaking build #20721

oboukli opened this issue Nov 15, 2023 · 4 comments · Fixed by #20722

Comments

@oboukli
Copy link
Contributor

oboukli commented Nov 15, 2023

This popped up in v3.1.48

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.48 (e967e20b4727956a30592165a3c1cde5c67fa0a8)
clang version 18.0.0 (https://github.com/llvm/llvm-project a54545ba6514802178cf7cf1c1dd9f7efbf3cde7)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /emsdk/upstream/bin

Failing command line in full:

main.c

int main() {}
emcc -Wall -Werror \
--closure 1 \
-s ASSERTIONS=0 \
-s ASYNCIFY \
-s STRICT \
main.c \
-o main.js
building:ERROR: Closure compiler run failed:

building:ERROR: /tmp/emscripten_temp_pxv1nuis/main.js:927:20: ERROR - [JSC_UNDEFINED_VARIABLE] variable assert is undeclared
  927|                     assert(y === x);
                           ^^^^^^

1 error(s), 0 warning(s)

emcc: error: closure compiler failed (rc: 1): /emsdk/node/16.20.0_64bit/bin/node --max_old_space_size=8192 /emsdk/upstream/emscripten/node_modules/.bin/google-closure-compiler --compilation_level ADVANCED_OPTIMIZATIONS --language_in ECMASCRIPT_2021 --language_out NO_TRANSPILE --emit_use_strict=false --externs /emsdk/upstream/emscripten/src/closure-externs/closure-externs.js --externs /emsdk/upstream/emscripten/src/closure-externs/node-externs.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/buffer.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/string_decoder.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/os.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/tty.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/domain.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/process.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/readline.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/path.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/vm.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/dgram.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/events.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/util.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/dns.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/zlib.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/punycode.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/url.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/cluster.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/stream.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/repl.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/child_process.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/https.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/querystring.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/net.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/tls.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/fs.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/http.js --externs /emsdk/upstream/emscripten/third_party/closure-compiler/node-externs/core.js --externs /emsdk/upstream/emscripten/src/closure-externs/dyncall-externs.js --js /tmp/emscripten_temp_pxv1nuis/main.js --js_output_file tmp1jccdo1f.cc.js --formatting PRETTY_PRINT
@sbc100
Copy link
Collaborator

sbc100 commented Nov 15, 2023

Looks like you fell afoul of #20592. With this change, when you enable STRICT mode we only define assert in debug builds.

It looks like we don't find all the places in the standard library that use assert with being guarded with #if ASSERTIONS. This one looks like its in src/library_async.js. WE can fix that, but for quick workaround you could temporarily remove -sSTRICT

sbc100 added a commit to sbc100/emscripten that referenced this issue Nov 15, 2023
This fixes STRICT + ASYNCIFY + ASSERTIONS=0 builds.

Fixes: emscripten-core#20721
sbc100 added a commit that referenced this issue Nov 15, 2023
This fixes STRICT + ASYNCIFY + ASSERTIONS=0 builds.

Fixes: #20721
@oboukli
Copy link
Contributor Author

oboukli commented Nov 15, 2023

Removed -s ASSERTIONS=0 from the command-line above and the build succeeded, while retaining -s STRICT.

I'm afraid #if ASSERTIONS is evaluating to true when ASSERTIONS=0. Probably the #if ASSERTIONS guards need to be changed to #if ASSERTIONS == 1

@sbc100
Copy link
Collaborator

sbc100 commented Nov 15, 2023

Removed -s ASSERTIONS=0 from the command-line above and the build succeeded, while retaining -s STRICT.

I'm afraid #if ASSERTIONS is evaluating to true when ASSERTIONS=0. Probably the #if ASSERTIONS guards need to be changed to #if ASSERTIONS == 1

No, #if ASSERTIONS block will not be included when -sASSERTIONS=0. This is very well tested.

You shouldn't need to change anything about -sASSERTIONS, its only the -sSTRICT that you will have to remove until the fix lands. When STRICT is not used you should have no problem at all.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 15, 2023

The reason removing -s ASSERTIONS=0 worked for you is that when you build without optimizations ASSERTIONS defaults to 1 so the problem doesn't manifest. The bug will only manifest when you combine ASSERTIONS=0 (or equivalently optimizations e.g. -O2) with STRICT.

tims-bsquare added a commit to tims-bsquare/wasmoon that referenced this issue Nov 20, 2023
* Modify build.sh to use 64 bit floats but 32 bit ints
  wasm supports this but not 64 bit ints
* When pushing a number see if it's within 32 bits as well as being an int
  if it's not then push it as a 64 bit float
* Add tests for negative numbers and numbers greater than 32 bit
* Fixed build issue with current emscripten emscripten-core/emscripten#20721
* Updated docs with edge cases to hopefully close off ceifa#22 ceifa#23 and ceifa#54
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