[libc][stdlib] Implement setenv() with environment management infrastructure #163018
+656
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ructure
Implement setenv() for LLVM libc, enabling modification of process
environment variables in compliance with POSIX specifications.
Implementation Overview:
- Shared infrastructure in environ_internal.{h,cpp} providing thread-safe
environment array management with copy-on-write semantics
- Memory ownership tracking to distinguish library-allocated strings
from external strings (startup environ)
- Thread-safe operations using environ_mutex for all modifications
- Dynamic array growth with configurable initial capacity
Key Design Decisions:
- Linear array approach (O(n) lookups) suitable for typical environment
sizes (30-80 variables) with minimal memory overhead
- Copy-on-write: starts with startup environment, allocates on first
modification to avoid unnecessary copying
- Memory ownership model prevents double-free bugs: tracks which strings
can be safely freed (setenv allocations)
Function Implementation:
- setenv(name, value, overwrite): Allocates "name=value" string, respects
overwrite flag, validates inputs per POSIX (rejects NULL, empty names,
names with '='). Returns 0 on success, -1 with errno on error.
Testing:
- Comprehensive integration test suite covering basic functionality,
POSIX compliance, error conditions, and edge cases
- 12 test cases passing
|
@llvm/pr-subscribers-libc Author: Jeff Bailey (kaladron) ChangesImplement setenv() for LLVM libc, enabling modification of process environment variables in compliance with POSIX specifications. Implementation Overview:
Key Design Decisions:
Function Implementation:
Testing:
Patch is 23.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163018.diff 9 Files Affected:
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index b4ab073ec912f..ac0475cd5af46 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -1248,6 +1248,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.stdlib.exit
libc.src.stdlib.getenv
libc.src.stdlib.quick_exit
+ libc.src.stdlib.setenv
# signal.h entrypoints
libc.src.signal.kill
diff --git a/libc/include/stdlib.yaml b/libc/include/stdlib.yaml
index 3b2ff13c684b1..13a353a2dca1d 100644
--- a/libc/include/stdlib.yaml
+++ b/libc/include/stdlib.yaml
@@ -180,6 +180,14 @@ functions:
return_type: int
arguments:
- type: void
+ - name: setenv
+ standards:
+ - posix
+ return_type: int
+ arguments:
+ - type: const char *
+ - type: const char *
+ - type: int
- name: srand
standards:
- stdc
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index c464f82dcbda7..fd6493527c6b0 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -65,6 +65,38 @@ add_entrypoint_object(
libc.config.app_h
)
+add_object_library(
+ environ_internal
+ SRCS
+ environ_internal.cpp
+ HDRS
+ environ_internal.h
+ DEPENDS
+ libc.config.app_h
+ libc.hdr.types.size_t
+ libc.src.__support.CPP.string_view
+ libc.src.__support.threads.mutex
+ libc.src.stdlib.free
+ libc.src.stdlib.malloc
+ libc.src.string.memcpy
+)
+
+add_entrypoint_object(
+ setenv
+ SRCS
+ setenv.cpp
+ HDRS
+ setenv.h
+ DEPENDS
+ .environ_internal
+ libc.src.__support.CPP.string_view
+ libc.src.__support.libc_errno
+ libc.src.__support.threads.mutex
+ libc.src.stdlib.malloc
+ libc.src.string.memcpy
+ libc.src.string.strlen
+)
+
add_entrypoint_object(
strfromf
SRCS
diff --git a/libc/src/stdlib/environ_internal.cpp b/libc/src/stdlib/environ_internal.cpp
new file mode 100644
index 0000000000000..fbb0b968074a6
--- /dev/null
+++ b/libc/src/stdlib/environ_internal.cpp
@@ -0,0 +1,165 @@
+//===-- Implementation of internal environment utilities ------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "environ_internal.h"
+#include "config/app.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/macros/config.h"
+#include "src/string/memcpy.h"
+
+// We use extern "C" declarations for malloc/free/realloc instead of including
+// src/stdlib/malloc.h, src/stdlib/free.h, and src/stdlib/realloc.h. This allows
+// the implementation to work with different allocator implementations,
+// particularly in integration tests which provide a simple bump allocator. The
+// extern "C" linkage ensures we use whatever allocator is linked with the test
+// or application.
+extern "C" void *malloc(size_t);
+extern "C" void free(void *);
+extern "C" void *realloc(void *, size_t);
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+
+// Minimum initial capacity for the environment array when first allocated.
+// This avoids frequent reallocations for small environments.
+constexpr size_t MIN_ENVIRON_CAPACITY = 32;
+
+// Growth factor for environment array capacity when expanding.
+// When capacity is exceeded, new_capacity = old_capacity *
+// ENVIRON_GROWTH_FACTOR.
+constexpr size_t ENVIRON_GROWTH_FACTOR = 2;
+
+// Global state for environment management
+Mutex environ_mutex(false, false, false, false);
+char **environ_storage = nullptr;
+EnvStringOwnership *environ_ownership = nullptr;
+size_t environ_capacity = 0;
+size_t environ_size = 0;
+bool environ_is_ours = false;
+
+char **get_environ_array() {
+ if (environ_is_ours)
+ return environ_storage;
+ return reinterpret_cast<char **>(LIBC_NAMESPACE::app.env_ptr);
+}
+
+void init_environ() {
+ // Count entries in the startup environ
+ char **env_ptr = reinterpret_cast<char **>(LIBC_NAMESPACE::app.env_ptr);
+ if (!env_ptr)
+ return;
+
+ size_t count = 0;
+ for (char **env = env_ptr; *env != nullptr; env++)
+ count++;
+
+ environ_size = count;
+}
+
+int find_env_var(cpp::string_view name) {
+ char **env_array = get_environ_array();
+ if (!env_array)
+ return -1;
+
+ for (size_t i = 0; i < environ_size; i++) {
+ cpp::string_view current(env_array[i]);
+ if (!current.starts_with(name))
+ continue;
+
+ // Check that name is followed by '='
+ if (current.size() > name.size() && current[name.size()] == '=')
+ return static_cast<int>(i);
+ }
+
+ return -1;
+}
+
+bool ensure_capacity(size_t needed) {
+ // IMPORTANT: This function assumes environ_mutex is already held by the
+ // caller. Do not add locking here as it would cause deadlock.
+
+ // If we're still using the startup environ, we need to copy it
+ if (!environ_is_ours) {
+ char **old_env = reinterpret_cast<char **>(LIBC_NAMESPACE::app.env_ptr);
+
+ // Allocate new array with room to grow
+ size_t new_capacity = needed < MIN_ENVIRON_CAPACITY
+ ? MIN_ENVIRON_CAPACITY
+ : needed * ENVIRON_GROWTH_FACTOR;
+ char **new_storage =
+ reinterpret_cast<char **>(malloc(sizeof(char *) * (new_capacity + 1)));
+ if (!new_storage)
+ return false;
+
+ // Allocate ownership tracking array
+ EnvStringOwnership *new_ownership = reinterpret_cast<EnvStringOwnership *>(
+ malloc(sizeof(EnvStringOwnership) * (new_capacity + 1)));
+ if (!new_ownership) {
+ free(new_storage);
+ return false;
+ }
+
+ // Copy existing pointers (we don't own the strings yet, so just copy
+ // pointers)
+ if (old_env) {
+ for (size_t i = 0; i < environ_size; i++) {
+ new_storage[i] = old_env[i];
+ // Initialize ownership: startup strings are not owned by us
+ new_ownership[i] = EnvStringOwnership();
+ }
+ }
+ new_storage[environ_size] = nullptr;
+
+ environ_storage = new_storage;
+ environ_ownership = new_ownership;
+ environ_capacity = new_capacity;
+ environ_is_ours = true;
+
+ // Update app.env_ptr to point to our storage
+ LIBC_NAMESPACE::app.env_ptr =
+ reinterpret_cast<uintptr_t *>(environ_storage);
+
+ return true;
+ }
+
+ // We already own environ, check if we need to grow it
+ if (needed <= environ_capacity)
+ return true;
+
+ // Grow capacity by the growth factor
+ size_t new_capacity = needed * ENVIRON_GROWTH_FACTOR;
+
+ // Use realloc to grow the arrays
+ char **new_storage = reinterpret_cast<char **>(
+ realloc(environ_storage, sizeof(char *) * (new_capacity + 1)));
+ if (!new_storage)
+ return false;
+
+ EnvStringOwnership *new_ownership =
+ reinterpret_cast<EnvStringOwnership *>(realloc(
+ environ_ownership, sizeof(EnvStringOwnership) * (new_capacity + 1)));
+ if (!new_ownership) {
+ // If ownership realloc fails, we still have the old storage in new_storage
+ // which was successfully reallocated. We need to restore or handle this.
+ // For safety, we'll keep the successfully reallocated storage.
+ environ_storage = new_storage;
+ return false;
+ }
+
+ environ_storage = new_storage;
+ environ_ownership = new_ownership;
+ environ_capacity = new_capacity;
+
+ // Update app.env_ptr to point to our new storage
+ LIBC_NAMESPACE::app.env_ptr = reinterpret_cast<uintptr_t *>(environ_storage);
+
+ return true;
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/environ_internal.h b/libc/src/stdlib/environ_internal.h
new file mode 100644
index 0000000000000..4d4d2b85bc779
--- /dev/null
+++ b/libc/src/stdlib/environ_internal.h
@@ -0,0 +1,82 @@
+//===-- Internal utilities for environment management ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDLIB_ENVIRON_INTERNAL_H
+#define LLVM_LIBC_SRC_STDLIB_ENVIRON_INTERNAL_H
+
+#include "hdr/types/size_t.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/macros/attributes.h"
+#include "src/__support/macros/config.h"
+#include "src/__support/threads/mutex.h"
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+
+// Ownership information for environment strings.
+// We need to track ownership because environment strings come from three
+// sources:
+// 1. Startup environment (from program loader) - we don't own these
+// 2. putenv() calls where caller provides the string - we don't own these
+// 3. setenv() calls where we allocate the string - we DO own these
+// Only strings we allocated can be freed when replaced or removed.
+struct EnvStringOwnership {
+ bool allocated_by_us; // True if we malloc'd this string (must free).
+ // False for startup environ or putenv strings (don't
+ // free).
+
+ // Default: not owned by us (startup or putenv - don't free).
+ LIBC_INLINE EnvStringOwnership() : allocated_by_us(false) {}
+
+ // Returns true if this string can be safely freed.
+ LIBC_INLINE bool can_free() const { return allocated_by_us; }
+};
+
+// Global mutex protecting all environ modifications
+extern Mutex environ_mutex;
+
+// Our allocated environ array (nullptr if using startup environ)
+extern char **environ_storage;
+
+// Parallel array tracking ownership of each environ string
+// Same size/capacity as environ_storage
+extern EnvStringOwnership *environ_ownership;
+
+// Allocated capacity of environ_storage
+extern size_t environ_capacity;
+
+// Current number of variables in environ
+extern size_t environ_size;
+
+// True if we allocated environ_storage (and are responsible for freeing it)
+extern bool environ_is_ours;
+
+// Search for a variable by name in the current environ array.
+// Returns the index if found, or -1 if not found.
+// This function assumes the mutex is already held.
+int find_env_var(cpp::string_view name);
+
+// Ensure environ has capacity for at least `needed` entries (plus null
+// terminator). May allocate or reallocate environ_storage. Returns true on
+// success, false on allocation failure. This function assumes the mutex is
+// already held.
+bool ensure_capacity(size_t needed);
+
+// Get a pointer to the current environ array.
+// This may be app.env_ptr (startup environ) or environ_storage (our copy).
+char **get_environ_array();
+
+// Initialize environ management from the startup environment.
+// This must be called before any setenv/unsetenv operations.
+// This function is thread-safe and idempotent.
+void init_environ();
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDLIB_ENVIRON_INTERNAL_H
diff --git a/libc/src/stdlib/setenv.cpp b/libc/src/stdlib/setenv.cpp
new file mode 100644
index 0000000000000..156c94cc66cb7
--- /dev/null
+++ b/libc/src/stdlib/setenv.cpp
@@ -0,0 +1,141 @@
+//===-- Implementation of setenv ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdlib/setenv.h"
+#include "environ_internal.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/common.h"
+#include "src/__support/libc_errno.h"
+#include "src/__support/macros/config.h"
+#include "src/string/memcpy.h"
+#include "src/string/strlen.h"
+
+// We use extern "C" declarations for malloc/free instead of including
+// src/stdlib/malloc.h and src/stdlib/free.h. This allows the implementation
+// to work with different allocator implementations, particularly in integration
+// tests which provide a simple bump allocator. The extern "C" linkage ensures
+// we use whatever allocator is linked with the test or application.
+extern "C" void *malloc(size_t);
+extern "C" void free(void *);
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, setenv,
+ (const char *name, const char *value, int overwrite)) {
+ // Validate inputs
+ if (name == nullptr || value == nullptr) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ cpp::string_view name_view(name);
+ if (name_view.empty()) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ // POSIX: name cannot contain '='
+ if (name_view.find_first_of('=') != cpp::string_view::npos) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ // Lock mutex for thread safety
+ internal::environ_mutex.lock();
+
+ // Initialize environ if not already done
+ internal::init_environ();
+
+ // Search for existing variable
+ int index = internal::find_env_var(name_view);
+
+ if (index >= 0) {
+ // Variable exists
+ if (overwrite == 0) {
+ // Don't overwrite, just return success
+ internal::environ_mutex.unlock();
+ return 0;
+ }
+
+ // Need to replace the value
+ // Calculate size for "name=value" string
+ size_t name_len = LIBC_NAMESPACE::strlen(name);
+ size_t value_len = LIBC_NAMESPACE::strlen(value);
+ size_t total_len =
+ name_len + 1 + value_len + 1; // name + '=' + value + '\0'
+
+ char *new_string = reinterpret_cast<char *>(malloc(total_len));
+ if (!new_string) {
+ internal::environ_mutex.unlock();
+ libc_errno = ENOMEM;
+ return -1;
+ }
+
+ // Build "name=value" string
+ LIBC_NAMESPACE::memcpy(new_string, name, name_len);
+ new_string[name_len] = '=';
+ LIBC_NAMESPACE::memcpy(new_string + name_len + 1, value, value_len);
+ new_string[name_len + 1 + value_len] = '\0';
+
+ // Replace in environ array
+ char **env_array = internal::get_environ_array();
+
+ // Free old string if we allocated it
+ if (internal::environ_ownership[index].can_free()) {
+ free(env_array[index]);
+ }
+
+ env_array[index] = new_string;
+ // Mark this string as allocated by us
+ internal::environ_ownership[index].allocated_by_us = true;
+
+ internal::environ_mutex.unlock();
+ return 0;
+ }
+
+ // Variable doesn't exist, need to add it
+ // Ensure we have capacity for one more entry
+ if (!internal::ensure_capacity(internal::environ_size + 1)) {
+ internal::environ_mutex.unlock();
+ libc_errno = ENOMEM;
+ return -1;
+ }
+
+ // Calculate size for "name=value" string
+ size_t name_len = LIBC_NAMESPACE::strlen(name);
+ size_t value_len = LIBC_NAMESPACE::strlen(value);
+ size_t total_len = name_len + 1 + value_len + 1; // name + '=' + value + '\0'
+
+ char *new_string = reinterpret_cast<char *>(malloc(total_len));
+ if (!new_string) {
+ internal::environ_mutex.unlock();
+ libc_errno = ENOMEM;
+ return -1;
+ }
+
+ // Build "name=value" string
+ LIBC_NAMESPACE::memcpy(new_string, name, name_len);
+ new_string[name_len] = '=';
+ LIBC_NAMESPACE::memcpy(new_string + name_len + 1, value, value_len);
+ new_string[name_len + 1 + value_len] = '\0';
+
+ // Add to environ array
+ char **env_array = internal::get_environ_array();
+ env_array[internal::environ_size] = new_string;
+
+ // Mark this string as allocated by us
+ internal::environ_ownership[internal::environ_size].allocated_by_us = true;
+
+ internal::environ_size++;
+ env_array[internal::environ_size] = nullptr; // Maintain null terminator
+
+ internal::environ_mutex.unlock();
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdlib/setenv.h b/libc/src/stdlib/setenv.h
new file mode 100644
index 0000000000000..0a6ad78be92cd
--- /dev/null
+++ b/libc/src/stdlib/setenv.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for setenv ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDLIB_SETENV_H
+#define LLVM_LIBC_SRC_STDLIB_SETENV_H
+
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int setenv(const char *name, const char *value, int overwrite);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDLIB_SETENV_H
diff --git a/libc/test/integration/src/stdlib/CMakeLists.txt b/libc/test/integration/src/stdlib/CMakeLists.txt
index 1773d9fc9f0f5..c7d005b2e81db 100644
--- a/libc/test/integration/src/stdlib/CMakeLists.txt
+++ b/libc/test/integration/src/stdlib/CMakeLists.txt
@@ -16,3 +16,15 @@ add_integration_test(
FRANCE=Paris
GERMANY=Berlin
)
+
+add_integration_test(
+ setenv_test
+ SUITE
+ stdlib-integration-tests
+ SRCS
+ setenv_test.cpp
+ DEPENDS
+ libc.src.stdlib.getenv
+ libc.src.stdlib.setenv
+ libc.src.string.strcmp
+)
diff --git a/libc/test/integration/src/stdlib/setenv_test.cpp b/libc/test/integration/src/stdlib/setenv_test.cpp
new file mode 100644
index 0000000000000..fbedadbf3a2e6
--- /dev/null
+++ b/libc/test/integration/src/stdlib/setenv_test.cpp
@@ -0,0 +1,190 @@
+//===-- Unittests for setenv ----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdlib/getenv.h"
+#include "src/stdlib/setenv.h"
+#include "src/stdlib/unsetenv.h"
+#include "src/string/strcmp.h"
+
+#include "test/IntegrationTest/test.h"
+
+#include <errno.h>
+
+TEST_MAIN([[maybe_unused]] int argc, [[maybe_unused]] char **argv,
+ [[maybe_unused]] char **envp) {
+ // Test: Basic
+ {
+ // Set a simple environment variable
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("SETENV_TEST_VAR", "test_value", 1), 0);
+
+ // Verify it was set
+ char *value = LIBC_NAMESPACE::getenv("SETENV_TEST_VAR");
+ ASSERT_TRUE(value != nullptr);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(value, "test_value"), 0);
+
+ // Clean up
+ LIBC_NAMESPACE::unsetenv("SETENV_TEST_VAR");
+ }
+
+ // Test: OverwriteExisting
+ {
+ // Set initial value
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("OVERWRITE_VAR", "original", 1), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("OVERWRITE_VAR"),
+ "original"),
+ 0);
+
+ // Overwrite with new value (overwrite = 1)
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("OVERWRITE_VAR", "replaced", 1), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("OVERWRITE_VAR"),
+ "replaced"),
+ 0);
+
+ // Clean up
+ LIBC_NAMESPACE::unsetenv("OVERWRITE_VAR");
+ }
+
+ // Test: NoOverwriteFlag
+ {
+ // Set initial value
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("NO_OVERWRITE_VAR", "original", 1), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("NO_OVERWRITE_VAR"),
+ "original"),
+ 0);
+
+ // Try to set with overwrite = 0 (should not change)
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("NO_OVERWRITE_VAR", "ignored", 0), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("NO_OVERWRITE_VAR"),
+ "original"),
+ 0);
+
+ // Verify it still works with overwrite = 1
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("NO_OVERWRITE_VAR", "changed", 1), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("NO_OVERWRITE_VAR"),
+ "changed"),
+ 0);
+
+ // Clean up
+ LIBC_NAMESPACE::unsetenv("NO_OVERWRITE_VAR");
+ }
+
+ // Test: NullName
+ {
+ errno = 0;
+ ASSERT_EQ(LIBC_NAMESPACE::setenv(nullptr, "value", 1), -1);
+ ASSERT_ERRNO_EQ(EINVAL);
+ }
+
+ // Test: NullValue
+ {
+ errno = 0;
+ ASSERT_EQ(LIBC_NAMESPACE::setenv("NULL_VALUE_VAR", nullptr,...
[truncated]
|
petrhosek
reviewed
Oct 11, 2025
Comment on lines
+83
to
+84
| // IMPORTANT: This function assumes environ_mutex is already held by the | ||
| // caller. Do not add locking here as it would cause deadlock. |
There was a problem hiding this comment.
Can we use Clang Thread Safety Analysis to statically check this?
There was a problem hiding this comment.
I'm happy to give it a try! I'm heading to bed now, but will take a look in the morning (likewise with the windows and Mac bot failures).
I tried running with ASan / UBSan but couldn't figure those out with the bump allocator.
There was a problem hiding this comment.
I took a look - today, no. They're only really used for testing so far, not actually deployed inside LLVM. It looks like we'd have to build some infrastructure to use them, and I think that's best saved for a follow-up PR. I have a few things queued up that I'd like to get landed and then would happily loop back to this.
…ter unsetenv implementation (coming in subsequent commit)
… on Windows and Mac overlay builds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement setenv() for LLVM libc, enabling modification of process environment variables in compliance with POSIX specifications.
Implementation Overview:
Key Design Decisions:
Function Implementation:
Testing: