Convert preopen initialization to be lazy. (#408)

* Convert preopen initialization to be lazy.

Insteead of eagerly initializing the preopens in a static constructor,
perform preopen initialization the first time it's needed, or before a
close or a renumber which might disrupt the file descriptor space.

And, use a weak symbol with a stub function for use by `close` or
`fd_renumber`, we that they can trigger preopen initialization only
if it's actually needed.

This way, if a program doesn't contain any calls to any function that
needs preopens, it can avoid linking in the preopen initialization code.
And if it contains calls but doesn't execute them at runtime, it can
avoid executing the preopen intiailization code.

A downside here is that this may cause problems for users that call
`__wasi_fd_close` or `__wasi_fd_renumber` directly and close over
overwrite some preopens before libc has a chance to scan them. To
partially address this, this PR does add a declaration for
`__wasilibc_populate_preopens` to <wasi/libc.h> so that users can call
it manually if they need to.

* Fix calling `internal_register_preopened_fd` with the lock held.

Factor out the lock acquisition from the implementation of
`internal_register_preopened_fd` so that we can call it from
`__wasilibc_populate_preopens` with the lock held.

Author: sunfishcode

expected/wasm32-wasi-threads/defined-symbols.txt

@@ -390,6 +390,7 @@ __wasilibc_nocwd_scandirat
 __wasilibc_nocwd_symlinkat
 __wasilibc_nocwd_utimensat
 __wasilibc_open_nomode
+__wasilibc_populate_preopens
 __wasilibc_pthread_self
 __wasilibc_register_preopened_fd
 __wasilibc_rename_newat

expected/wasm32-wasi/defined-symbols.txt

@@ -329,6 +329,7 @@ __wasilibc_nocwd_scandirat
 __wasilibc_nocwd_symlinkat
 __wasilibc_nocwd_utimensat
 __wasilibc_open_nomode
+__wasilibc_populate_preopens
 __wasilibc_register_preopened_fd
 __wasilibc_rename_newat
 __wasilibc_rename_oldat

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

@@ -11,6 +11,12 @@ extern "C" {
 struct stat;
 struct timespec;
 
+/// Populate libc's preopen tables. This is normally done automatically
+/// just before it's needed, however if you call `__wasi_fd_renumber` or
+/// `__wasi_fd_close` directly, and you need the preopens to be accurate
+/// afterward, you should call this before doing so.
+void __wasilibc_populate_preopens(void);
+
 /// Register the given pre-opened file descriptor under the given path.
 ///
 /// This function does not take ownership of `prefix` (it makes its own copy).

libc-bottom-half/headers/public/wasi/libc.h

@@ -4,10 +4,32 @@
 #include <unistd.h>
 
 int __wasilibc_fd_renumber(int fd, int newfd) {
+    // Scan the preopen fds before making any changes.
+    __wasilibc_populate_preopens();
+
     __wasi_errno_t error = __wasi_fd_renumber(fd, newfd);
     if (error != 0) {
         errno = error;
         return -1;
     }
     return 0;
 }
+
+int close(int fd) {
+    // Scan the preopen fds before making any changes.
+    __wasilibc_populate_preopens();
+
+    __wasi_errno_t error = __wasi_fd_close(fd);
+    if (error != 0) {
+        errno = error;
+        return -1;
+    }
+
+    return 0;
+}
+
+weak void __wasilibc_populate_preopens(void) {
+    // This version does nothing. It may be overridden by a version which does
+    // something if `__wasilibc_find_abspath` or `__wasilibc_find_relpath` are
+    // used.
+}

libc-bottom-half/sources/__wasilibc_fd_renumber.c

@@ -25,6 +25,7 @@ typedef struct preopen {
 } preopen;
 
 /// A simple growable array of `preopen`.
+static _Atomic _Bool preopens_populated = false;
 static preopen *preopens;
 static size_t num_preopens;
 static size_t preopen_capacity;
@@ -100,35 +101,42 @@ static const char *strip_prefixes(const char *path) {
     return path;
 }
 
-/// Register the given preopened file descriptor under the given path.
-///
-/// This function takes ownership of `prefix`.
-static int internal_register_preopened_fd(__wasi_fd_t fd, const char *relprefix) {
-    LOCK(lock);
-
+/// Similar to `internal_register_preopened_fd_unlocked` but does not
+/// take a lock.
+static int internal_register_preopened_fd_unlocked(__wasi_fd_t fd, const char *relprefix) {
     // Check preconditions.
     assert_invariants();
     assert(fd != AT_FDCWD);
     assert(fd != -1);
     assert(relprefix != NULL);
 
     if (num_preopens == preopen_capacity && resize() != 0) {
-        UNLOCK(lock);
         return -1;
     }
 
     char *prefix = strdup(strip_prefixes(relprefix));
     if (prefix == NULL) {
-        UNLOCK(lock);
         return -1;
     }
     preopens[num_preopens++] = (preopen) { prefix, fd, };
 
     assert_invariants();
-    UNLOCK(lock);
     return 0;
 }
 
+/// Register the given preopened file descriptor under the given path.
+///
+/// This function takes ownership of `prefix`.
+static int internal_register_preopened_fd(__wasi_fd_t fd, const char *relprefix) {
+    LOCK(lock);
+
+    int r = internal_register_preopened_fd_unlocked(fd, relprefix);
+
+    UNLOCK(lock);
+
+    return r;
+}
+
 /// Are the `prefix_len` bytes pointed to by `prefix` a prefix of `path`?
 static bool prefix_matches(const char *prefix, size_t prefix_len, const char *path) {
     // Allow an empty string as a prefix of any relative path.
@@ -152,6 +160,8 @@ static bool prefix_matches(const char *prefix, size_t prefix_len, const char *pa
 
 // See the documentation in libc.h
 int __wasilibc_register_preopened_fd(int fd, const char *prefix) {
+    __wasilibc_populate_preopens();
+
     return internal_register_preopened_fd((__wasi_fd_t)fd, prefix);
 }
 
@@ -172,6 +182,8 @@ int __wasilibc_find_relpath(const char *path,
 int __wasilibc_find_abspath(const char *path,
                             const char **abs_prefix,
                             const char **relative_path) {
+    __wasilibc_populate_preopens();
+
     // Strip leading `/` characters, the prefixes we're mataching won't have
     // them.
     while (*path == '/')
@@ -219,13 +231,20 @@ int __wasilibc_find_abspath(const char *path,
     return fd;
 }
 
-/// This is referenced by weak reference from crt1.c and lives in the same
-/// source file as `__wasilibc_find_relpath` so that it's linked in when it's
-/// needed.
-// Concerning the 51 -- see the comment by the constructor priority in
-// libc-bottom-half/sources/environ.c.
-__attribute__((constructor(51)))
-static void __wasilibc_populate_preopens(void) {
+void __wasilibc_populate_preopens(void) {
+    // Fast path: If the preopens are already initialized, do nothing.
+    if (preopens_populated) {
+        return;
+    }
+
+    LOCK(lock);
+
+    // Check whether another thread initialized the preopens already.
+    if (preopens_populated) {
+        UNLOCK(lock);
+        return;
+    }
+
     // Skip stdin, stdout, and stderr, and count up until we reach an invalid
     // file descriptor.
     for (__wasi_fd_t fd = 3; fd != 0; ++fd) {
@@ -249,7 +268,7 @@ static void __wasilibc_populate_preopens(void) {
                 goto oserr;
             prefix[prestat.u.dir.pr_name_len] = '\0';
 
-            if (internal_register_preopened_fd(fd, prefix) != 0)
+            if (internal_register_preopened_fd_unlocked(fd, prefix) != 0)
                 goto software;
             free(prefix);
 
@@ -260,6 +279,11 @@ static void __wasilibc_populate_preopens(void) {
         }
     }
 
+    // Preopens are now initialized.
+    preopens_populated = true;
+
+    UNLOCK(lock);
+
     return;
 oserr:
     _Exit(EX_OSERR);