Use correct types for syscalls#19559
Conversation
sbc100
left a comment
There was a problem hiding this comment.
If the tests pass then LGTM.
IIRC there was a reason we did it this way when porting the musl syscall layer, but I can't remember exactly what that was.. so lets just go with this if the tests pass.
| return g_ppid; | ||
| } | ||
|
|
||
| weak int __syscall_linkat(int olddirfd, intptr_t oldpath, int newdirfd, intptr_t newpath, int flags) { |
There was a problem hiding this comment.
Should we also use pointer types here for oldpath/newpath? and elsewhere?
|
|
||
| weak int __syscall_getgroups32(int size, intptr_t list) { | ||
| if (size < 1) { | ||
| weak int __syscall_getgroups32(int count, intptr_t list) { |
There was a problem hiding this comment.
Should list be gid_t* (avoiding the case below?)
| if (internal_fstat(fd, &st)) | ||
| return -1; | ||
| return (uptr)st.st_size; | ||
| } |
There was a problem hiding this comment.
Why are these new functions needed?
sbc100
left a comment
There was a problem hiding this comment.
Can we avoid the all the changes/additions to the sanitizers? This change seems a lot more risky once you add those in?
Also changing the socket syscalls to be varargs seems a little out of scope and perhaps separable.
| int __syscall_chmod(intptr_t path, int mode); | ||
| int __syscall_getpid(void); | ||
| typedef unsigned socklen_t; | ||
| typedef unsigned long nfds_t; |
There was a problem hiding this comment.
Duplicating these definitions here seems a little sad.
|
Let me mark this as a draft, there are many more changes needed than expected. For example, doing this: -int __syscall_openat(int dirfd, intptr_t path, int flags, ...); // mode is optional
+int __syscall_openat(int dirfd, const char *path, int flags, ...); // mode is optionalWill fail within the sanitizers (which uses the internal emscripten/tools/system_libs.py Lines 1925 to 1928 in b182182 DetailsThe solution is to rework the sanitizers to avoid musl's syscall layer, similar to the NetBSD and macOS implementations. emscripten/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp Lines 165 to 167 in b182182 |
|
Yeah I think this is one reason things are the way they today.. to allow the SYSCALL stuff to work. Perhaps we could land the initial/smaller version of this change that doesn't cause some much churn? |
f7a239f to
7b111a0
Compare
768961e to
7ffdaab
Compare
|
I've just split the compiler-rt changes to PR #24655. |
0fa87e4 to
8b152e2
Compare
d7f806a to
9f9f2d1
Compare
9f9f2d1 to
23eb9af
Compare
23eb9af to
f67f2eb
Compare
f67f2eb to
ef0e3f3
Compare
By reverting musl commit 5850546e9669f793aab61dfc7c4f2c1ff35c4b29.
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (3) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_codesize_files_js_fs.json: 18351 => 18351 [+0 bytes / +0.00%] codesize/test_codesize_files_wasmfs.json: 63811 => 63811 [+0 bytes / +0.00%] codesize/test_codesize_hello_dylink_all.json: 855809 => 856045 [+236 bytes / +0.03%] Average change: +0.01% (+0.00% - +0.03%) ```
Split from: #13007.