Implement ppoll and pselect in terms of poll and select#26482
Implement ppoll and pselect in terms of poll and select#26482sbc100 merged 1 commit intoemscripten-core:mainfrom
Conversation
bdead93 to
6ece7ce
Compare
Because we don't support true async signals this is safe to do in userspace. Followup to emscripten-core#26084
kleisauke
left a comment
There was a problem hiding this comment.
LGTM % a question regarding the code size test.
| "$__syscall_ppoll", | ||
| "$__syscall_prlimit64", | ||
| "$__syscall_pselect6", | ||
| "$__syscall_recvmmsg", |
There was a problem hiding this comment.
Do you perhaps know why this symbol (and $__syscall_shutdown below) is included now? I noticed a similar thing in PR #19559, but haven't investigated this further.
There was a problem hiding this comment.
Hmm.. I actually have no idea. Asking gemini if it can figure that out now..
There was a problem hiding this comment.
Wow, AI figured it out pretty quick:
"""
The investigation reveals that the appearance of __syscall_recvmmsg and __syscall_shutdown in the funcs list of test_codesize_hello_dylink_all.json is a side effect of Identical Code Folding (ICF) in the linker, combined with the removal of ppoll and pselect6 from the UNIMPLEMENTED syscall stubs.
"""
There was a problem hiding this comment.
Key Findings:
- ICF Merging: In system/lib/libc/emscripten_syscall_stubs.c, many syscalls are implemented using the UNIMPLEMENTED macro. When NDEBUG is
defined (as it is in this test), these functions all have an identical body (return -ENOSYS;). The linker (Wasm-LD) merges functions with
identical bodies and the same signatures into a single WASM function to save space. - Canonical Names: When multiple functions are merged, the linker chooses one name to be the internal name of the function in the WASM.
This is usually the alphabetically first name among the merged symbols. - The Change:
- Group 1 (5 arguments): Previously contained __syscall_ppoll, __syscall_recvmmsg, and __syscall_sendmmsg. Since ppoll was
alphabetically first, it was chosen as the canonical name ($__syscall_ppoll). - Group 2 (6 arguments): Previously contained __syscall_pselect6, __syscall_shutdown, and __syscall_socketpair. Since pselect6 was
alphabetically first, it was chosen as the canonical name ($__syscall_pselect6).
- Group 1 (5 arguments): Previously contained __syscall_ppoll, __syscall_recvmmsg, and __syscall_sendmmsg. Since ppoll was
- Effect of the Commit: The commit removed ppoll and pselect6 from the UNIMPLEMENTED list in emscripten_syscall_stubs.c (as they now have
real implementations).- In the 5-arg group, __syscall_recvmmsg is now the alphabetically first member, so it became the new canonical name.
- In the 6-arg group, __syscall_shutdown is now the alphabetically first member, so it became the new canonical name. Key Findings:
- ICF Merging: In system/lib/libc/emscripten_syscall_stubs.c, many syscalls are implemented using the UNIMPLEMENTED macro. When NDEBUG is
defined (as it is in this test), these functions all have an identical body (return -ENOSYS;). The linker (Wasm-LD) merges functions with
identical bodies and the same signatures into a single WASM function to save space. - Canonical Names: When multiple functions are merged, the linker chooses one name to be the internal name of the function in the WASM.
This is usually the alphabetically first name among the merged symbols. - The Change:
- Group 1 (5 arguments): Previously contained __syscall_ppoll, __syscall_recvmmsg, and __syscall_sendmmsg. Since ppoll was
alphabetically first, it was chosen as the canonical name ($__syscall_ppoll). - Group 2 (6 arguments): Previously contained __syscall_pselect6, __syscall_shutdown, and __syscall_socketpair. Since pselect6 was
alphabetically first, it was chosen as the canonical name ($__syscall_pselect6).
- Group 1 (5 arguments): Previously contained __syscall_ppoll, __syscall_recvmmsg, and __syscall_sendmmsg. Since ppoll was
- Effect of the Commit: The commit removed ppoll and pselect6 from the UNIMPLEMENTED list in emscripten_syscall_stubs.c (as they now have
real implementations).- In the 5-arg group, __syscall_recvmmsg is now the alphabetically first member, so it became the new canonical name.
- In the 6-arg group, __syscall_shutdown is now the alphabetically first member, so it became the new canonical name.
- Validation: The total number of functions in the funcs list remained exactly the same (1689 entries). Only the names changed because the
previous "representatives" of these merged groups were removed from the codebase.
The symbols were already present as exports before the change; they only "appeared" as new internal function names because they were promoted
to being the representatives for their respective merged groups.
- Validation: The total number of functions in the funcs list remained exactly the same (1689 entries). Only the names changed because the
previous "representatives" of these merged groups were removed from the codebase.
The symbols were already present as exports before the change; they only "appeared" as new internal function names because they were promoted
to being the representatives for their respective merged groups.
Because we don't support true async signals this is safe to do in userspace.
Followup to #26084