wasip2: Fix/optimize read/write family of syscalls (WebAssembly#670)

This commit contains a number of fixes to `{p,}{read,write}{,v}`
functions, notably:

* For `write`-style functions the buffer passed in need not be copied to
a heap allocation. Instead it can be used as-is in-place.
* `for `*v`-style functions it's not valid to perform operations in a
loop. The functions are documented as having atomic semantics (e.g.
`writev` writes everything as one "chunk") which means that the
one-write-behavior of the adapter is required.
* `preadv` no longer leaks memory for >1 buffer passed in.
* `pwritev` is implemented in terms of `pwrite` to reduce duplication.
* `writev` only performs one write instead of performing multiple and
not handling errors in the middle.
* For `read`-style functions the return value of `memcpy` need not be
checked as the function is infallible.
* The `write` function uses the return value from `check-write` to
determine the length of the write instead of discarding it.
* The download of a precompiled wasmtime was updated to support macOS as
well.

Author: alexcrichton

libc-bottom-half/cloudlibc/src/libc/sys/uio/preadv.c

@@ -8,6 +8,7 @@
 #include <wasi/wasip2.h>
 #include <wasi/file_utils.h>
 #include <common/errors.h>
+#include <unistd.h>
 #else
 #include <wasi/api.h>
 #endif
@@ -20,48 +21,16 @@ ssize_t preadv(int fildes, const struct iovec *iov, int iovcnt, off_t offset) {
   }
   size_t bytes_read = 0;
 #ifdef __wasilibc_use_wasip2
-  // Translate the file descriptor to an internal handle
-  filesystem_borrow_descriptor_t file_handle;
-  if (fd_to_file_handle(fildes, &file_handle) < 0)
-    return -1;
-
-  // Create a WASI buffer to receive the contents
-  wasip2_tuple2_list_u8_bool_t buffer;
-  filesystem_error_code_t error_code;
-  bool ok = true;
-  bool free_buffer = false;
-
-  // Differently from the behavior of the preview1 component adapter,
-  // read into all non-empty iovecs
-  for (size_t i = 0; i < iovcnt; i++) {
-    if (iov[i].iov_len == 0)
-      continue;
-
-    // Read the data
-    ok = filesystem_method_descriptor_read(file_handle,
-                                           iov[i].iov_len,
-                                           offset,
-                                           &buffer,
-                                           &error_code);
-    free_buffer = true;
-    if (buffer.f0.len > INT32_MAX) {
-      // In this case, the number of bytes read can't
-      // be represented by an ssize_t
-      errno = EINVAL;
-      wasip2_list_u8_free(&buffer.f0);
-      return -1;
+  // Skip empty iovecs and then delegate to `pread` with the first non-empty
+  // iovec.
+  while (iovcnt) {
+    if (iov->iov_len != 0) {
+      return pread(fildes, iov->iov_base, iov->iov_len, offset);
     }
-    bytes_read += buffer.f0.len;
-    // Copy the contents of the buffer into the iov
-    memcpy(iov[i].iov_base, buffer.f0.ptr, buffer.f0.len);
-  }
-  if (free_buffer)
-    wasip2_list_u8_free(&buffer.f0);
-
-  if (!ok) {
-    translate_error(error_code);
-    return -1;
+    iovcnt--;
+    iov++;
   }
+  return pread(fildes, NULL, 0, offset);
 #else
   __wasi_errno_t error = __wasi_fd_pread(
       fildes, (const __wasi_iovec_t *)iov, iovcnt, offset, &bytes_read);

libc-bottom-half/cloudlibc/src/libc/sys/uio/pwritev.c

@@ -8,6 +8,7 @@
 #include <wasi/wasip2.h>
 #include <wasi/file_utils.h>
 #include <common/errors.h>
+#include <unistd.h>
 #else
 #include <wasi/api.h>
 #endif
@@ -18,62 +19,25 @@ ssize_t pwritev(int fildes, const struct iovec *iov, int iovcnt, off_t offset) {
     errno = EINVAL;
     return -1;
   }
-  size_t bytes_written;
 #ifdef __wasilibc_use_wasip2
-  // Note: following the preview1 adapter's behavior, only the first non-empty
-  // iov is written.
-
-  // Find the first non-empty iov
-  int32_t i = 0;
-  while (i < iovcnt) {
-    if (iov[i].iov_len != 0)
-      break;
-    i++;
-  }
-
-  // If there is none, return
-  if (i >= iovcnt) {
-    return 0;
-  }
-
-  // Translate the file descriptor to an internal handle
-  filesystem_borrow_descriptor_t file_handle;
-  if (fd_to_file_handle(fildes, &file_handle) < 0)
-    return -1;
-
-  // Create a WASI buffer to copy the contents into
-  wasip2_list_u8_t buffer;
-  buffer.len = iov[i].iov_len;
-  buffer.ptr = malloc(buffer.len);
-  memcpy(buffer.ptr, iov[i].iov_base, buffer.len);
-
-  // Write the data
-  filesystem_error_code_t error_code;
-  filesystem_filesize_t num_bytes;
-  bool ok = filesystem_method_descriptor_write(file_handle,
-                                               &buffer,
-                                               offset,
-                                               &num_bytes,
-                                               &error_code);
-  wasip2_list_u8_free(&buffer);
-  if (!ok) {
-    translate_error(error_code);
-    return -1;
-  }
-  if (num_bytes > INT32_MAX) {
-    // In this case, the number of bytes written can't
-    // be represented by an ssize_t
-    errno = EINVAL;
-    return -1;
+  // Skip empty iovecs and then delegate to `pwrite` with the first non-empty
+  // iovec.
+  while (iovcnt) {
+    if (iov->iov_len != 0) {
+      return pwrite(fildes, iov->iov_base, iov->iov_len, offset);
+    }
+    iovcnt--;
+    iov++;
   }
-  bytes_written = (ssize_t) num_bytes;
+  return pwrite(fildes, NULL, 0, offset);
 #else
+  size_t bytes_written;
   __wasi_errno_t error = __wasi_fd_pwrite(
       fildes, (const __wasi_ciovec_t *)iov, iovcnt, offset, &bytes_written);
   if (error != 0) {
     errno = error;
     return -1;
   }
-#endif
   return bytes_written;
+#endif
 }

libc-bottom-half/cloudlibc/src/libc/sys/uio/writev.c

@@ -36,24 +36,26 @@ ssize_t writev(int fildes, const struct iovec *iov, int iovcnt) {
     errno = EINVAL;
     return -1;
   }
-  size_t bytes_written = 0;
 
 #ifdef __wasilibc_use_wasip2
-  // Differently from the behavior of the preview1 component adapter,
-  // write all non-empty iovecs
-
-  for (size_t i = 0; i < iovcnt; i++) {
-    if (iov[i].iov_len == 0)
-      continue;
-    bytes_written += write(fildes, iov[i].iov_base, iov[i].iov_len);
+  // Skip empty iovecs and then delegate to `read` with the first non-empty
+  // iovec.
+  while (iovcnt) {
+    if (iov->iov_len != 0) {
+      return write(fildes, iov->iov_base, iov->iov_len);
+    }
+    iovcnt--;
+    iov++;
   }
+  return write(fildes, NULL, 0);
 #else
+  size_t bytes_written = 0;
   __wasi_errno_t error = __wasi_fd_write(
       fildes, (const __wasi_ciovec_t *)iov, iovcnt, &bytes_written);
   if (error != 0) {
     errno = error;
     return -1;
   }
-#endif
   return bytes_written;
+#endif
 }

libc-bottom-half/cloudlibc/src/libc/unistd/pread.c

@@ -36,13 +36,8 @@ ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset) {
                                               &contents,
                                               &error_code);
   bytes_read = contents.f0.len;
-  // Copy the result into the buffer
-  if (!memcpy(buf, contents.f0.ptr, bytes_read)) {
-    errno = EINVAL;
-    return -1;
-  }
-
-  // Free the list in the tuple
+  // Copy the bytes allocated in the canonical ABI to `buf`
+  memcpy(buf, contents.f0.ptr, bytes_read);
   wasip2_list_u8_free(&contents.f0);
 
   // Check for errors

libc-bottom-half/cloudlibc/src/libc/unistd/pwrite.c

@@ -27,11 +27,7 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte, off_t offset) {
   // Convert `buf` to a WASI byte list
   wasip2_list_u8_t contents;
   contents.len = nbyte;
-  contents.ptr = malloc(nbyte);
-  if (!memcpy(contents.ptr, buf, nbyte)) {
-    errno = EINVAL;
-    return -1;
-  }
+  contents.ptr = (uint8_t*) buf;
 
   // Write the bytes
   filesystem_filesize_t bytes_written;
@@ -41,9 +37,6 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte, off_t offset) {
                                                offset,
                                                &bytes_written,
                                                &error_code);
-  // Free the byte list
-  wasip2_list_u8_free(&contents);
-
   // Check for errors
   if (!ok) {
     translate_error(error_code);

libc-bottom-half/cloudlibc/src/libc/unistd/read.c

@@ -42,11 +42,8 @@ ssize_t read(int fildes, void *buf, size_t nbyte) {
     }
   }
 
-  // Copy the bytes to `buf` so we can free the list
-  if (!memcpy(buf, contents.ptr, contents.len)) {
-    errno = EINVAL;
-    return -1;
-  }
+  // Copy the bytes allocated in the canonical ABI to `buf`
+  memcpy(buf, contents.ptr, contents.len);
   wasip2_list_u8_free(&contents);
 
   // Update the offset

libc-bottom-half/cloudlibc/src/libc/unistd/write.c

@@ -30,32 +30,28 @@ ssize_t write(int fildes, const void *buf, size_t nbyte) {
   // Check readiness for writing
   uint64_t num_bytes_permitted = 0;
   streams_stream_error_t stream_error;
-  ok = streams_method_output_stream_check_write(output_stream,
-                                                &num_bytes_permitted,
-                                                &stream_error);
-  if (!ok) {
-    errno = EIO;
-    return -1;
-  }
+  while (num_bytes_permitted == 0) {
+    ok = streams_method_output_stream_check_write(output_stream,
+                                                  &num_bytes_permitted,
+                                                  &stream_error);
+    if (!ok) {
+      errno = EIO;
+      return -1;
+    }
 
-  if (num_bytes_permitted < nbyte)
-    poll_method_pollable_block(pollable);
+    if (num_bytes_permitted == 0)
+      poll_method_pollable_block(pollable);
+  }
 
   // Convert the buffer to a WASI list of bytes
   wasip2_list_u8_t contents;
-  contents.len = nbyte;
-  contents.ptr = malloc(nbyte);
-  if (!memcpy(contents.ptr, buf, nbyte)) {
-    errno = EINVAL;
-    return -1;
-  }
+  contents.len = num_bytes_permitted < nbyte ? num_bytes_permitted : nbyte;
+  contents.ptr = (uint8_t*) buf;
 
   // Write the bytes to the stream
   ok = streams_method_output_stream_write(output_stream,
                                           &contents,
                                           &stream_error);
-  wasip2_list_u8_free(&contents);
-
   if (!ok) {
     errno = EIO;
     return -1;
@@ -69,8 +65,8 @@ ssize_t write(int fildes, const void *buf, size_t nbyte) {
   }
 
   if (off)
-    *off += nbyte;
-  return nbyte;
+    *off += contents.len;
+  return contents.len;
 #else
   __wasi_ciovec_t iov = {.buf = buf, .buf_len = nbyte};
   size_t bytes_written;

test/CMakeLists.txt

@@ -50,12 +50,27 @@ message(STATUS "libc-test source directory: ${LIBC_TEST}")
 
 if(NOT ENGINE OR ENGINE STREQUAL "")
   set(WASMTIME_VERSION "v38.0.2")
-  set(WASMTIME_ARCH "${CMAKE_HOST_SYSTEM_PROCESSOR}")
   set(WASMTIME_REPO "https://github.com/bytecodealliance/wasmtime")
 
+  if (CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64")
+    set(WASMTIME_ARCH "x86_64")
+  elseif (CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "arm64")
+    set(WASMTIME_ARCH "aarch64")
+  else()
+    message(FATAL_ERROR "Unsupported architecture ${CMAKE_HOST_SYSTEM_PROCESSOR} for Wasmtime")
+  endif()
+
+  if (CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin")
+    set(WASMTIME_OS macos)
+  elseif (CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
+    set(WASMTIME_OS linux)
+  else()
+    message(FATAL_ERROR "Unsupported system ${CMAKE_SYSTEM_NAME} for Wasmtime")
+  endif()
+
   ExternalProject_Add(
     engine
-    URL "${WASMTIME_REPO}/releases/download/${WASMTIME_VERSION}/wasmtime-${WASMTIME_VERSION}-${WASMTIME_ARCH}-linux.tar.xz"
+    URL "${WASMTIME_REPO}/releases/download/${WASMTIME_VERSION}/wasmtime-${WASMTIME_VERSION}-${WASMTIME_ARCH}-${WASMTIME_OS}.tar.xz"
     CONFIGURE_COMMAND ""
     BUILD_COMMAND ""
     INSTALL_COMMAND ""
@@ -314,6 +329,7 @@ add_wasilibc_test(utime.c FS)
 add_wasilibc_test(rewinddir.c FS)
 add_wasilibc_test(seekdir.c FS)
 add_wasilibc_test(usleep.c)
+add_wasilibc_test(write.c FS)
 
 if (TARGET_TRIPLE MATCHES "-threads")
   add_wasilibc_test(busywait.c)

test/src/write.c

@@ -0,0 +1,41 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/uio.h>
+#include "test.h"
+
+#define TEST(c) do { \
+	errno = 0; \
+	if (!(c)) \
+		t_error("%s failed (errno = %d)\n", #c, errno); \
+} while(0)
+
+#define N (100 << 20)
+
+int main(void) {
+  char tmp[] = "testsuite-XXXXXX";
+  int fd;
+  ssize_t r;
+
+  TEST((fd = open(tmp, O_RDWR | O_CREAT | O_EXCL, 0600)) > 2);
+
+  // write a big file of zeros, looping until it's fully written.
+  char *bytes = calloc(1, N);
+  TEST(bytes != NULL);
+  size_t written = 0;
+  while (written < N) {
+    TEST((r = write(fd, bytes + written, N - written)) > 0);
+    written += r;
+  }
+  TEST(close(fd) == 0);
+
+  struct stat buf;
+  TEST(stat(tmp, &buf) == 0);
+  TEST(buf.st_size == N);
+
+  return t_status;
+}