Skip to content

Conversation

@Sterling-Augustine
Copy link
Contributor

There is no guarantee that this environment variable is set. Eg, when running a test outside of the build system, such as under a debugger. And passing a nullptr to the string constructor is undefined.

Use an empty string, which seems like it is close to the original intent.

There is no guarantee that this environment variable is set. Eg, when
running a test outside of the build system, such as under a debugger.
And passing a nullptr to the string constructor is undefined.

Use an empty string, which seems like it is close to the original
intent.
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2025

@llvm/pr-subscribers-libc

Author: None (Sterling-Augustine)

Changes

There is no guarantee that this environment variable is set. Eg, when running a test outside of the build system, such as under a debugger. And passing a nullptr to the string constructor is undefined.

Use an empty string, which seems like it is close to the original intent.


Full diff: https://github.com/llvm/llvm-project/pull/167422.diff

1 Files Affected:

  • (modified) libc/test/UnitTest/BazelFilePath.cpp (+4)
diff --git a/libc/test/UnitTest/BazelFilePath.cpp b/libc/test/UnitTest/BazelFilePath.cpp
index ee5fcaaa63d91..7f9f42b46dca9 100644
--- a/libc/test/UnitTest/BazelFilePath.cpp
+++ b/libc/test/UnitTest/BazelFilePath.cpp
@@ -20,6 +20,10 @@ namespace testing {
 CString libc_make_test_file_path_func(const char *file_name) {
   // This is the path to the folder bazel wants the test outputs written to.
   const char *UNDECLARED_OUTPUTS_PATH = getenv("TEST_UNDECLARED_OUTPUTS_DIR");
+  // Do something sensible if not run under bazel, otherwise this may segfault
+  // when constructing the string.
+  if (UNDECLARED_OUTPUTS_PATH == nullptr)
+    UNDECLARED_OUTPUTS_PATH = "";
 
   return cpp::string(UNDECLARED_OUTPUTS_PATH) + file_name;
 }

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

This shouldn't be necessary, this file is only built under the bazel build. For cmake we use CmakeFilePath.cpp

@Sterling-Augustine
Copy link
Contributor Author

This shouldn't be necessary, this file is only built under the bazel build. For cmake we use CmakeFilePath.cpp

If you run the bazel-built test under gdb (as I did while working on #167350) it will segfault.

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

Ah, that makes sense. LGTM

@Sterling-Augustine Sterling-Augustine merged commit e77001c into llvm:main Nov 11, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants