[libc-commits] [libc] [libc][stdlib] Avoid scanning values during environment lookup (PR #228633)

[via libc-commits]

https://github.com/ChenMiaoi created https://github.com/llvm/llvm-project/pull/228633

Constructing a string_view for each environment entry scans the entire value to determine its length, even when the name does not match. This makes lookups unnecessarily expensive for long values such as PATH.

Filter entries by their first character, then compare the remaining name up to a mismatch or null terminator. Require a following '=' to match the complete key, preserving support for empty names in putenv.

Closes #196016

>From bdb3bbfe0eb7786758cad3887bbb2db0178f495b Mon Sep 17 00:00:00 2001
From: Chen Miao <chenmiao.ku at gmail.com>
Date: Fri, 2 Oct 2026 23:31:27 +0800
Subject: [PATCH] [libc][stdlib] Avoid scanning values during environment
 lookup

Constructing a string_view for each environment entry scans the entire
value to determine its length, even when the name does not match. This
makes lookups unnecessarily expensive for long values such as PATH.

Filter entries by their first character, then compare the remaining name
up to a mismatch or null terminator. Require a following '=' to match the
complete key, preserving support for empty names in putenv.

Closes #196016
---
 libc/src/stdlib/environ_internal.cpp          | 31 +++++++++++++++---
 .../integration/src/stdlib/CMakeLists.txt     |  1 +
 .../integration/src/stdlib/getenv_test.cpp    | 32 +++++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/libc/src/stdlib/environ_internal.cpp b/libc/src/stdlib/environ_internal.cpp
index da2e5c8185ca06c..29cb83f22bbe691 100644
--- a/libc/src/stdlib/environ_internal.cpp
+++ b/libc/src/stdlib/environ_internal.cpp
@@ -82,10 +82,33 @@ cpp::optional<size_t> EnvironmentManager::find_var(cpp::string_view name) {
   if (!env_array)
     return cpp::nullopt;
 
-  for (size_t i = 0; i < count; i++) {
-    cpp::string_view current(env_array[i]);
-    if (current.starts_with(name) && current.size() > name.size() &&
-        current[name.size()] == '=')
+  // Compare entries directly: constructing a string_view from the entry's C
+  // string would scan the entire value to determine the entry's length, even
+  // when the name does not match.
+  // Empty names, supported by putenv, match entries starting with '='.
+  const char first = name.empty() ? '=' : name.front();
+
+  // Reject a leading null so matching an empty entry cannot advance past its
+  // terminator when the comparison resumes at index 1.
+  if (first == '\0')
+    return cpp::nullopt;
+  const size_t start = name.empty() ? 0 : 1;
+
+  for (size_t i = 0; i < count; ++i) {
+    const char *entry = env_array[i];
+    // Reject entries with a different first character before comparing the
+    // rest.
+    if (entry[0] != first)
+      continue;
+
+    size_t j = start;
+    // Bound the comparison by the name's length and stop at a mismatch or null
+    // terminator, so shorter entries cannot cause an out-of-bounds read.
+    while (j < name.size() && entry[j] != '\0' && entry[j] == name[j])
+      ++j;
+
+    // Require the full key to match, not just a prefix of a longer key.
+    if (j == name.size() && entry[j] == '=')
       return i;
   }
 
diff --git a/libc/test/integration/src/stdlib/CMakeLists.txt b/libc/test/integration/src/stdlib/CMakeLists.txt
index 7976e3833e282ba..da3acfbf74d7dc1 100644
--- a/libc/test/integration/src/stdlib/CMakeLists.txt
+++ b/libc/test/integration/src/stdlib/CMakeLists.txt
@@ -11,6 +11,7 @@ add_integration_test(
   SRCS
     getenv_test.cpp
   DEPENDS
+    libc.src.stdlib.environ_internal
     libc.src.stdlib.getenv
   ENV
     FRANCE=Paris
diff --git a/libc/test/integration/src/stdlib/getenv_test.cpp b/libc/test/integration/src/stdlib/getenv_test.cpp
index fe323c948afc487..d0218767ca8faf6 100644
--- a/libc/test/integration/src/stdlib/getenv_test.cpp
+++ b/libc/test/integration/src/stdlib/getenv_test.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/stdlib/environ_internal.h"
 #include "src/stdlib/getenv.h"
 
 #include "test/IntegrationTest/test.h"
@@ -21,5 +22,36 @@ TEST_MAIN([[maybe_unused]] int argc, [[maybe_unused]] char **argv,
   ASSERT_TRUE(LIBC_NAMESPACE::getenv("FRANC") == nullptr);
   ASSERT_TRUE(LIBC_NAMESPACE::getenv("FRANCE1") == nullptr);
 
+  auto &env = LIBC_NAMESPACE::internal::EnvironmentManager::get_instance();
+  // Remove inherited variables that could interfere with the lookup tests.
+  ASSERT_EQ(env.unset("LIBC_GETENV_TEST"), 0);
+  ASSERT_EQ(env.unset("LIBC_GETENV_TEST_LONG"), 0);
+
+  // Exercise entries shorter than the requested name, including entries
+  // without '='. Lookup must stop at the terminator of each entry.
+  char *const ORIGINAL = *env.begin();
+  static char empty_entry[] = "";
+  static char short_entry[] = "LIBC_GETENV_TEST";
+  static char long_key[] = "LIBC_GETENV_TEST_LONG=value";
+  static char empty_value[] = "LIBC_GETENV_TEST=";
+  static char value_with_equals[] = "LIBC_GETENV_TEST=first=second";
+
+  *env.begin() = empty_entry;
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST") == nullptr);
+  *env.begin() = short_entry;
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST") == nullptr);
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST_LONG") == nullptr);
+
+  *env.begin() = long_key;
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST") == nullptr);
+
+  *env.begin() = empty_value;
+  ASSERT_STREQ(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST"), "");
+
+  *env.begin() = value_with_equals;
+  ASSERT_STREQ(LIBC_NAMESPACE::getenv("LIBC_GETENV_TEST"), "first=second");
+
+  *env.begin() = ORIGINAL;
+
   return 0;
 }