Merge #7920 'env: use libuv v1.12 getenv/setenv API'

21 files changed, 370 insertions, 118 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 988c69cd2d..848e100b02 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -379,7 +379,7 @@ endif()
 include_directories("${PROJECT_BINARY_DIR}/config")
 include_directories("${PROJECT_SOURCE_DIR}/src")
 
-find_package(LibUV REQUIRED)
+find_package(LibUV REQUIRED)  # minimum version: v1.12
 include_directories(SYSTEM ${LIBUV_INCLUDE_DIRS})
 
 find_package(Msgpack 1.0.0 REQUIRED)
diff --git a/config/CMakeLists.txt b/config/CMakeLists.txt
index 442d91524b..91c1e7c629 100644
--- a/config/CMakeLists.txt
+++ b/config/CMakeLists.txt
@@ -47,17 +47,8 @@ if(Iconv_FOUND)
   set(HAVE_ICONV 1)
 endif()
 
-check_function_exists(_putenv_s HAVE_PUTENV_S)
-if(WIN32 AND NOT HAVE_PUTENV_S)
-  message(SEND_ERROR "_putenv_s() function not found on your system.")
-endif()
 check_function_exists(opendir HAVE_OPENDIR)
 check_function_exists(readlink HAVE_READLINK)
-check_function_exists(setenv HAVE_SETENV)
-if(UNIX AND NOT HAVE_SETENV)
-  message(SEND_ERROR "setenv() function not found on your system.")
-endif()
-check_function_exists(unsetenv HAVE_UNSETENV)
 check_function_exists(setpgid HAVE_SETPGID)
 check_function_exists(setsid HAVE_SETSID)
 check_function_exists(sigaction HAVE_SIGACTION)
diff --git a/config/config.h.in b/config/config.h.in
index 56d46e9f14..ad636632f4 100644
--- a/config/config.h.in
+++ b/config/config.h.in
@@ -27,14 +27,11 @@
 #cmakedefine HAVE_LOCALE_H
 #cmakedefine HAVE_NL_LANGINFO_CODESET
 #cmakedefine HAVE_NL_MSG_CAT_CNTR
-#cmakedefine HAVE_PUTENV_S
 #cmakedefine HAVE_PWD_H
 #cmakedefine HAVE_READLINK
 #cmakedefine HAVE_UV_TRANSLATE_SYS_ERROR
 // TODO: add proper cmake check
 // #define HAVE_SELINUX 1
-#cmakedefine HAVE_SETENV
-#cmakedefine HAVE_UNSETENV
 #cmakedefine HAVE_SETPGID
 #cmakedefine HAVE_SETSID
 #cmakedefine HAVE_SIGACTION
diff --git a/src/clint.py b/src/clint.py
index 34af5d15fd..1ef31820ee 100755
--- a/src/clint.py
+++ b/src/clint.py
@@ -29,10 +29,10 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-"""Does neovim-lint on c files.
+"""Lints C files in the Neovim source tree.
 
 The goal of this script is to identify places in the code that *may*
-be in non-compliance with neovim style.  It does not attempt to fix
+be in non-compliance with Neovim style.  It does not attempt to fix
 up these problems -- the point is to educate.  It does also not
 attempt to find all problems, or to ensure that everything it does
 find is legitimately a problem.
@@ -88,7 +88,7 @@ Syntax: clint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...]
    * [whitespace/braces] { should almost always be at the end of the previous
      line
    * [build/include] Include the directory when naming .h files
-   * [runtime/int] Use int16/int64/etc, rather than the C type.
+   * [runtime/int] Use int16_t/int64_t/etc, rather than the C type.
 
   Every problem is given a confidence score from 1-5, with 5 meaning we are
   certain of the problem, and 1 meaning it could be a legitimate construct.
@@ -1487,6 +1487,37 @@ def CheckMemoryFunctions(filename, clean_lines, linenum, error):
                   '...) instead of ' + function + '...).')
 
 
+os_functions = (
+    ('setenv(', 'os_setenv('),
+    ('getenv(', 'os_getenv('),
+    ('_wputenv(', 'os_setenv('),
+    ('_putenv_s(', 'os_setenv('),
+    ('putenv(', 'os_setenv('),
+    ('unsetenv(', 'os_unsetenv('),
+)
+
+
+def CheckOSFunctions(filename, clean_lines, linenum, error):
+    """Checks for calls to invalid functions.
+
+    Args:
+      filename: The name of the current file.
+      clean_lines: A CleansedLines instance containing the file.
+      linenum: The number of the line to check.
+      error: The function to call with any errors found.
+    """
+    line = clean_lines.elided[linenum]
+    for function, suggested_function in os_functions:
+        ix = line.find(function)
+        # Comparisons made explicit for clarity -- pylint:
+        # disable=g-explicit-bool-comparison
+        if ix >= 0 and (ix == 0 or (not line[ix - 1].isalnum() and
+                                    line[ix - 1] not in ('_', '.', '>'))):
+            error(filename, linenum, 'runtime/os_fn', 2,
+                  'Use ' + suggested_function +
+                  '...) instead of ' + function + '...).')
+
+
 # Matches invalid increment: *count++, which moves pointer instead of
 # incrementing a value.
 _RE_PATTERN_INVALID_INCREMENT = re.compile(
@@ -3370,6 +3401,7 @@ def ProcessLine(filename, file_extension, clean_lines, line,
                                   nesting_state, error)
     CheckPosixThreading(filename, clean_lines, line, error)
     CheckMemoryFunctions(filename, clean_lines, line, error)
+    CheckOSFunctions(filename, clean_lines, line, error)
     for check_fn in extra_check_functions:
         check_fn(filename, clean_lines, line, error)
 
diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h
index 973b97d476..a70a8bea63 100644
--- a/src/nvim/hashtab.h
+++ b/src/nvim/hashtab.h
@@ -19,7 +19,7 @@ typedef size_t hash_T;
 #define HASHITEM_EMPTY(hi) ((hi)->hi_key == NULL \
                             || (hi)->hi_key == (char_u *)&hash_removed)
 
-/// A hastable item.
+/// Hashtable item.
 ///
 /// Each item has a NUL terminated string key.
 /// A key can appear only once in the table.
diff --git a/src/nvim/lib/khash.h b/src/nvim/lib/khash.h
index 8287cb14da..b2994a3159 100644
--- a/src/nvim/lib/khash.h
+++ b/src/nvim/lib/khash.h
@@ -24,24 +24,24 @@
 */
 
 /*
-  An example:
+  Example:
 
 #include "nvim/khash.h"
 KHASH_MAP_INIT_INT(32, char)
 int main() {
-	int ret, is_missing;
-	khiter_t k;
-	khash_t(32) *h = kh_init(32);
-	k = kh_put(32, h, 5, &ret);
-	kh_value(h, k) = 10;
-	k = kh_get(32, h, 10);
-	is_missing = (k == kh_end(h));
-	k = kh_get(32, h, 5);
-	kh_del(32, h, k);
-	for (k = kh_begin(h); k != kh_end(h); ++k)
-		if (kh_exist(h, k)) kh_value(h, k) = 1;
-	kh_destroy(32, h);
-	return 0;
+    int ret, is_missing;
+    khiter_t k;
+    khash_t(32) *h = kh_init(32);
+    k = kh_put(32, h, 5, &ret);
+    kh_value(h, k) = 10;
+    k = kh_get(32, h, 10);
+    is_missing = (k == kh_end(h));
+    k = kh_get(32, h, 5);
+    kh_del(32, h, k);
+    for (k = kh_begin(h); k != kh_end(h); ++k)
+        if (kh_exist(h, k)) kh_value(h, k) = 1;
+    kh_destroy(32, h);
+    return 0;
 }
 */
 
@@ -539,7 +539,7 @@ static kh_inline khint_t __ac_Wang_hash(khint_t key)
   @param  r     Extra return code: -1 if the operation failed;
                 0 if the key is present in the hash table;
                 1 if the bucket is empty (never used); 2 if the element in
-				the bucket has been deleted [int*]
+                the bucket has been deleted [int*]
   @return       Iterator to the inserted element [khint_t]
  */
 #define kh_put(name, h, k, r) kh_put_##name(h, k, r)
diff --git a/src/nvim/main.c b/src/nvim/main.c
index 251a54ad5b..79513b1c21 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -184,6 +184,7 @@ bool event_teardown(void)
 void early_init(void)
 {
   log_init();
+  env_init();
   fs_init();
   handle_init();
   eval_init();          // init global variables
@@ -1769,7 +1770,7 @@ static bool do_user_initialization(void)
   FUNC_ATTR_WARN_UNUSED_RESULT
 {
   bool do_exrc = p_exrc;
-  if (process_env("VIMINIT") == OK) {
+  if (execute_env("VIMINIT") == OK) {
     do_exrc = p_exrc;
     return do_exrc;
   }
@@ -1814,7 +1815,7 @@ static bool do_user_initialization(void)
     } while (iter != NULL);
     xfree(config_dirs);
   }
-  if (process_env("EXINIT") == OK) {
+  if (execute_env("EXINIT") == OK) {
     do_exrc = p_exrc;
     return do_exrc;
   }
@@ -1878,7 +1879,7 @@ static void source_startup_scripts(const mparm_T *const parmp)
 ///
 /// @return FAIL if the environment variable was not executed,
 ///         OK otherwise.
-static int process_env(char *env)
+static int execute_env(char *env)
   FUNC_ATTR_NONNULL_ALL
 {
   const char *initstr = os_getenv(env);
diff --git a/src/nvim/map.c b/src/nvim/map.c
index 53ab734802..9b6f57a56f 100644
--- a/src/nvim/map.c
+++ b/src/nvim/map.c
@@ -1,6 +1,13 @@
 // This is an open source non-commercial project. Dear PVS-Studio, please check
 // it. PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
 
+///
+/// map.c: khash.h wrapper
+///
+/// NOTE: Callers must manage memory (allocate) for keys and values.
+///       khash.h does not make its own copy of the key or value.
+///
+
 #include <stdlib.h>
 #include <stdbool.h>
 #include <string.h>
@@ -72,6 +79,16 @@
     return kh_get(T##_##U##_map, map->table, key) != kh_end(map->table); \
   } \
   \
+  T map_##T##_##U##_key(Map(T, U) *map, T key) \
+  { \
+    khiter_t k; \
+    \
+    if ((k = kh_get(T##_##U##_map, map->table, key)) == kh_end(map->table)) { \
+      abort();  /* Caller must check map_has(). */ \
+    } \
+    \
+    return kh_key(map->table, k); \
+  } \
   U map_##T##_##U##_put(Map(T, U) *map, T key, U value) \
   { \
     int ret; \
@@ -167,3 +184,18 @@ MAP_IMPL(String, MsgpackRpcRequestHandler, MSGPACK_HANDLER_INITIALIZER)
 #define KVEC_INITIALIZER { .size = 0, .capacity = 0, .items = NULL }
 MAP_IMPL(HlEntry, int, DEFAULT_INITIALIZER)
 MAP_IMPL(String, handle_T, 0)
+
+
+/// Deletes a key:value pair from a string:pointer map, and frees the
+/// storage of both key and value.
+///
+void pmap_del2(PMap(cstr_t) *map, const char *key)
+{
+  if (pmap_has(cstr_t)(map, key)) {
+    void *k = (void *)pmap_key(cstr_t)(map, key);
+    void *v = pmap_get(cstr_t)(map, key);
+    pmap_del(cstr_t)(map, key);
+    xfree(k);
+    xfree(v);
+  }
+}
diff --git a/src/nvim/map.h b/src/nvim/map.h
index 0e4308b953..75ab64cca4 100644
--- a/src/nvim/map.h
+++ b/src/nvim/map.h
@@ -25,11 +25,15 @@
   void map_##T##_##U##_free(Map(T, U) *map); \
   U map_##T##_##U##_get(Map(T, U) *map, T key); \
   bool map_##T##_##U##_has(Map(T, U) *map, T key); \
+  T map_##T##_##U##_key(Map(T, U) *map, T key); \
   U map_##T##_##U##_put(Map(T, U) *map, T key, U value); \
   U *map_##T##_##U##_ref(Map(T, U) *map, T key, bool put); \
   U map_##T##_##U##_del(Map(T, U) *map, T key); \
   void map_##T##_##U##_clear(Map(T, U) *map);
 
+//
+// NOTE: Keys AND values must be allocated! khash.h does not make a copy.
+//
 MAP_DECLS(int, int)
 MAP_DECLS(cstr_t, ptr_t)
 MAP_DECLS(ptr_t, ptr_t)
@@ -43,6 +47,7 @@ MAP_DECLS(String, handle_T)
 #define map_free(T, U) map_##T##_##U##_free
 #define map_get(T, U) map_##T##_##U##_get
 #define map_has(T, U) map_##T##_##U##_has
+#define map_key(T, U) map_##T##_##U##_key
 #define map_put(T, U) map_##T##_##U##_put
 #define map_ref(T, U) map_##T##_##U##_ref
 #define map_del(T, U) map_##T##_##U##_del
@@ -52,7 +57,9 @@ MAP_DECLS(String, handle_T)
 #define pmap_free(T) map_free(T, ptr_t)
 #define pmap_get(T) map_get(T, ptr_t)
 #define pmap_has(T) map_has(T, ptr_t)
+#define pmap_key(T) map_key(T, ptr_t)
 #define pmap_put(T) map_put(T, ptr_t)
+/// @see pmap_del2
 #define pmap_del(T) map_del(T, ptr_t)
 #define pmap_clear(T) map_clear(T, ptr_t)
 
@@ -62,4 +69,6 @@ MAP_DECLS(String, handle_T)
 #define map_foreach_value(map, value, block) \
   kh_foreach_value(map->table, value, block)
 
+void pmap_del2(PMap(cstr_t) *map, const char *key);
+
 #endif  // NVIM_MAP_H
diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c
index ead6b4405d..1cf045d7e0 100644
--- a/src/nvim/mbyte.c
+++ b/src/nvim/mbyte.c
@@ -1375,6 +1375,7 @@ int utf8_to_utf16(const char *str, wchar_t **strw)
 int utf16_to_utf8(const wchar_t *strw, char **str)
   FUNC_ATTR_NONNULL_ALL
 {
+  *str = NULL;
   // Compute the space required to store the string as UTF-8.
   DWORD utf8_len = WideCharToMultiByte(CP_UTF8,
                                        0,
@@ -1400,7 +1401,7 @@ int utf16_to_utf8(const wchar_t *strw, char **str)
                                  NULL,
                                  NULL);
   if (utf8_len == 0) {
-    free(*str);
+    xfree(*str);
     *str = NULL;
     return GetLastError();
   }
diff --git a/src/nvim/memory.c b/src/nvim/memory.c
index b49b521bc9..0f402611df 100644
--- a/src/nvim/memory.c
+++ b/src/nvim/memory.c
@@ -109,7 +109,7 @@ void *xmalloc(size_t size)
   return ret;
 }
 
-/// free wrapper that returns delegates to the backing memory manager
+/// free() wrapper that delegates to the backing memory manager
 void xfree(void *ptr)
 {
   free(ptr);
diff --git a/src/nvim/os/env.c b/src/nvim/os/env.c
index c6794e4be5..8a8220b6e7 100644
--- a/src/nvim/os/env.c
+++ b/src/nvim/os/env.c
@@ -19,6 +19,7 @@
 #include "nvim/eval.h"
 #include "nvim/ex_getln.h"
 #include "nvim/version.h"
+#include "nvim/map.h"
 
 #ifdef WIN32
 #include "nvim/mbyte.h"  // for utf8_to_utf16, utf16_to_utf8
@@ -32,65 +33,119 @@
 #include <sys/utsname.h>
 #endif
 
+// Because `uv_os_getenv` requires allocating, we must manage a map to maintain
+// the behavior of `os_getenv`.
+static PMap(cstr_t) *envmap;
+static uv_mutex_t mutex;
+
+void env_init(void)
+{
+  envmap = pmap_new(cstr_t)();
+  uv_mutex_init(&mutex);
+}
+
 /// Like getenv(), but returns NULL if the variable is empty.
 const char *os_getenv(const char *name)
   FUNC_ATTR_NONNULL_ALL
 {
-  const char *e = getenv(name);
-  return e == NULL || *e == NUL ? NULL : e;
+  char *e;
+  size_t size = 64;
+  if (name[0] == '\0') {
+    return NULL;
+  }
+  uv_mutex_lock(&mutex);
+  if (pmap_has(cstr_t)(envmap, name)
+      && !!(e = (char *)pmap_get(cstr_t)(envmap, name))) {
+    if (e[0] != '\0') {
+      // Found non-empty cached env var.
+      // NOTE: This risks incoherence if an in-process library changes the
+      //       environment without going through our os_setenv() wrapper.  If
+      //       that turns out to be a problem, we can just remove this codepath.
+      goto end;
+    }
+    pmap_del2(envmap, name);
+  }
+  e = xmalloc(size);
+  int r = uv_os_getenv(name, e, &size);
+  if (r == UV_ENOBUFS) {
+    e = xrealloc(e, size);
+    r = uv_os_getenv(name, e, &size);
+  }
+  if (r != 0 || size == 0 || e[0] == '\0') {
+    xfree(e);
+    e = NULL;
+    if (r != 0 && r != UV_ENOENT && r != UV_UNKNOWN) {
+      ELOG("uv_os_getenv(%s) failed: %d %s", name, r, uv_err_name(r));
+    }
+    goto end;
+  }
+  pmap_put(cstr_t)(envmap, xstrdup(name), e);
+end:
+  uv_mutex_unlock(&mutex);
+  return (e == NULL || size == 0 || e[0] == '\0') ? NULL : e;
 }
 
-/// Returns `true` if the environment variable, `name`, has been defined
-/// (even if empty).
+/// Returns true if environment variable `name` is defined (even if empty).
+/// Returns false if not found (UV_ENOENT) or other failure.
 bool os_env_exists(const char *name)
   FUNC_ATTR_NONNULL_ALL
 {
-  return getenv(name) != NULL;
+  if (name[0] == '\0') {
+    return false;
+  }
+  // Use a tiny buffer because we don't care about the value: if uv_os_getenv()
+  // returns UV_ENOBUFS, the env var was found.
+  char buf[1];
+  size_t size = sizeof(buf);
+  int r = uv_os_getenv(name, buf, &size);
+  assert(r != UV_EINVAL);
+  if (r != 0 && r != UV_ENOENT && r != UV_ENOBUFS) {
+    ELOG("uv_os_getenv(%s) failed: %d %s", name, r, uv_err_name(r));
+  }
+  return (r == 0 || r == UV_ENOBUFS);
 }
 
 int os_setenv(const char *name, const char *value, int overwrite)
   FUNC_ATTR_NONNULL_ALL
 {
-#ifdef WIN32
-  size_t envbuflen = strlen(name) + strlen(value) + 2;
-  char *envbuf = xmalloc(envbuflen);
-  snprintf(envbuf, envbuflen, "%s=%s", name, value);
-
-  wchar_t *p;
-  utf8_to_utf16(envbuf, &p);
-  xfree(envbuf);
-  if (p == NULL) {
+  if (name[0] == '\0') {
     return -1;
   }
-  _wputenv(p);
-  xfree(p);  // Unlike Unix systems, we can free the string for _wputenv().
-  return 0;
-#elif defined(HAVE_SETENV)
-  return setenv(name, value, overwrite);
-#elif defined(HAVE_PUTENV_S)
+#ifdef WIN32
   if (!overwrite && os_getenv(name) != NULL) {
     return 0;
   }
-  if (_putenv_s(name, value) == 0) {
+#else
+  if (!overwrite && os_env_exists(name)) {
     return 0;
   }
-  return -1;
-#else
-# error "This system has no implementation available for os_setenv()"
 #endif
+  uv_mutex_lock(&mutex);
+  pmap_del2(envmap, name);
+  int r = uv_os_setenv(name, value);
+  assert(r != UV_EINVAL);
+  if (r != 0) {
+    ELOG("uv_os_setenv(%s) failed: %d %s", name, r, uv_err_name(r));
+  }
+  uv_mutex_unlock(&mutex);
+  return r == 0 ? 0 : -1;
 }
 
 /// Unset environment variable
-///
-/// For systems where unsetenv() is not available the value will be set as an
-/// empty string
 int os_unsetenv(const char *name)
+  FUNC_ATTR_NONNULL_ALL
 {
-#ifdef HAVE_UNSETENV
-  return unsetenv(name);
-#else
-  return os_setenv(name, "", 1);
-#endif
+  if (name[0] == '\0') {
+    return -1;
+  }
+  uv_mutex_lock(&mutex);
+  pmap_del2(envmap, name);
+  int r = uv_os_unsetenv(name);
+  if (r != 0) {
+    ELOG("uv_os_unsetenv(%s) failed: %d %s", name, r, uv_err_name(r));
+  }
+  uv_mutex_unlock(&mutex);
+  return r == 0 ? 0 : -1;
 }
 
 char *os_getenvname_at_index(size_t index)
@@ -527,7 +582,7 @@ static char *remove_tail(char *path, char *pend, char *dirname)
   return pend;
 }
 
-/// Iterate over a delimited list.
+/// Iterates $PATH-like delimited list `val`.
 ///
 /// @note Environment variables must not be modified during iteration.
 ///
@@ -562,7 +617,7 @@ const void *vim_env_iter(const char delim,
   }
 }
 
-/// Iterate over a delimited list in reverse order.
+/// Iterates $PATH-like delimited list `val` in reverse order.
 ///
 /// @note Environment variables must not be modified during iteration.
 ///
@@ -599,11 +654,12 @@ const void *vim_env_iter_rev(const char delim,
   }
 }
 
-/// Vim's version of getenv().
-/// Special handling of $HOME, $VIM and $VIMRUNTIME, allowing the user to
-/// override the vim runtime directory at runtime.  Also does ACP to 'enc'
-/// conversion for Win32.  Result must be freed by the caller.
+/// Vim getenv() wrapper with special handling of $HOME, $VIM, $VIMRUNTIME,
+/// allowing the user to override the Nvim runtime directory at runtime.
+/// Result must be freed by the caller.
+///
 /// @param name Environment variable to expand
+/// @return [allocated] Expanded environment variable, or NULL
 char *vim_getenv(const char *name)
 {
   // init_path() should have been called before now.
@@ -869,9 +925,8 @@ char_u * home_replace_save(buf_T *buf, char_u *src) FUNC_ATTR_NONNULL_RET
   return dst;
 }
 
-/// Our portable version of setenv.
-/// Has special handling for $VIMRUNTIME to keep the localization machinery
-/// sane.
+/// Vim setenv() wrapper with special handling for $VIMRUNTIME to keep the
+/// localization machinery sane.
 void vim_setenv(const char *name, const char *val)
 {
   os_setenv(name, val, 1);
diff --git a/src/nvim/os/fs_defs.h b/src/nvim/os/fs_defs.h
index 2277d926b3..c9b148a5aa 100644
--- a/src/nvim/os/fs_defs.h
+++ b/src/nvim/os/fs_defs.h
@@ -21,9 +21,7 @@ typedef struct {
   uv_dirent_t ent;  ///< @private The entry information.
 } Directory;
 
-/// Function to convert libuv error to char * error description
-///
-/// negative libuv error codes are returned by a number of os functions.
+/// Converts libuv error (negative int) to error description string.
 #define os_strerror uv_strerror
 
 // Values returned by os_nodetype()
diff --git a/src/nvim/os/pty_process_unix.c b/src/nvim/os/pty_process_unix.c
index bcf57e1b5b..5fdf0e6181 100644
--- a/src/nvim/os/pty_process_unix.c
+++ b/src/nvim/os/pty_process_unix.c
@@ -157,11 +157,11 @@ static void init_child(PtyProcess *ptyproc)
   // New session/process-group. #6530
   setsid();
 
-  unsetenv("COLUMNS");
-  unsetenv("LINES");
-  unsetenv("TERMCAP");
-  unsetenv("COLORTERM");
-  unsetenv("COLORFGBG");
+  os_unsetenv("COLUMNS");
+  os_unsetenv("LINES");
+  os_unsetenv("TERMCAP");
+  os_unsetenv("COLORTERM");
+  os_unsetenv("COLORFGBG");
 
   signal(SIGCHLD, SIG_DFL);
   signal(SIGHUP, SIG_DFL);
@@ -177,7 +177,7 @@ static void init_child(PtyProcess *ptyproc)
   }
 
   char *prog = ptyproc->process.argv[0];
-  setenv("TERM", ptyproc->term_name ? ptyproc->term_name : "ansi", 1);
+  os_setenv("TERM", ptyproc->term_name ? ptyproc->term_name : "ansi", 1);
   execvp(prog, ptyproc->process.argv);
   ELOG("execvp failed: %s: %s", strerror(errno), prog);
   _exit(122);  // 122 is EXEC_FAILED in the Vim source.
diff --git a/src/nvim/path.c b/src/nvim/path.c
index 7903e3f4f4..a706e32773 100644
--- a/src/nvim/path.c
+++ b/src/nvim/path.c
@@ -2271,7 +2271,7 @@ int path_is_absolute(const char_u *fname)
 void path_guess_exepath(const char *argv0, char *buf, size_t bufsize)
   FUNC_ATTR_NONNULL_ALL
 {
-  char *path = getenv("PATH");
+  const char *path = os_getenv("PATH");
 
   if (path == NULL || path_is_absolute((char_u *)argv0)) {
     xstrlcpy(buf, argv0, bufsize);
diff --git a/test/functional/eval/let_spec.lua b/test/functional/eval/let_spec.lua
index 1bd3405698..0cbf40137e 100644
--- a/test/functional/eval/let_spec.lua
+++ b/test/functional/eval/let_spec.lua
@@ -2,13 +2,16 @@ local helpers = require('test.functional.helpers')(after_each)
 
 local eq = helpers.eq
 local clear = helpers.clear
+local command = helpers.command
+local eval = helpers.eval
 local meths = helpers.meths
 local redir_exec = helpers.redir_exec
 local source = helpers.source
+local nvim_dir = helpers.nvim_dir
 
 before_each(clear)
 
-describe(':let command', function()
+describe(':let', function()
   it('correctly lists variables with curly-braces', function()
     meths.set_var('v', {0})
     eq('\nv                     [0]', redir_exec('let {"v"}'))
@@ -42,4 +45,38 @@ describe(':let command', function()
       call feedkeys(":\e:echo l1 l3\n:echo 42\n:cq\n", "t")
     ]=])
   end)
+
+  it("multibyte env var #8398 #9267", function()
+    command("let $NVIM_TEST = 'AìaB'")
+    eq('AìaB', eval('$NVIM_TEST'))
+    command("let $NVIM_TEST = 'AaあB'")
+    eq('AaあB', eval('$NVIM_TEST'))
+    local mbyte = [[\p* .ม .ม .ม .ม่ .ม่ .ม่ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹֻ ֹֻ ֹֻ
+                    .ֹֻ .ֹֻ .ֹֻ ֹֻ ֹֻ ֹֻ .ֹֻ .ֹֻ .ֹֻ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹֻ ֹֻ
+                    .ֹֻ .ֹֻ .ֹֻ a a a ca ca ca à à à]]
+    command("let $NVIM_TEST = '"..mbyte.."'")
+    eq(mbyte, eval('$NVIM_TEST'))
+  end)
+
+  it("multibyte env var to child process #8398 #9267",  function()
+    if (not helpers.iswin()) and require('test.helpers').isCI() then
+      -- Fails on non-Windows CI. Buffering/timing issue?
+      pending('fails on unix CI', function() end)
+    end
+    local cmd_get_child_env = "let g:env_from_child = system(['"..nvim_dir.."/printenv-test', 'NVIM_TEST'])"
+    command("let $NVIM_TEST = 'AìaB'")
+    command(cmd_get_child_env)
+    eq(eval('$NVIM_TEST'), eval('g:env_from_child'))
+
+    command("let $NVIM_TEST = 'AaあB'")
+    command(cmd_get_child_env)
+    eq(eval('$NVIM_TEST'), eval('g:env_from_child'))
+
+    local mbyte = [[\p* .ม .ม .ม .ม่ .ม่ .ม่ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹֻ ֹֻ ֹֻ
+                    .ֹֻ .ֹֻ .ֹֻ ֹֻ ֹֻ ֹֻ .ֹֻ .ֹֻ .ֹֻ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹ ֹ ֹ .ֹ .ֹ .ֹ ֹֻ ֹֻ
+                    .ֹֻ .ֹֻ .ֹֻ a a a ca ca ca à à à]]
+    command("let $NVIM_TEST = '"..mbyte.."'")
+    command(cmd_get_child_env)
+    eq(eval('$NVIM_TEST'), eval('g:env_from_child'))
+  end)
 end)
diff --git a/test/functional/fixtures/CMakeLists.txt b/test/functional/fixtures/CMakeLists.txt
index 8537ea390f..a7cd214b6b 100644
--- a/test/functional/fixtures/CMakeLists.txt
+++ b/test/functional/fixtures/CMakeLists.txt
@@ -3,3 +3,7 @@ target_link_libraries(tty-test ${LIBUV_LIBRARIES})
 
 add_executable(shell-test shell-test.c)
 add_executable(printargs-test printargs-test.c)
+add_executable(printenv-test printenv-test.c)
+if(WIN32)
+  set_target_properties(printenv-test PROPERTIES LINK_FLAGS -municode)
+endif()
diff --git a/test/functional/fixtures/printenv-test.c b/test/functional/fixtures/printenv-test.c
new file mode 100644
index 0000000000..5ac076f653
--- /dev/null
+++ b/test/functional/fixtures/printenv-test.c
@@ -0,0 +1,59 @@
+// This is an open source non-commercial project. Dear PVS-Studio, please check
+// it. PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
+
+#include <stdio.h>
+
+#ifdef WIN32
+# include <windows.h>
+#else
+# include <stdlib.h>
+#endif
+
+#ifdef WIN32
+int wmain(int argc, wchar_t **argv)
+#else
+int main(int argc, char **argv)
+#endif
+{
+  if (argc != 2) {
+    return 1;
+  }
+
+#ifdef WIN32
+  wchar_t *value = _wgetenv(argv[1]);
+  if (value == NULL) {
+    return 1;
+  }
+  int utf8_len = WideCharToMultiByte(CP_UTF8,
+                                     0,
+                                     value,
+                                     -1,
+                                     NULL,
+                                     0,
+                                     NULL,
+                                     NULL);
+  if (utf8_len == 0) {
+    return (int)GetLastError();
+  }
+  char *utf8_value = (char *)calloc((size_t)utf8_len, sizeof(char));
+  utf8_len = WideCharToMultiByte(CP_UTF8,
+                                 0,
+                                 value,
+                                 -1,
+                                 utf8_value,
+                                 utf8_len,
+                                 NULL,
+                                 NULL);
+  fprintf(stderr, "%s", utf8_value);
+  free(utf8_value);
+#else
+  char *value = getenv(argv[1]);
+  if (value == NULL) {
+    fprintf(stderr, "env var not found: %s", argv[1]);
+    return 1;
+  }
+  // Print to stderr to avoid buffering.
+  fprintf(stderr, "%s", value);
+#endif
+  return 0;
+}
diff --git a/test/helpers.lua b/test/helpers.lua
index 59da274e87..9f998ef919 100644
--- a/test/helpers.lua
+++ b/test/helpers.lua
@@ -708,7 +708,7 @@ end
 local function isCI()
   local is_travis = nil ~= os.getenv('TRAVIS')
   local is_appveyor = nil ~= os.getenv('APPVEYOR')
-  local is_quickbuild = nil ~= os.getenv('PR_NUMBER')
+  local is_quickbuild = nil ~= lfs.attributes('/usr/home/quickbuild')
   return is_travis or is_appveyor or is_quickbuild
 end
 
@@ -751,6 +751,7 @@ local module = {
   hasenv = hasenv,
   hexdump = hexdump,
   intchar2lua = intchar2lua,
+  isCI = isCI,
   map = map,
   matches = matches,
   mergedicts_copy = mergedicts_copy,
diff --git a/test/unit/helpers.lua b/test/unit/helpers.lua
index f8143a0125..beb25f25db 100644
--- a/test/unit/helpers.lua
+++ b/test/unit/helpers.lua
@@ -645,16 +645,16 @@ local function itp_child(wr, func)
     s = s:sub(1, hook_msglen - 2)
     sc.write(wr, '>' .. s .. (' '):rep(hook_msglen - 2 - #s) .. '\n')
   end
-  local err, emsg = pcall(init)
-  if err then
+  local status, result = pcall(init)
+  if status then
     collectgarbage('stop')
     child_sethook(wr)
-    err, emsg = pcall(func)
+    status, result = pcall(func)
     debug.sethook()
   end
-  emsg = tostring(emsg)
   sc.write(wr, trace_end_msg)
-  if not err then
+  if not status then
+    local emsg = tostring(result)
     if #emsg > 99999 then
       emsg = emsg:sub(1, 99999)
     end
@@ -668,7 +668,7 @@ local function itp_child(wr, func)
   collectgarbage()
   sc.write(wr, '$\n')
   sc.close(wr)
-  sc.exit(err and 0 or 1)
+  sc.exit(status and 0 or 1)
 end
 
 local function check_child_err(rd)
diff --git a/test/unit/os/env_spec.lua b/test/unit/os/env_spec.lua
index c54d5a9b77..c543551607 100644
--- a/test/unit/os/env_spec.lua
+++ b/test/unit/os/env_spec.lua
@@ -8,17 +8,22 @@ local ffi = helpers.ffi
 local cstr = helpers.cstr
 local to_cstr = helpers.to_cstr
 local NULL = helpers.NULL
+local OK = 0
 
 require('lfs')
 
 local cimp = cimport('./src/nvim/os/os.h')
 
 describe('env.c', function()
+  local function os_env_exists(name)
+    return cimp.os_env_exists(to_cstr(name))
+  end
+
   local function os_setenv(name, value, override)
     return cimp.os_setenv(to_cstr(name), to_cstr(value), override)
   end
 
-  local function os_unsetenv(name, _, _)
+  local function os_unsetenv(name)
     return cimp.os_unsetenv(to_cstr(name))
   end
 
@@ -31,25 +36,44 @@ describe('env.c', function()
     end
   end
 
-  describe('os_setenv', function()
-    local OK = 0
+  itp('os_env_exists', function()
+    eq(false, os_env_exists(''))
+    eq(false, os_env_exists('      '))
+    eq(false, os_env_exists('\t'))
+    eq(false, os_env_exists('\n'))
+    eq(false, os_env_exists('AaあB <= very weird name...'))
 
-    itp('sets an env variable and returns OK', function()
+    local varname = 'NVIM_UNIT_TEST_os_env_exists'
+    eq(false, os_env_exists(varname))
+    eq(OK, os_setenv(varname, 'foo bar baz ...', 1))
+    eq(true, os_env_exists(varname))
+  end)
+
+  describe('os_setenv', function()
+    itp('sets an env var and returns success', function()
       local name = 'NVIM_UNIT_TEST_SETENV_1N'
       local value = 'NVIM_UNIT_TEST_SETENV_1V'
       eq(nil, os.getenv(name))
-      eq(OK, (os_setenv(name, value, 1)))
+      eq(OK, os_setenv(name, value, 1))
       eq(value, os.getenv(name))
+
+      -- Set empty, then set non-empty, then retrieve.
+      eq(OK, os_setenv(name, '', 1))
+      eq('', os.getenv(name))
+      eq(OK, os_setenv(name, 'non-empty', 1))
+      eq('non-empty', os.getenv(name))
     end)
 
-    itp("dosn't overwrite an env variable if overwrite is 0", function()
+    itp("`overwrite` behavior", function()
       local name = 'NVIM_UNIT_TEST_SETENV_2N'
       local value = 'NVIM_UNIT_TEST_SETENV_2V'
       local value_updated = 'NVIM_UNIT_TEST_SETENV_2V_UPDATED'
-      eq(OK, (os_setenv(name, value, 0)))
+      eq(OK, os_setenv(name, value, 0))
       eq(value, os.getenv(name))
-      eq(OK, (os_setenv(name, value_updated, 0)))
+      eq(OK, os_setenv(name, value_updated, 0))
       eq(value, os.getenv(name))
+      eq(OK, os_setenv(name, value_updated, 1))
+      eq(value_updated, os.getenv(name))
     end)
   end)
 
@@ -93,31 +117,42 @@ describe('env.c', function()
   end)
 
   describe('os_getenv', function()
-    itp('reads an env variable', function()
+    itp('reads an env var', function()
       local name = 'NVIM_UNIT_TEST_GETENV_1N'
       local value = 'NVIM_UNIT_TEST_GETENV_1V'
       eq(NULL, os_getenv(name))
       -- Use os_setenv because Lua dosen't have setenv.
       os_setenv(name, value, 1)
       eq(value, os_getenv(name))
+
+      -- Get a big value.
+      local bigval = ('x'):rep(256)
+      eq(OK, os_setenv(name, bigval, 1))
+      eq(bigval, os_getenv(name))
+
+      -- Set non-empty, then set empty.
+      eq(OK, os_setenv(name, 'non-empty', 1))
+      eq('non-empty', os_getenv(name))
+      eq(OK, os_setenv(name, '', 1))
+      eq(NULL, os_getenv(name))
     end)
 
-    itp('returns NULL if the env variable is not found', function()
-      local name = 'NVIM_UNIT_TEST_GETENV_NOTFOUND'
-      return eq(NULL, os_getenv(name))
+    itp('returns NULL if the env var is not found', function()
+      eq(NULL, os_getenv('NVIM_UNIT_TEST_GETENV_NOTFOUND'))
     end)
   end)
 
-  describe('os_unsetenv', function()
-    itp('unsets environment variable', function()
-      local name = 'TEST_UNSETENV'
-      local value = 'TESTVALUE'
-      os_setenv(name, value, 1)
-      os_unsetenv(name)
-      neq(os_getenv(name), value)
-      -- Depending on the platform the var might be unset or set as ''
-      assert.True(os_getenv(name) == nil or os_getenv(name) == '')
-    end)
+  itp('os_unsetenv', function()
+    local name = 'TEST_UNSETENV'
+    local value = 'TESTVALUE'
+    os_setenv(name, value, 1)
+    eq(OK, os_unsetenv(name))
+    neq(os_getenv(name), value)
+    -- Depending on the platform the var might be unset or set as ''
+    assert.True(os_getenv(name) == nil or os_getenv(name) == '')
+    if os_getenv(name) == nil then
+      eq(false, os_env_exists(name))
+    end
   end)
 
   describe('os_getenvname_at_index', function()