Call populate_environ only if we actually need environment variables. (#109)

* Link `populate_environ` only if we actually need environment variables.

This avoids linking in the environment variable initialization code,
and the __wasi_environ_sizes_get and __wasi_environ_get imports, in
programs that don't use environment variables.

This also removes the "___environ" (three underscores) alias symbol,
which is only in musl for backwards compatibility.

* Switch to //-style comments.

* If malloc fails, don't leave `__environ` pointing to an uninitialized buffer.

* Fix a memory leak if one malloc succeeds and the other fails.

* Use calloc to handle multiplication overflow.

This also handles the NULL terminator.

* Don't initialize __environ until everything has succeeded.

* Avoid leaking in case __wasi_environ_get fails.

* Handle overflow in the add too.

* Add #include <stdlib.h> for malloc etc.

* If the environment is empty, don't allocate any memory.

Author: sunfishcode

Makefile

@@ -120,7 +120,6 @@ LIBC_TOP_HALF_MUSL_SOURCES = \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/fcntl/creat.c \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/dirent/alphasort.c \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/dirent/versionsort.c \
-    $(LIBC_TOP_HALF_MUSL_SRC_DIR)/env/__environ.c \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/env/clearenv.c \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/env/getenv.c \
     $(LIBC_TOP_HALF_MUSL_SRC_DIR)/env/putenv.c \
@@ -359,7 +358,7 @@ $(OBJDIR)/%.o: $(CURDIR)/%.c include_dirs
 $(DLMALLOC_OBJS): override WASM_CFLAGS += \
     -I$(DLMALLOC_INC)
 
-$(LIBC_BOTTOM_HALF_ALL_OBJS): override WASM_CFLAGS += \
+startup_files $(LIBC_BOTTOM_HALF_ALL_OBJS): override WASM_CFLAGS += \
     -I$(LIBC_BOTTOM_HALF_HEADERS_PRIVATE) \
     -I$(LIBC_BOTTOM_HALF_CLOUDLIBC_SRC_INC) \
     -I$(LIBC_BOTTOM_HALF_CLOUDLIBC_SRC) \

expected/wasm32-wasi/defined-symbols.txt

@@ -11,7 +11,6 @@ _IO_putc
 _IO_putc_unlocked
 __EINVAL
 __ENOMEM
-___environ
 __asctime_r
 __assert_fail
 __c_dot_utf8
@@ -252,6 +251,7 @@ __utc
 __wasilibc_fd_renumber
 __wasilibc_find_relpath
 __wasilibc_init_preopen
+__wasilibc_populate_environ
 __wasilibc_register_preopened_fd
 __wasilibc_rmdirat
 __wasilibc_tell

libc-bottom-half/crt/crt1.c

@@ -2,8 +2,9 @@
 #include <sysexits.h>
 #include <wasi/core.h>
 #include <wasi/libc.h>
+#include <wasi/libc-internal.h>
 
-extern char **__environ;
+__wasi_errno_t __wasilibc_populate_environ(void) __attribute__((weak));
 extern void __wasm_call_ctors(void);
 extern int main(int, char *[]);
 extern void __prepare_for_exit(void);
@@ -37,32 +38,6 @@ static __wasi_errno_t populate_args(size_t *argc, char ***argv) {
     return __wasi_args_get(*argv, argv_buf);
 }
 
-static __wasi_errno_t populate_environ(void) {
-    __wasi_errno_t err;
-
-    /* Get the sizes of the arrays we'll have to create to copy in the environment. */
-    size_t environ_count;
-    size_t environ_buf_size;
-    err = __wasi_environ_sizes_get(&environ_count, &environ_buf_size);
-    if (err != __WASI_ESUCCESS) {
-        return err;
-    }
-
-    /* Allocate memory for the array of pointers, adding null terminator. */
-    __environ = malloc(sizeof(char *) * (environ_count + 1));
-    /* Allocate memory for storing the environment chars. */
-    char *environ_buf = malloc(sizeof(char) * environ_buf_size);
-    if (__environ == NULL || environ_buf == NULL) {
-        return __WASI_ENOMEM;
-    }
-
-    /* Make sure the last pointer in the array is NULL. */
-    __environ[environ_count] = NULL;
-
-    /* Fill the environment chars, and the __environ array with pointers into those chars. */
-    return __wasi_environ_get(__environ, environ_buf);
-}
-
 static __wasi_errno_t populate_libpreopen(void) {
     __wasilibc_init_preopen();
 
@@ -110,9 +85,11 @@ void _start(void) {
         _Exit(EX_OSERR);
     }
 
-    /* Fill in the environment from WASI syscalls. */
-    if (populate_environ() != __WASI_ESUCCESS) {
-        _Exit(EX_OSERR);
+    /* Fill in the environment from WASI syscalls, if needed. */
+    if (&__wasilibc_populate_environ != NULL) {
+        if (__wasilibc_populate_environ() != __WASI_ESUCCESS) {
+            _Exit(EX_OSERR);
+        }
     }
 
     /* Fill in the arguments from WASI syscalls. */

libc-bottom-half/headers/private/wasi/libc-internal.h

@@ -0,0 +1,17 @@
+#ifndef __wasi_libc_internal_h
+#define __wasi_libc_internal_h
+
+#include <wasi/core.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+void __wasilibc_init_preopen(void);
+__wasi_errno_t __wasilibc_populate_environ(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

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

@@ -7,7 +7,6 @@
 extern "C" {
 #endif
 
-void __wasilibc_init_preopen(void);
 int __wasilibc_register_preopened_fd(int fd, const char *path);
 int __wasilibc_fd_renumber(int fd, int newfd);
 int __wasilibc_unlinkat(int fd, const char *path);

libc-bottom-half/sources/__environ.c

@@ -0,0 +1,57 @@
+#include <unistd.h>
+#include <stdlib.h>
+#include <wasi/core.h>
+#include <wasi/libc.h>
+#include <wasi/libc-internal.h>
+
+char **__environ = NULL;
+extern __typeof(__environ) _environ __attribute__((weak, alias("__environ")));
+extern __typeof(__environ) environ __attribute__((weak, alias("__environ")));
+
+// This function is referenced by a weak symbol in crt1.c, and we define
+// it here in the same source file as __environ, so that this function is
+// linked in iff environment variable support is used.
+__wasi_errno_t __wasilibc_populate_environ(void) {
+    __wasi_errno_t err;
+
+    // Get the sizes of the arrays we'll have to create to copy in the environment.
+    size_t environ_count;
+    size_t environ_buf_size;
+    err = __wasi_environ_sizes_get(&environ_count, &environ_buf_size);
+    if (err != __WASI_ESUCCESS) {
+        return err;
+    }
+    if (environ_count == 0) {
+        return __WASI_ESUCCESS;
+    }
+
+    // Add 1 for the NULL pointer to mark the end, and check for overflow.
+    size_t num_ptrs = environ_count + 1;
+    if (num_ptrs == 0) {
+        return __WASI_ENOMEM;
+    }
+
+    // Allocate memory for storing the environment chars.
+    char *environ_buf = malloc(environ_buf_size);
+    if (environ_buf == NULL) {
+        return __WASI_ENOMEM;
+    }
+
+    // Allocate memory for the array of pointers. This uses `calloc` both to
+    // handle overflow and to initialize the NULL pointer at the end.
+    char **environ_ptrs = calloc(num_ptrs, sizeof(char *));
+    if (environ_ptrs == NULL) {
+        free(environ_buf);
+        return __WASI_ENOMEM;
+    }
+
+    // Fill the environment chars, and the __environ array with pointers into those chars.
+    err = __wasi_environ_get(environ_ptrs, environ_buf);
+    if (err == __WASI_ESUCCESS) {
+        __environ = environ_ptrs;
+    } else {
+        free(environ_buf);
+        free(environ_ptrs);
+    }
+    return err;
+}