Fix sanitizer builds of test_minimal_runtime_hello_world. NFC #24860
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brendandahl
approved these changes
Aug 5, 2025
This test updated in emscripten-core#24849 which caused the asan and lsan versions of the test to start failing. The reason is that prior to emscripten-core#24849 there was a bug that caused the test not actually to be run via the santizers since self.cflags was being assigned using `=` rather then `+=`. This was clobbering all the builtin cflags such as `-fsanitizer=address`. Once emscripten-core#24849 this bug was fixed but that exposed another bug in the test which is that it was not including the `minimal_runtime_exit_handling.js` file. Without this file included tests that use MINIMAL_RUNTIME and EXIT_RUNTIME would see an unhandled excpetion then the program exits (due to the `throw` of `exit(0)`).
sbc100
changed the title
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Aug 5, 2025
- Avoid assignment (prefer mutation) to self.cflags (see emscripten-core#24860 for why this is better). - Parameterize test_wasm_intrinsics_simd and remove unneeded arguments. - Use inline cflags where it improves test readability. - Remove use of EMSCRIPTEN_KEEPALIVE with `main` function.
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Aug 5, 2025
- Avoid assignment (prefer mutation) to self.cflags (see emscripten-core#24860 for why this is better). - Parameterize test_wasm_intrinsics_simd and remove unneeded arguments. - Use inline cflags where it improves test readability. - Remove use of EMSCRIPTEN_KEEPALIVE with `main` function.
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Aug 5, 2025
- Avoid assignment (prefer mutation) to self.cflags (see emscripten-core#24860 for why this is better). - Parameterize test_wasm_intrinsics_simd and remove unneeded arguments. - Use inline cflags where it improves test readability. - Remove use of EMSCRIPTEN_KEEPALIVE with `main` function.
sbc100
added a commit
that referenced
this pull request
Aug 5, 2025
- Avoid assignment (prefer mutation) to self.cflags (see #24860 for why this is better). - Parameterize test_wasm_intrinsics_simd and remove unneeded arguments. - Use inline cflags where it improves test readability. - Remove use of EMSCRIPTEN_KEEPALIVE with `main` function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test updated in #24849 which caused the asan and lsan versions of the test to start failing. The reason is that prior to #24849 there was a bug that caused the test not actually to be run via the santizers since self.cflags was being assigned using
=rather then+=. This was clobbering all the builtin cflags such as-fsanitizer=address.Once #24849 this bug was fixed but that exposed another bug in the test which is that it was not including the
minimal_runtime_exit_handling.jsfile. Without this file included tests that use MINIMAL_RUNTIME and EXIT_RUNTIME would see an unhandled excpetion then the program exits (due to thethrowofexit(0)).