Skip to content

Commit

Permalink
Implement select. (#1162)
Browse files Browse the repository at this point in the history 
* Implement `select`.

Add a `select` function, defined only on platforms where it doesn't
have an `FD_SETSIZE` limitation.

* Fix test hangs on macos.

In the `waitpid` tests, ensure that the child process has exited, as
dropping `Command` otherwise leaves the process running. This fixes
test hangs on macos.

* Wait for the child process after signaling it.

* Fix the vector sizes in the test.

* Add `fd_set` and other convenience functions.

* Switch to a safe API.

* Support `select` on Linux and Windows too.

This uses a trick where we still allow users to allocate a
`FdSetElement` array, but we just allocate a `FD_SET` on Windows
out of it.

And make `select` unsafe due to I/O safety.

* Fix qemu to implment arbitrary-sized fd sets for `select`.

* Support WASI.

* Ignore "unstable name collisions" warnings for now.
  • Loading branch information
@sunfishcode
sunfishcode committed Oct 17, 2024
1 parent 181f640 commit f371289
Show file tree
Hide file tree
Showing 18 changed files with 1,413 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ jobs:
patch -p1 < $GITHUB_WORKSPACE/ci/tiocgsid.patch
patch -p1 < $GITHUB_WORKSPACE/ci/more-sockopts.patch
patch -p1 < $GITHUB_WORKSPACE/ci/pidfd-open.patch
patch -p1 < $GITHUB_WORKSPACE/ci/select-setsize.patch
./configure --target-list=${{ matrix.qemu_target }} --prefix=${{ runner.tool_cache }}/qemu --disable-tools --disable-slirp --disable-fdt --disable-capstone --disable-docs
ninja -C build install
if: matrix.qemu != '' && matrix.os == 'ubuntu-latest'
Expand Down Expand Up @@ -624,6 +625,7 @@ jobs:
patch -p1 < $GITHUB_WORKSPACE/ci/tiocgsid.patch
patch -p1 < $GITHUB_WORKSPACE/ci/more-sockopts.patch
patch -p1 < $GITHUB_WORKSPACE/ci/pidfd-open.patch
patch -p1 < $GITHUB_WORKSPACE/ci/select-setsize.patch
./configure --target-list=${{ matrix.qemu_target }} --prefix=${{ runner.tool_cache }}/qemu --disable-tools --disable-slirp --disable-fdt --disable-capstone --disable-docs
ninja -C build install
if: matrix.qemu != '' && matrix.os == 'ubuntu-latest'
Expand Down Expand Up @@ -718,6 +720,7 @@ jobs:
patch -p1 < $GITHUB_WORKSPACE/ci/tiocgsid.patch
patch -p1 < $GITHUB_WORKSPACE/ci/more-sockopts.patch
patch -p1 < $GITHUB_WORKSPACE/ci/pidfd-open.patch
patch -p1 < $GITHUB_WORKSPACE/ci/select-setsize.patch
./configure --target-list=${{ matrix.qemu_target }} --prefix=${{ runner.tool_cache }}/qemu --disable-tools --disable-slirp --disable-fdt --disable-capstone --disable-docs
ninja -C build install
if: matrix.qemu != '' && matrix.os == 'ubuntu-latest'
Expand Down
269 changes: 269 additions & 0 deletions ci/select-setsize.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
From Dan Gohman <dev@sunfishcode.online>
Subject: [PATCH] Remove the `FD_SETSIZE` limitation in `select`

The `fd_set` type is limited to a fixed `FD_SETSIZE` number of file
descriptors, however Linux's `select has no such limitation. Change
the `select` implementation to using manual bit-vector logic to better
implement the Linux semantics.

diff -ur a/linux-user/syscall.c b/linux-user/syscall.c
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -664,8 +664,9 @@
char **, argv, char **, envp, int, flags)
#if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
-safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \
- fd_set *, exceptfds, struct timespec *, timeout, void *, sig)
+safe_syscall6(int, pselect6, int, nfds, unsigned long *, readfds, \
+ unsigned long *, writefds, unsigned long *, exceptfds, \
+ struct timespec *, timeout, void *, sig)
#endif
#if defined(TARGET_NR_ppoll) || defined(TARGET_NR_ppoll_time64)
safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
@@ -861,7 +862,7 @@

#if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
-static inline abi_long copy_from_user_fdset(fd_set *fds,
+static inline abi_long copy_from_user_fdset(unsigned long *fds,
abi_ulong target_fds_addr,
int n)
{
@@ -875,7 +876,8 @@
1)))
return -TARGET_EFAULT;

- FD_ZERO(fds);
+ memset(fds, 0, DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
k = 0;
for (i = 0; i < nw; i++) {
/* grab the abi_ulong */
@@ -883,7 +885,8 @@
for (j = 0; j < TARGET_ABI_BITS; j++) {
/* check the bit inside the abi_ulong */
if ((b >> j) & 1)
- FD_SET(k, fds);
+ fds[k / (sizeof(unsigned long) * 8)] |=
+ 1ul << (k % (sizeof(unsigned long) * 8));
k++;
}
}
@@ -893,7 +896,8 @@
return 0;
}

-static inline abi_ulong copy_from_user_fdset_ptr(fd_set *fds, fd_set **fds_ptr,
+static inline abi_ulong copy_from_user_fdset_ptr(unsigned long *fds,
+ unsigned long **fds_ptr,
abi_ulong target_fds_addr,
int n)
{
@@ -908,7 +912,7 @@
}

static inline abi_long copy_to_user_fdset(abi_ulong target_fds_addr,
- const fd_set *fds,
+ const unsigned long *fds,
int n)
{
int i, nw, j, k;
@@ -926,7 +930,10 @@
for (i = 0; i < nw; i++) {
v = 0;
for (j = 0; j < TARGET_ABI_BITS; j++) {
- v |= ((abi_ulong)(FD_ISSET(k, fds) != 0) << j);
+ bool set =
+ (fds[k / (sizeof(unsigned long) * 8)] &
+ (1ul << (k % (sizeof(unsigned long) * 8)))) != 0;
+ v |= ((abi_ulong)set << j);
k++;
}
__put_user(v, &target_fds[i]);
@@ -1295,28 +1302,40 @@
abi_ulong rfd_addr, abi_ulong wfd_addr,
abi_ulong efd_addr, abi_ulong target_tv_addr)
{
- fd_set rfds, wfds, efds;
- fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
+ unsigned long *rfds, *wfds, *efds;
+ unsigned long *rfds_ptr, *wfds_ptr, *efds_ptr;
struct timeval tv;
struct timespec ts, *ts_ptr;
abi_long ret;

- ret = copy_from_user_fdset_ptr(&rfds, &rfds_ptr, rfd_addr, n);
+ rfds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+ wfds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+ efds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+
+ ret = copy_from_user_fdset_ptr(rfds, &rfds_ptr, rfd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}
- ret = copy_from_user_fdset_ptr(&wfds, &wfds_ptr, wfd_addr, n);
+ ret = copy_from_user_fdset_ptr(wfds, &wfds_ptr, wfd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}
- ret = copy_from_user_fdset_ptr(&efds, &efds_ptr, efd_addr, n);
+ ret = copy_from_user_fdset_ptr(efds, &efds_ptr, efd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}

if (target_tv_addr) {
- if (copy_from_user_timeval(&tv, target_tv_addr))
+ if (copy_from_user_timeval(&tv, target_tv_addr)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
+ }
ts.tv_sec = tv.tv_sec;
ts.tv_nsec = tv.tv_usec * 1000;
ts_ptr = &ts;
@@ -1328,22 +1347,30 @@
ts_ptr, NULL));

if (!is_error(ret)) {
- if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n))
+ if (rfd_addr && copy_to_user_fdset(rfd_addr, rfds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
- if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n))
+ }
+ if (wfd_addr && copy_to_user_fdset(wfd_addr, wfds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
- if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
+ }
+ if (efd_addr && copy_to_user_fdset(efd_addr, efds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
+ }

if (target_tv_addr) {
tv.tv_sec = ts.tv_sec;
tv.tv_usec = ts.tv_nsec / 1000;
if (copy_to_user_timeval(target_tv_addr, &tv)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
}
}

+ free(rfds); free(wfds); free(efds);
return ret;
}

@@ -1377,8 +1404,8 @@
bool time64)
{
abi_long rfd_addr, wfd_addr, efd_addr, n, ts_addr;
- fd_set rfds, wfds, efds;
- fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
+ unsigned long *rfds, *wfds, *efds;
+ unsigned long *rfds_ptr, *wfds_ptr, *efds_ptr;
struct timespec ts, *ts_ptr;
abi_long ret;

@@ -1399,16 +1426,26 @@
efd_addr = arg4;
ts_addr = arg5;

- ret = copy_from_user_fdset_ptr(&rfds, &rfds_ptr, rfd_addr, n);
+ rfds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+ wfds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+ efds = malloc(DIV_ROUND_UP(n, sizeof(unsigned long) * 8) *
+ sizeof(unsigned long));
+
+ ret = copy_from_user_fdset_ptr(rfds, &rfds_ptr, rfd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}
- ret = copy_from_user_fdset_ptr(&wfds, &wfds_ptr, wfd_addr, n);
+ ret = copy_from_user_fdset_ptr(wfds, &wfds_ptr, wfd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}
- ret = copy_from_user_fdset_ptr(&efds, &efds_ptr, efd_addr, n);
+ ret = copy_from_user_fdset_ptr(efds, &efds_ptr, efd_addr, n);
if (ret) {
+ free(rfds); free(wfds); free(efds);
return ret;
}

@@ -1419,10 +1456,12 @@
if (ts_addr) {
if (time64) {
if (target_to_host_timespec64(&ts, ts_addr)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
} else {
if (target_to_host_timespec(&ts, ts_addr)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
}
@@ -1436,6 +1475,7 @@
if (arg6) {
arg7 = lock_user(VERIFY_READ, arg6, sizeof(*arg7) * 2, 1);
if (!arg7) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
arg_sigset = tswapal(arg7[0]);
@@ -1445,6 +1485,7 @@
if (arg_sigset) {
ret = process_sigsuspend_mask(&sig.set, arg_sigset, arg_sigsize);
if (ret != 0) {
+ free(rfds); free(wfds); free(efds);
return ret;
}
sig_ptr = &sig;
@@ -1460,25 +1501,31 @@
}

if (!is_error(ret)) {
- if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n)) {
+ if (rfd_addr && copy_to_user_fdset(rfd_addr, rfds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
- if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n)) {
+ if (wfd_addr && copy_to_user_fdset(wfd_addr, wfds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
- if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n)) {
+ if (efd_addr && copy_to_user_fdset(efd_addr, efds, n)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
if (time64) {
if (ts_addr && host_to_target_timespec64(ts_addr, &ts)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
} else {
if (ts_addr && host_to_target_timespec(ts_addr, &ts)) {
+ free(rfds); free(wfds); free(efds);
return -TARGET_EFAULT;
}
}
}
+ free(rfds); free(wfds); free(efds);
return ret;
}
#endif
Loading

0 comments on commit f371289

Please sign in to comment.