Skip to content

[WIP] Merge musl-1.2.1 into #13006 #13007

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

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

kleisauke
Copy link
Collaborator

Just a test PR to see how this behaves in CI and to compare my changes with #13006. See #7279 for context.

Lines with TODO(kleisauke): needs some further investigation.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 14, 2020

Since I'm close would it help if I landed my musl upgrade and you could add your pthread fixes on top ?

One thing I'm worried about is combining fixed with the musl upgrade. Ideally the upgrade could be as isolated as possible from other changes.

@kleisauke
Copy link
Collaborator Author

Since I'm close would it help if I landed my musl upgrade and you could add your pthread fixes on top ?

That would certainly help. Let me know if you need help to resolve the CI failures.

One thing I'm worried about is combining fixed with the musl upgrade. Ideally the upgrade could be as isolated as possible from other changes.

I agree, this PR needs to be split up. I plan to make separate PRs for these changes:

  • Move pthread_testcancel, pthread_detach, pthread_cancel and pthread_join from JS to musl.
  • Remove support for pthread_setschedprio, pthread_setschedparam and pthread_getschedparam (this was previously mimicked and not actually supported). Also, since commit 6f1866768a86239672ed811777dfe89994d64a65 the thread attributes are no longer stored in the pthread struct, so this makes it difficult to mimic again.
  • Implement mmap/msync for -s NODERAWFS=1 (see for e.g. this patch, was an attempt to support __map_file, but I'm not sure if it can be supported at all).
  • Don't warn (Blocking on the main thread is very dangerous ...) when attempting to join exited threads (see kleisauke@8bd9f22).
  • Build libcxx without the _LIBCPP_HAS_CATOPEN define (see kleisauke@839a856).
    • Or try to actually support catopen, catgets, catclose, etc. (see for details kleisauke@d8d2c46).
  • Move websocket_to_posix_socket.cpp to C (see kleisauke@a9312d6, this to avoid compile failures when building musl internals with a C++ compiler).
  • Fix incorrect dup2 patch (see kleisauke@f0ae46b).
  • Ignore default timezone from filesystem for standalone wasm (see kleisauke@396a5a9).
  • Fix flaky wasm2js1.test_pthread_c11_threads test (see kleisauke@b2194e4, not sure if this actually fixes it).

@sbc100
Copy link
Collaborator

sbc100 commented Dec 15, 2020

That sounds great! Really appreciate the work you are doing here.

Are there any of those patches we can split out and land now before the musl update?

@kleisauke
Copy link
Collaborator Author

As far as I know, only bullet 1 and 4 needs to be done after the musl update, the rest can be done independently.

(I actually tried to do bullet 1 with the current musl, but I didn't succeed, unfortunately)

@sbc100 sbc100 force-pushed the update_musl branch 3 times, most recently from 870e53a to 8ad514f Compare January 19, 2021 01:15
@kleisauke kleisauke force-pushed the musl-1.2.1-merge branch 2 times, most recently from 8d8cb24 to be6e8fd Compare February 16, 2021 19:47
@sbc100 sbc100 force-pushed the update_musl branch 5 times, most recently from d24d4e3 to 7b7807e Compare February 24, 2021 19:56
@kleisauke kleisauke force-pushed the musl-1.2.1-merge branch 3 times, most recently from 9724bfb to 8f7fa62 Compare February 28, 2021 19:26
@kleisauke kleisauke force-pushed the musl-1.2.1-merge branch 4 times, most recently from 8a9d113 to c08a603 Compare March 22, 2021 16:49
@kleisauke
Copy link
Collaborator Author

kleisauke commented Mar 23, 2021

All green now. Here's a list of changes that can land independently, in no order:

  • Ignore default timezone from filesystem for standalone wasm (commit daf73df).
  • Improve exit logic. NFC. (commit 094013d).
  • Disable warnings on thread pool exhaustion in posixtest (commit cd207c4).
  • Avoid __has_feature(leak_sanitizer). NFC. (commit 94314fe).

These changes require the musl update to land first (PR #13006):

  • Unify #ifdef's. NFC. (commit 9e5d54a and 6a336df).
  • Update alltypes.h{,in} to have emscripten-specific changes (commit 7943ca1 and c7c77bf).
    • This introduces an update_alltypes.sh (from musl's tools/mkalltypes.sed) script to simplify the process of updating alltypes.h.
  • Deduplicate/resync arch/emscripten/bits directory (commit bc10c37 and 83d4cc1).
  • Minor cleanups/improvements (commit fcd5d2a and dc62be8).
  • Ignore unsupported syscalls (commit 0ff3c8e).
  • PI mutexes are unsupported (commit 2c79984).
  • Robust mutexes are unsupported (commit 492a79c).
  • get/setsockopt: remove fallback (commit e7642f1).
  • Improve test_nl_types (commit 8dc1153).
  • Correct printf usage in test_statvfs.c (commit 07e894f).
  • Move pthread_barrier_wait logic to __futexwait. NFC. (commit 57ad420).
  • Simplify thread clean / exit / terminate logic. NFC. (commit 36ec7f7).
  • Rebaseline other.test_gen_struct_info (commit 7a04743).
  • Rebaseline other.test_metadce_minimal_pthreads (commit 093eade).
  • Rebaseline other.test_metadce_hello_O0 (commit 0e8295d).

Changes already landed or no longer needed:

Details
  • Remove support for pthread_setschedprio, pthread_setschedparam and pthread_getschedparam (landed as b97bbc5).
  • Implement mmap/msync for -s NODERAWFS=1 (landed as 0320c04).
  • Use the _POSIX_VERSION definition instead of 200809 (commit 7bb06de, superseded by 83d5a15).
  • Move websocket_to_posix_socket.cpp to C (commit a9312d6, superseded by 510f84d).
  • Fix incorrect dup2 patch (commit 4bfc0c0, landed as f5f4ff8).
  • Build libcxx without the _LIBCPP_HAS_CATOPEN define. NFC. (commit 5669597, landed as acdc25a).
  • Avoid nesting in worker.js. NFC. (commit 32ac920, dropped as this increases code size).
  • Rebaseline test_setlocale (commit c08a603, superseded by 477c3c2).
  • Only include C11 thread library functions when targeting pthreads (commit 829bd3f, dropped as this is necessary for the stub implementation).
  • Transform pthread allow list to deny list. NFC. (commit 3e34bdf, landed as 33cbb24).
  • Use __wasi_fd_is_valid in fstatat.c (commit d862445, no longer needed since we ensure that kstat == stat).
  • Move PTHREADS_PROFILING freeing to freeThreadData. NFC. (commit dc0588d, landed as 649b1f9).
  • Assume that exit() does not throw or requires a proxy (commit b2caa24, superseded by 3205328).
  • Fix flaky wasm2js1.test_pthread_c11_threads test (commit f21de99, has no effect, see: f21de99#r52932431).
  • Avoid generating libc struct info wtih em++ (commit 4c72c04, superseded by ee93f57).
  • Don't run pthread key dtors on application exit (commit dce4aa0, landed as 5af6a11).
  • Avoid unnecessary main browser check. NFC. (commit 1fba85e, this check was probably added to avoid the JS bounce).
  • Prefer using the files_in_path helper. NFC. (commit 10873ef, landed as deba923).
  • Explicitly create the proxied main thread as detached. NFC. (commit a0b3696, landed as 24fdc1a).
  • Ensure libc_rt_wasm inherits from MTLibrary (commit 2399dad, landed as fdd486f)
  • Ensure fprintf/printf is thread-safe (commit f78d601, landed as fdd486f).
  • Run atexits before terminating all threads (commit d73b161, landed as 181cb72).
  • Avoid casting volatile void* to void*. NFC. (commit 17b78f0, not needed).
  • Avoid global thread list in WASM (commit bbc1faa, this is not correct, see: 4dfe1d6#r59640313).
  • Remove duplicated key in dict. NFC. (commit f0dffab, landed as 8cebb9b).
  • Check against thread id instead. NFC. (commit 51099e5, landed as d9a4cf9).
  • Update pthread deny list for latest musl (commit b89c421, landed as 8b6cc91).
  • Call __stdio_exit rather than fflush (commit e2e5787, superseded by eff9ae7).
  • Align/unify wait in slices logic (commit 649c96f, landed as ba84d40).
  • Inline __run_cleanup_handlers. NFC. (commit 5f7c913, dropped as this makes debugging difficult).
  • Move pthread_cancel to native code (commit dfc1d98, landed as 3add470).
  • Remove __pthread_detached_exit in favor of __emscripten_thread_cleanup. NFC. (commit a9b8f72, landed as 97d4676).
  • Use pthread_{testcancel,join} from musl (commit 919d34a, landed as 1ac2418 and a1a7559).

I'll open PRs for the non-functional changes first, but let me know if some changes need to be prioritized. As always, any help would be greatly appreciated (I don't mind if these changes are landed by someone else).

@sbc100
Copy link
Collaborator

sbc100 commented Mar 23, 2021

Wow! Thats a great amount of work.

I will try to get #13006 landed soon so you can start landing some of these.

Splitting out and landing the non-function stuff sounds like a great first step.

@sbc100 sbc100 force-pushed the update_musl branch 3 times, most recently from b81af8a to e1bf2e6 Compare April 24, 2021 00:19
kleisauke added 22 commits June 29, 2024 09:54
See musl commit 54ca677983d47529bab8752315ac1a2b49888870.
Reverts musl commit 5994de4e02a05c19a6bddadcfb687ab2e7511bd6.
Reverts musl commit 51fd67fcbfa598e2fe1885b517451b84c0bfe3b7.
Moreover, it never worked because it uses __map_file, which is currently no-op.
posixtest.test_pthread_cond_broadcast_1_2 seems to trigger this.
`exitRuntime()` is never called when `ENVIRONMENT_IS_PTHREAD` is true.
__has_feature(leak_sanitizer) cannot be used to detect the existence of LSan.
This change was generated using:
$ ./test/runner other.*code_size* other.*metadce* --rebase
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.

Upgrade musl to latest release
2 participants