Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy-initialize the environment variables. #184

Merged
merged 2 commits into from
Mar 19, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion expected/wasm32-wasi/defined-symbols.txt
Original file line number Diff line number Diff line change
@@ -39,7 +39,6 @@ __env_rm_add
__env_rm_add
__env_rm_add
__env_rm_add
__environ
__exp2f_data
__exp_data
__expo2
@@ -252,8 +251,12 @@ __uflow
__unlist_locked_file
__uselocale
__utc
__wasilibc_ensure_environ
__wasilibc_environ
__wasilibc_environ
__wasilibc_fd_renumber
__wasilibc_find_relpath
__wasilibc_initialize_environ
__wasilibc_open_nomode
__wasilibc_openat_nomode
__wasilibc_register_preopened_fd
1 change: 1 addition & 0 deletions expected/wasm32-wasi/include-all.c
Original file line number Diff line number Diff line change
@@ -166,6 +166,7 @@
#include <unistd.h>
#include <values.h>
#include <wasi/api.h>
#include <wasi/libc-environ.h>
#include <wasi/libc-find-relpath.h>
#include <wasi/libc.h>
#include <wchar.h>
1 change: 1 addition & 0 deletions expected/wasm32-wasi/predefined-macros.txt
Original file line number Diff line number Diff line change
@@ -3098,6 +3098,7 @@
#define __va_copy(d,s) __builtin_va_copy(d,s)
#define __wasi__ 1
#define __wasi_api_h
#define __wasi_libc_environ_h
#define __wasi_libc_find_relpath_h
#define __wasi_libc_h
#define __wasilibc___errno_values_h
19 changes: 19 additions & 0 deletions libc-bottom-half/headers/public/wasi/libc-environ.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#ifndef __wasi_libc_environ_h
#define __wasi_libc_environ_h

#ifdef __cplusplus
extern "C" {
#endif

/// Initialize the global environment variable state. Only needs to be
/// called once; most users should call `__wasilibc_ensure_environ` instead.
void __wasilibc_initialize_environ(void);

/// If `__wasilibc_initialize_environ` has not yet been called, call it.
void __wasilibc_ensure_environ(void);

#ifdef __cplusplus
}
#endif

#endif
2 changes: 1 addition & 1 deletion libc-bottom-half/libpreopen/libpreopen.c
Original file line number Diff line number Diff line change
@@ -524,7 +524,7 @@ __wasilibc_find_relpath(
/// 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.
// libc-bottom-half/sources/__wasilibc_environ.c.
__attribute__((constructor(51)))
static void
__wasilibc_populate_libpreopen(void)
Original file line number Diff line number Diff line change
@@ -3,29 +3,38 @@
#include <sysexits.h>
#include <wasi/api.h>
#include <wasi/libc.h>
#include <wasi/libc-environ.h>

static char *empty_environ[1] = { NULL };
char **__environ = empty_environ;
extern __typeof(__environ) _environ __attribute__((weak, alias("__environ")));
extern __typeof(__environ) environ __attribute__((weak, alias("__environ")));
/// If the program doesn't use `environ`, it'll get this version of
/// `__wasilibc_environ`, which isn't initialized with a constructor function.
/// `getenv` etc. call `__wasilibc_ensure_environ()` before accessing it.
/// Statically-initialize it to an invalid pointer value so that we can
/// detect if it's been explicitly initialized (we can't use `NULL` because
/// `clearenv` sets it to NULL.
char **__wasilibc_environ __attribute__((weak)) = (char **)-1;

// We define this function here in the same source file as __environ, so that
// this function is called in iff environment variable support is used.
// Concerning the 50 -- levels up to 100 are reserved for the implementation,
// so we an arbitrary number in the middle of the range to allow other
// reserved things to go before or after.
__attribute__((constructor(50)))
static void __wasilibc_populate_environ(void) {
__wasi_errno_t err;
// See the comments in libc-environ.h.
void __wasilibc_ensure_environ(void) {
if (__wasilibc_environ == (char **)-1) {
__wasilibc_initialize_environ();
}
}

/// Avoid dynamic allocation for the case where there are no environment
/// variables, but we still need a non-NULL pointer to an (empty) array.
static char *empty_environ[1] = { NULL };

// See the comments in libc-environ.h.
void __wasilibc_initialize_environ(void) {
// 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);
__wasi_errno_t err = __wasi_environ_sizes_get(&environ_count, &environ_buf_size);
if (err != __WASI_ERRNO_SUCCESS) {
goto oserr;
}
if (environ_count == 0) {
__wasilibc_environ = empty_environ;
return;
}

@@ -49,7 +58,8 @@ static void __wasilibc_populate_environ(void) {
goto software;
}

// Fill the environment chars, and the __environ array with pointers into those chars.
// Fill the environment chars, and the `__wasilibc_environ` array with
// pointers into those chars.
// TODO: Remove the casts on `environ_ptrs` and `environ_buf` once the witx is updated with char8 support.
err = __wasi_environ_get((uint8_t **)environ_ptrs, (uint8_t *)environ_buf);
if (err != __WASI_ERRNO_SUCCESS) {
@@ -58,7 +68,7 @@ static void __wasilibc_populate_environ(void) {
goto oserr;
}

__environ = environ_ptrs;
__wasilibc_environ = environ_ptrs;
return;
oserr:
_Exit(EX_OSERR);
26 changes: 26 additions & 0 deletions libc-bottom-half/sources/environ.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include <unistd.h>
#include <stdlib.h>
#include <sysexits.h>
#include <wasi/api.h>
#include <wasi/libc.h>
#include <wasi/libc-environ.h>

// If the program does use `environ`, it'll get this version of
// `__wasilibc_environ`, which is initialized with a constructor function, so
// that it's initialized whenever user code might want to access it.
char **__wasilibc_environ;
extern __typeof(__wasilibc_environ) _environ
__attribute__((weak, alias("__wasilibc_environ")));
extern __typeof(__wasilibc_environ) environ
__attribute__((weak, alias("__wasilibc_environ")));

// We define this function here in the same source file as
// `__wasilibc_environ`, so that this function is called in iff environment
// variable support is used.
// Concerning the 50 -- levels up to 100 are reserved for the implementation,
// so we an arbitrary number in the middle of the range to allow other
// reserved things to go before or after.
__attribute__((constructor(50)))
static void __wasilibc_initialize_environ_eagerly(void) {
__wasilibc_initialize_environ();
}
12 changes: 12 additions & 0 deletions libc-top-half/headers/private/wasi/libc-environ-compat.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// This header file is meant to be included withinin the body of a function
// which uses `__environ`. Code using `__environ` expects it will be initialized
// eagerly. `__wasilibc_environ` is initialized lazily. Provide `__environ` as
// an alias and arrange for the lazy initialization to be performed.

extern char **__wasilibc_environ;

__wasilibc_ensure_environ();

#ifndef __wasilibc_environ
#define __environ __wasilibc_environ
#endif
7 changes: 7 additions & 0 deletions libc-top-half/musl/src/env/clearenv.c
Original file line number Diff line number Diff line change
@@ -7,6 +7,13 @@ weak_alias(dummy, __env_rm_add);

int clearenv()
{
#ifdef __wasilibc_unmodified_upstream // Lazy environment variable init.
#else
// This specialized header is included within the function body to arranges for
// the environment variables to be lazily initialized. It redefined `__environ`,
// so don't remove or reorder it with respect to other code.
#include "wasi/libc-environ-compat.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment here (& in the other uses) indicating that this header is intended to be used inside a func body, and it redefines the __environ symbol. The latter seems like it may cause unintended consequences if someone performed otherwise innocent-looking code motion without understanding the details of that header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added. The most common cases of code motion are guarded against, because if you use __environ before it's defined, you'll get errors, and if you use the redefined __environ in a function other than the one the header was included in, __wasilibc_environ won't be declared and you'll get errors. But I agree, this is particularly tricky code, so comments are good.

#endif
char **e = __environ;
__environ = 0;
if (e) while (*e) __env_rm_add(*e++, 0);
7 changes: 7 additions & 0 deletions libc-top-half/musl/src/env/getenv.c
Original file line number Diff line number Diff line change
@@ -4,6 +4,13 @@

char *getenv(const char *name)
{
#ifdef __wasilibc_unmodified_upstream // Lazy environment variable init.
#else
// This specialized header is included within the function body to arranges for
// the environment variables to be lazily initialized. It redefined `__environ`,
// so don't remove or reorder it with respect to other code.
#include "wasi/libc-environ-compat.h"
#endif
size_t l = __strchrnul(name, '=') - name;
if (l && !name[l] && __environ)
for (char **e = __environ; *e; e++)
7 changes: 7 additions & 0 deletions libc-top-half/musl/src/env/putenv.c
Original file line number Diff line number Diff line change
@@ -7,6 +7,13 @@ weak_alias(dummy, __env_rm_add);

int __putenv(char *s, size_t l, char *r)
{
#ifdef __wasilibc_unmodified_upstream // Lazy environment variable init.
#else
// This specialized header is included within the function body to arranges for
// the environment variables to be lazily initialized. It redefined `__environ`,
// so don't remove or reorder it with respect to other code.
#include "wasi/libc-environ-compat.h"
#endif
size_t i=0;
if (__environ) {
for (char **e = __environ; *e; e++, i++)
7 changes: 7 additions & 0 deletions libc-top-half/musl/src/env/unsetenv.c
Original file line number Diff line number Diff line change
@@ -13,6 +13,13 @@ int unsetenv(const char *name)
errno = EINVAL;
return -1;
}
#ifdef __wasilibc_unmodified_upstream // Lazy environment variable init.
#else
// This specialized header is included within the function body to arranges for
// the environment variables to be lazily initialized. It redefined `__environ`,
// so don't remove or reorder it with respect to other code.
#include "wasi/libc-environ-compat.h"
#endif
if (__environ) {
char **e = __environ, **eo = e;
for (; *e; e++)
7 changes: 7 additions & 0 deletions libc-top-half/musl/src/include/unistd.h
Original file line number Diff line number Diff line change
@@ -3,7 +3,14 @@

#include "../../include/unistd.h"

#ifdef __wasilibc_unmodified_upstream // Lazy environment variable init.
extern char **__environ;
#else
// To support lazy initialization of environment variables, `__environ` is
// omitted, and a lazy `__wasilibc_environ` is used instead. Use
// "wasi/libc-environ-compat.h" in functions that use `__environ`.
#include "wasi/libc-environ.h"
#endif

hidden int __dup3(int, int, int);
hidden int __mkostemps(char *, int, int);