Skip to content

Always enable deterministic_paths #23256

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

Merged
merged 3 commits into from
Jan 2, 2025

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Dec 27, 2024

This removes deterministic_paths option by turning it on all the time, in order to produce reproducible builds, both for __FILE__ macro and debug info paths.

This is an alternative to #23212, which did not remove deterministic_paths but always set only -fmacro-prefix-map on.

This PR is what was suggested in
#23195 (comment) and
#23212 (comment).

Fixes #23195.

This removes `deterministic_paths` option by turning it on all the time,
in order to produce reproducible builds, both for `__FILE__` macro and
debug info paths.

This is an alternative to emscripten-core#23212, which did not remove
`deterministic_paths` but always set only `-fmacro-prefix-map` on.

This PR is what was suggested in
emscripten-core#23195 (comment)
and
emscripten-core#23212 (comment).

Fixes emscripten-core#23195.
@aheejin aheejin requested review from sbc100 and dschuff December 27, 2024 08:57
This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (4) test expectation files were updated by
running the tests with `--rebaseline`:

```
other/codesize/test_codesize_cxx_except.size: 170928 => 170950 [+22 bytes / +0.01%]
other/codesize/test_codesize_cxx_except_wasm.size: 142143 => 142154 [+11 bytes / +0.01%]
other/codesize/test_codesize_cxx_except_wasm_exnref.size: 144730 => 144741 [+11 bytes / +0.01%]
other/codesize/test_codesize_cxx_mangle.size: 232437 => 232468 [+31 bytes / +0.01%]

Average change: +0.01% (+0.01% - +0.01%)
```
@aheejin aheejin merged commit c040578 into emscripten-core:main Jan 2, 2025
28 checks passed
@aheejin aheejin deleted the always_deterministic_path branch January 2, 2025 21:40
aheejin added a commit that referenced this pull request Jan 3, 2025
This updates libcxx and libcxxabi to LLVM 19.1.4:
https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.4

The initial update was done using `update_libcxx.py` and
`update_libcxxabi.py`, and subsequent fixes were made in indidual
commits. The commit history here is kind of messy because of CI testing
so not all individual commits are noteworthy.

Additional changes:

- Build libcxx and libcxxabi with C++23:
8b0bfdf

https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp
was added in llvm/llvm-project#87390 and this
file assumes C++23 to be compiled. Apparently libc++ sources are always
built with C++23 so they don't guard things against it in `src/`:
llvm/llvm-project#87390 (comment)
This commit also builds libc++abi with C++23 because it doesn't seem
there's any downside to it.

- Exclude newly added `compiler_rt_shims.cpp`:
5bbcbf0
We have excluded files in
https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32.
This is a new file added in this directory in
llvm/llvm-project#83575.

- Disable time zone support:
a5f2cbe
We disabled C++20 time zone support in LLVM 18 update (#21638):
df9af64
The list of source files related to time zone support has changed in
llvm/llvm-project#74928, so this commit reflects
it.

- Re-add + update `__assertion_handler` from
`default_assertion_handler.in`:
41f8037
This file was added as a part of LLVM 18 update (#21638) in
8d51927
and mistakenly deleted when I ran `update_libcxx.py`. This file was
copied from
https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in,
so this also updates the file with the newest
`default_assertion_handler.in`.

- `_LIBCPP_PSTL_CPU_BACKEND_SERIAL` -> `_LIBCPP_PSTL_BACKEND_SERIAL`:
4b969c3
The name changed in this update, so reflecting it on our
`__config_site`.

- Directly include `pthread.h` from `emscripten/val.h`:
a5a76c3
  Due to file rearrangements happened, this was necessary.

---

Other things to note:

- `std::basic_string<unsigned_char>` is not supported anymore
The support for `std::basic_string<unsigned_char>`, which was being used
by embind, is removed in this version.#23070 removes the uses from
embind.

- libcxxabi uses `__FILE__` in more places
llvm/llvm-project#80689 started to use
`_LIBCXXABI_ASSERT`, which
[uses](https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxxabi/src/abort_message.h#L22)
`__FILE__`, in `private_typeinfo.cpp`.
`__FILE__` macro produces different paths depending on the local
directory names and how the file is given to clang in the command line,
and this file is included in the result of one of our code size tests,
`other.test_minimal_runtime_code_size_hello_embind`, which caused the
result of the test to vary depending on the CI bots and how the library
is built (e.g., whether by embuilder, ninja, or neither).
Even though this was brought to surface during this LLVM 19 update,
`__FILE__` macro could be a problem for producing reproducible builds
anyway. We discussed this problem in #23195, and the fixes landed in
#23222, #23225, and #23256.
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 this pull request may close these issues.

Handling __FILE__ in code size tests in CircleCI
2 participants