Avoid calling `poll_oneoff` with zero subscriptions. (#162)

* Avoid calling `poll_oneoff` with zero subscriptions.

With https://github.com/WebAssembly/WASI/pull/193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.

* Remove `pause` from the defined-symbols.txt list.

* Fix __wasilibc_unmodified_upstream markers.

* Check for zero subscriptions, rather than zero events.

Make `poll` and `pselect` return `ENOTSUP` when asked to poll on zero
subscriptions, rather than when the systerm returns zero events.

While here, drop the `__wasilibc_unmodified_upstream` markers, which
were already pretty noisy here, and would be significantly worse with
this change.

* Add comments about the subtle relationship between nfds and nsubscriptions.

* Rewrite the comment.

* Fix code quotes.

diff --git a/expected/wasm32-wasi/defined-symbols.txt b/expected/wasm32-wasi/defined-symbols.txt
index 448551862ccb1c37f046af7c4ae803c14f3c0092..990d1391a436f7294240fc935f6b2a4c096c46e4 100644 (file)
--- a/expected/wasm32-wasi/defined-symbols.txt
+++ b/expected/wasm32-wasi/defined-symbols.txt
@@ -805,7 +805,6 @@ optind
 optopt
 optreset
 pathconf
-pause
 perror
 poll
 posix_close

diff --git a/libc-bottom-half/cloudlibc/src/libc/poll/poll.c b/libc-bottom-half/cloudlibc/src/libc/poll/poll.c
index be76ecd7cb306822dfcf5e266244899fce976601..cde4e81c21b78897011c933ec0984a943a00450b 100644 (file)
--- a/libc-bottom-half/cloudlibc/src/libc/poll/poll.c
+++ b/libc-bottom-half/cloudlibc/src/libc/poll/poll.c
@@ -11,14 +11,14 @@ int poll(struct pollfd *fds, size_t nfds, int timeout) {
   // Construct events for poll().
   size_t maxevents = 2 * nfds + 1;
   __wasi_subscription_t subscriptions[maxevents];
-  size_t nevents = 0;
+  size_t nsubscriptions = 0;
   for (size_t i = 0; i < nfds; ++i) {
     struct pollfd *pollfd = &fds[i];
     if (pollfd->fd < 0)
       continue;
     bool created_events = false;
     if ((pollfd->events & POLLRDNORM) != 0) {
-      __wasi_subscription_t *subscription = &subscriptions[nevents++];
+      __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
       *subscription = (__wasi_subscription_t){
           .userdata = (uintptr_t)pollfd,
           .u.tag = __WASI_EVENTTYPE_FD_READ,
@@ -27,7 +27,7 @@ int poll(struct pollfd *fds, size_t nfds, int timeout) {
       created_events = true;
     }
     if ((pollfd->events & POLLWRNORM) != 0) {
-      __wasi_subscription_t *subscription = &subscriptions[nevents++];
+      __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
       *subscription = (__wasi_subscription_t){
           .userdata = (uintptr_t)pollfd,
           .u.tag = __WASI_EVENTTYPE_FD_WRITE,
@@ -47,7 +47,7 @@ int poll(struct pollfd *fds, size_t nfds, int timeout) {
 
   // Create extra event for the timeout.
   if (timeout >= 0) {
-    __wasi_subscription_t *subscription = &subscriptions[nevents++];
+    __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
     *subscription = (__wasi_subscription_t){
         .u.tag = __WASI_EVENTTYPE_CLOCK,
         .u.u.clock.id = __WASI_CLOCKID_REALTIME,
@@ -56,15 +56,24 @@ int poll(struct pollfd *fds, size_t nfds, int timeout) {
   }
 
   // Execute poll().
-  __wasi_event_t events[nevents];
+  size_t nevents;
+  __wasi_event_t events[nsubscriptions];
   __wasi_errno_t error =
-#ifdef __wasilibc_unmodified_upstream
-      __wasi_poll(subscriptions, events, nevents, &nevents);
-#else
-      __wasi_poll_oneoff(subscriptions, events, nevents, &nevents);
-#endif
+      __wasi_poll_oneoff(subscriptions, events, nsubscriptions, &nevents);
   if (error != 0) {
-    errno = error;
+    // WASI's poll requires at least one subscription, or else it returns
+    // `EINVAL`. Since a `poll` with nothing to wait for is valid in POSIX,
+    // return `ENOTSUP` to indicate that we don't support that case.
+    //
+    // Wasm has no signal handling, so if none of the user-provided `pollfd`
+    // elements, nor the timeout, led us to producing even one subscription
+    // to wait for, there would be no way for the poll to wake up. WASI
+    // returns `EINVAL` in this case, but for users of `poll`, `ENOTSUP` is
+    // more likely to be understood.
+    if (nsubscriptions == 0)
+      errno = ENOTSUP;
+    else
+      errno = error;
     return -1;
   }
 
@@ -80,18 +89,10 @@ int poll(struct pollfd *fds, size_t nfds, int timeout) {
     if (event->type == __WASI_EVENTTYPE_FD_READ ||
         event->type == __WASI_EVENTTYPE_FD_WRITE) {
       struct pollfd *pollfd = (struct pollfd *)(uintptr_t)event->userdata;
-#ifdef __wasilibc_unmodified_upstream // generated constant names
-      if (event->error == __WASI_EBADF) {
-#else
       if (event->error == __WASI_ERRNO_BADF) {
-#endif
         // Invalid file descriptor.
         pollfd->revents |= POLLNVAL;
-#ifdef __wasilibc_unmodified_upstream // generated constant names
-      } else if (event->error == __WASI_EPIPE) {
-#else
       } else if (event->error == __WASI_ERRNO_PIPE) {
-#endif
         // Hangup on write side of pipe.
         pollfd->revents |= POLLHUP;
       } else if (event->error != 0) {

diff --git a/libc-bottom-half/cloudlibc/src/libc/sys/select/pselect.c b/libc-bottom-half/cloudlibc/src/libc/sys/select/pselect.c
index bd1d2fdc0657ee582b72bae42494364d2a929675..fdc470ea85ae95ee2e284111c36e73a720675d87 100644 (file)
--- a/libc-bottom-half/cloudlibc/src/libc/sys/select/pselect.c
+++ b/libc-bottom-half/cloudlibc/src/libc/sys/select/pselect.c
@@ -11,11 +11,7 @@
 
 int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
             fd_set *restrict errorfds, const struct timespec *restrict timeout,
-#ifdef __wasilibc_unmodified_upstream
-            ...) {
-#else
             const sigset_t *sigmask) {
-#endif
   // Negative file descriptor upperbound.
   if (nfds < 0) {
     errno = EINVAL;
@@ -40,13 +36,13 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
   // Determine the maximum number of events.
   size_t maxevents = readfds->__nfds + writefds->__nfds + 1;
   __wasi_subscription_t subscriptions[maxevents];
-  size_t nevents = 0;
+  size_t nsubscriptions = 0;
 
   // Convert the readfds set.
   for (size_t i = 0; i < readfds->__nfds; ++i) {
     int fd = readfds->__fds[i];
     if (fd < nfds) {
-      __wasi_subscription_t *subscription = &subscriptions[nevents++];
+      __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
       *subscription = (__wasi_subscription_t){
           .userdata = fd,
           .u.tag = __WASI_EVENTTYPE_FD_READ,
@@ -59,7 +55,7 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
   for (size_t i = 0; i < writefds->__nfds; ++i) {
     int fd = writefds->__fds[i];
     if (fd < nfds) {
-      __wasi_subscription_t *subscription = &subscriptions[nevents++];
+      __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
       *subscription = (__wasi_subscription_t){
           .userdata = fd,
           .u.tag = __WASI_EVENTTYPE_FD_WRITE,
@@ -70,7 +66,7 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
 
   // Create extra event for the timeout.
   if (timeout != NULL) {
-    __wasi_subscription_t *subscription = &subscriptions[nevents++];
+    __wasi_subscription_t *subscription = &subscriptions[nsubscriptions++];
     *subscription = (__wasi_subscription_t){
         .u.tag = __WASI_EVENTTYPE_CLOCK,
         .u.u.clock.id = __WASI_CLOCKID_REALTIME,
@@ -82,15 +78,24 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
   }
 
   // Execute poll().
-  __wasi_event_t events[nevents];
+  size_t nevents;
+  __wasi_event_t events[nsubscriptions];
   __wasi_errno_t error =
-#ifdef __wasilibc_unmodified_upstream
-      __wasi_poll(subscriptions, events, nevents, &nevents);
-#else
-      __wasi_poll_oneoff(subscriptions, events, nevents, &nevents);
-#endif
+      __wasi_poll_oneoff(subscriptions, events, nsubscriptions, &nevents);
   if (error != 0) {
-    errno = error;
+    // WASI's poll requires at least one subscription, or else it returns
+    // `EINVAL`. Since a `pselect` with nothing to wait for is valid in POSIX,
+    // return `ENOTSUP` to indicate that we don't support that case.
+    //
+    // Wasm has no signal handling, so if none of the user-provided `pollfd`
+    // elements, nor the timeout, led us to producing even one subscription
+    // to wait for, there would be no way for the poll to wake up. WASI
+    // returns `EINVAL` in this case, but for users of `poll`, `ENOTSUP` is
+    // more likely to be understood.
+    if (nsubscriptions == 0)
+      errno = ENOTSUP;
+    else
+      errno = error;
     return -1;
   }
 
@@ -99,11 +104,7 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,
     const __wasi_event_t *event = &events[i];
     if ((event->type == __WASI_EVENTTYPE_FD_READ ||
          event->type == __WASI_EVENTTYPE_FD_WRITE) &&
-#ifdef __wasilibc_unmodified_upstream // generated constant names
-        event->error == __WASI_EBADF) {
-#else
         event->error == __WASI_ERRNO_BADF) {
-#endif
       errno = EBADF;
       return -1;
     }

diff --git a/libc-bottom-half/sources/pause.c b/libc-bottom-half/sources/pause.c
deleted file mode 100644 (file)
index 0e3e712..0000000
--- a/libc-bottom-half/sources/pause.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include <stddef.h>
-#include <errno.h>
-#include <wasi/api.h>
-#include <unistd.h>
-
-int pause(void) {
-    size_t n;
-    __wasi_errno_t error = __wasi_poll_oneoff(0, 0, 0, &n);
-    if (error != 0) {
-        errno = error;
-        return -1;
-    }
-    __builtin_trap();
-}

diff --git a/libc-top-half/musl/include/unistd.h b/libc-top-half/musl/include/unistd.h
index c6fe070bbbf3a02a6b22f80c2c3912950e8a3123..e55f638e9e59fce83cd114e8d6e16d149938dd9c 100644 (file)
--- a/libc-top-half/musl/include/unistd.h
+++ b/libc-top-half/musl/include/unistd.h
@@ -128,7 +128,9 @@ char *getcwd(char *, size_t);
 unsigned alarm(unsigned);
 #endif
 unsigned sleep(unsigned);
+#ifdef __wasilibc_unmodified_upstream /* WASI has no pause */
 int pause(void);
+#endif
 
 #ifdef __wasilibc_unmodified_upstream /* WASI has no fork/exec */
 pid_t fork(void);