From 552f85075df4ac333763af20c0ab11dba4b2537a Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Mon, 17 Jan 2022 20:52:45 +0800 Subject: [PATCH] Fix __wasi_subscription_t inconsistent with wasi-libc issue (#964) Fix __wasi_subscription_t structure definition inconsistent with wasi-libc definition issue, reported by #961, tested with sleep, poll API and other wasi cases on x86-64, x86-32 and arm32 targets. --- .../include/wasmtime_ssp.h | 133 ++++++++++++------ .../sandboxed-system-primitives/src/posix.c | 47 ++++--- 2 files changed, 117 insertions(+), 63 deletions(-) diff --git a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h index 297d0c50c..560fdeb00 100644 --- a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h +++ b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h @@ -441,47 +441,100 @@ _Static_assert(sizeof(void *) != 4 || _Static_assert(sizeof(void *) != 8 || _Alignof(__wasi_iovec_t) == 8, "non-wasi data layout"); -typedef struct __wasi_subscription_t { - __wasi_userdata_t userdata; +/** + * The contents of a `subscription` when type is `eventtype::clock`. + */ +typedef struct __wasi_subscription_clock_t { + /** + * The clock against which to compare the timestamp. + */ + __wasi_clockid_t clock_id; + + uint8_t __paddings1[4]; + + /** + * The absolute or relative timestamp. + */ + __wasi_timestamp_t timeout; + + /** + * The amount of time that the implementation may wait additionally + * to coalesce with other events. + */ + __wasi_timestamp_t precision; + + /** + * Flags specifying whether the timeout is absolute or relative + */ + __wasi_subclockflags_t flags; + + uint8_t __paddings2[4]; + +} __wasi_subscription_clock_t __attribute__((aligned(8))); + +_Static_assert(sizeof(__wasi_subscription_clock_t) == 32, "witx calculated size"); +_Static_assert(_Alignof(__wasi_subscription_clock_t) == 8, "witx calculated align"); +_Static_assert(offsetof(__wasi_subscription_clock_t, clock_id) == 0, "witx calculated offset"); +_Static_assert(offsetof(__wasi_subscription_clock_t, timeout) == 8, "witx calculated offset"); +_Static_assert(offsetof(__wasi_subscription_clock_t, precision) == 16, "witx calculated offset"); +_Static_assert(offsetof(__wasi_subscription_clock_t, flags) == 24, "witx calculated offset"); + +/** + * The contents of a `subscription` when type is type is + * `eventtype::fd_read` or `eventtype::fd_write`. + */ +typedef struct __wasi_subscription_fd_readwrite_t { + /** + * The file descriptor on which to wait for it to become ready for reading or writing. + */ + __wasi_fd_t fd; + +} __wasi_subscription_fd_readwrite_t; + +_Static_assert(sizeof(__wasi_subscription_fd_readwrite_t) == 4, "witx calculated size"); +_Static_assert(_Alignof(__wasi_subscription_fd_readwrite_t) == 4, "witx calculated align"); +_Static_assert(offsetof(__wasi_subscription_fd_readwrite_t, fd) == 0, "witx calculated offset"); + +/** + * The contents of a `subscription`. + */ +typedef union __wasi_subscription_u_u_t { + __wasi_subscription_clock_t clock; + __wasi_subscription_fd_readwrite_t fd_readwrite; +} __wasi_subscription_u_u_t ; + +typedef struct __wasi_subscription_u_t { __wasi_eventtype_t type; - uint8_t __paddings[7]; - union __wasi_subscription_u { - struct __wasi_subscription_u_clock_t { - __wasi_userdata_t identifier; - __wasi_clockid_t clock_id; - uint8_t __paddings1[4]; - __wasi_timestamp_t timeout; - __wasi_timestamp_t precision; - __wasi_subclockflags_t flags; - uint8_t __paddings2[6]; - } clock; - struct __wasi_subscription_u_fd_readwrite_t { - __wasi_fd_t fd; - } fd_readwrite; - } u; -} __wasi_subscription_t __attribute__((aligned(8))); -_Static_assert( - offsetof(__wasi_subscription_t, userdata) == 0, "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, type) == 8, "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.clock.identifier) == 16, - "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.clock.clock_id) == 24, - "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.clock.timeout) == 32, "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.clock.precision) == 40, - "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.clock.flags) == 48, "non-wasi data layout"); -_Static_assert( - offsetof(__wasi_subscription_t, u.fd_readwrite.fd) == 16, - "non-wasi data layout"); -_Static_assert(sizeof(__wasi_subscription_t) == 56, "non-wasi data layout"); -_Static_assert(_Alignof(__wasi_subscription_t) == 8, "non-wasi data layout"); + __wasi_subscription_u_u_t u; +} __wasi_subscription_u_t __attribute__((aligned(8))); + +_Static_assert(sizeof(__wasi_subscription_u_t) == 40, "witx calculated size"); +_Static_assert(_Alignof(__wasi_subscription_u_t) == 8, "witx calculated align"); +_Static_assert(offsetof(__wasi_subscription_u_t, u) == 8, "witx calculated union offset"); +_Static_assert(sizeof(__wasi_subscription_u_u_t) == 32, "witx calculated union size"); +_Static_assert(_Alignof(__wasi_subscription_u_u_t) == 8, "witx calculated union align"); + +/** + * Subscription to an event. + */ +typedef struct __wasi_subscription_t { + /** + * User-provided value that is attached to the subscription in the + * implementation and returned through `event::userdata`. + */ + __wasi_userdata_t userdata; + + /** + * The type of the event to which to subscribe, and its contents + */ + __wasi_subscription_u_t u; + +} __wasi_subscription_t; + +_Static_assert(sizeof(__wasi_subscription_t) == 48, "witx calculated size"); +_Static_assert(_Alignof(__wasi_subscription_t) == 8, "witx calculated align"); +_Static_assert(offsetof(__wasi_subscription_t, userdata) == 0, "witx calculated offset"); +_Static_assert(offsetof(__wasi_subscription_t, u) == 8, "witx calculated offset"); #if defined(WASMTIME_SSP_WASI_API) #define WASMTIME_SSP_SYSCALL_NAME(name) \ diff --git a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c index 94eb0024c..7e01f85ca 100644 --- a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c +++ b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c @@ -2418,19 +2418,19 @@ wasmtime_ssp_poll_oneoff( size_t *nevents) NO_LOCK_ANALYSIS { // Sleeping. - if (nsubscriptions == 1 && in[0].type == __WASI_EVENTTYPE_CLOCK) { + if (nsubscriptions == 1 && in[0].u.type == __WASI_EVENTTYPE_CLOCK) { out[0] = (__wasi_event_t){ .userdata = in[0].userdata, - .type = in[0].type, + .type = in[0].u.type, }; #if CONFIG_HAS_CLOCK_NANOSLEEP clockid_t clock_id; - if (convert_clockid(in[0].u.clock.clock_id, &clock_id)) { + if (convert_clockid(in[0].u.u.clock.clock_id, &clock_id)) { struct timespec ts; - convert_timestamp(in[0].u.clock.timeout, &ts); + convert_timestamp(in[0].u.u.clock.timeout, &ts); int ret = clock_nanosleep( clock_id, - (in[0].u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) != 0 + (in[0].u.u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) != 0 ? TIMER_ABSTIME : 0, &ts, NULL); @@ -2441,9 +2441,9 @@ wasmtime_ssp_poll_oneoff( out[0].error = __WASI_ENOTSUP; } #else - switch (in[0].u.clock.clock_id) { + switch (in[0].u.u.clock.clock_id) { case __WASI_CLOCK_MONOTONIC: - if ((in[0].u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) + if ((in[0].u.u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) != 0) { // TODO(ed): Implement. fputs("Unimplemented absolute sleep on monotonic clock\n", @@ -2454,12 +2454,12 @@ wasmtime_ssp_poll_oneoff( // Perform relative sleeps on the monotonic clock also using // nanosleep(). This is incorrect, but good enough for now. struct timespec ts; - convert_timestamp(in[0].u.clock.timeout, &ts); + convert_timestamp(in[0].u.u.clock.timeout, &ts); nanosleep(&ts, NULL); } break; case __WASI_CLOCK_REALTIME: - if ((in[0].u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) + if ((in[0].u.u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) != 0) { // Sleeping to an absolute point in time can only be done // by waiting on a condition variable. @@ -2473,7 +2473,8 @@ wasmtime_ssp_poll_oneoff( return -1; } mutex_lock(&mutex); - cond_timedwait(&cond, &mutex, in[0].u.clock.timeout, true); + cond_timedwait(&cond, &mutex, in[0].u.u.clock.timeout, + true); mutex_unlock(&mutex); mutex_destroy(&mutex); cond_destroy(&cond); @@ -2481,7 +2482,7 @@ wasmtime_ssp_poll_oneoff( else { // Relative sleeps can be done using nanosleep(). struct timespec ts; - convert_timestamp(in[0].u.clock.timeout, &ts); + convert_timestamp(in[0].u.u.clock.timeout, &ts); nanosleep(&ts, NULL); } break; @@ -2519,18 +2520,18 @@ wasmtime_ssp_poll_oneoff( const __wasi_subscription_t *clock_subscription = NULL; for (size_t i = 0; i < nsubscriptions; ++i) { const __wasi_subscription_t *s = &in[i]; - switch (s->type) { + switch (s->u.type) { case __WASI_EVENTTYPE_FD_READ: case __WASI_EVENTTYPE_FD_WRITE: { __wasi_errno_t error = - fd_object_get_locked(&fos[i], ft, s->u.fd_readwrite.fd, + fd_object_get_locked(&fos[i], ft, s->u.u.fd_readwrite.fd, __WASI_RIGHT_POLL_FD_READWRITE, 0); if (error == 0) { // Proper file descriptor on which we can poll(). pfds[i] = (struct pollfd){ .fd = fd_number(fos[i]), - .events = s->type == __WASI_EVENTTYPE_FD_READ + .events = s->u.type == __WASI_EVENTTYPE_FD_READ ? POLLRDNORM : POLLWRNORM, }; @@ -2542,14 +2543,14 @@ wasmtime_ssp_poll_oneoff( out[(*nevents)++] = (__wasi_event_t){ .userdata = s->userdata, .error = error, - .type = s->type, + .type = s->u.type, }; } break; } case __WASI_EVENTTYPE_CLOCK: if (clock_subscription == NULL - && (s->u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) + && (s->u.u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) == 0) { // Relative timeout. fos[i] = NULL; @@ -2565,7 +2566,7 @@ wasmtime_ssp_poll_oneoff( out[(*nevents)++] = (__wasi_event_t){ .userdata = s->userdata, .error = __WASI_ENOSYS, - .type = s->type, + .type = s->u.type, }; break; } @@ -2579,7 +2580,7 @@ wasmtime_ssp_poll_oneoff( timeout = 0; } else if (clock_subscription != NULL) { - __wasi_timestamp_t ts = clock_subscription->u.clock.timeout / 1000000; + __wasi_timestamp_t ts = clock_subscription->u.u.clock.timeout / 1000000; timeout = ts > INT_MAX ? -1 : (int)ts; } else { @@ -2603,7 +2604,7 @@ wasmtime_ssp_poll_oneoff( for (size_t i = 0; i < nsubscriptions; ++i) { if (pfds[i].fd >= 0) { __wasi_filesize_t nbytes = 0; - if (in[i].type == __WASI_EVENTTYPE_FD_READ) { + if (in[i].u.type == __WASI_EVENTTYPE_FD_READ) { int l; if (ioctl(fd_number(fos[i]), FIONREAD, &l) == 0) nbytes = (__wasi_filesize_t)l; @@ -2622,7 +2623,7 @@ wasmtime_ssp_poll_oneoff( #else .error = __WASI_EBADF, #endif - .type = in[i].type, + .type = in[i].u.type, }; } else if ((pfds[i].revents & POLLERR) != 0) { @@ -2630,14 +2631,14 @@ wasmtime_ssp_poll_oneoff( out[(*nevents)++] = (__wasi_event_t){ .userdata = in[i].userdata, .error = __WASI_EIO, - .type = in[i].type, + .type = in[i].u.type, }; } else if ((pfds[i].revents & POLLHUP) != 0) { // End-of-file. out[(*nevents)++] = (__wasi_event_t){ .userdata = in[i].userdata, - .type = in[i].type, + .type = in[i].u.type, .u.fd_readwrite.nbytes = nbytes, .u.fd_readwrite.flags = __WASI_EVENT_FD_READWRITE_HANGUP, @@ -2647,7 +2648,7 @@ wasmtime_ssp_poll_oneoff( // Read or write possible. out[(*nevents)++] = (__wasi_event_t){ .userdata = in[i].userdata, - .type = in[i].type, + .type = in[i].u.type, .u.fd_readwrite.nbytes = nbytes, }; }