Skip to content

Conversation

@ndrewh
Copy link
Contributor

@ndrewh ndrewh commented Nov 19, 2025

This test explicitly sets the environment for a spawned process. Without DYLD_LIBRARY_PATH, the spawned process may use a ASAN runtime other than the one that was used by the parent process That other runtime library may not work at all, or may not be in the default search path. Either case can cause the spawned process to die before it makes it to main, thus failing the test. The compiler-rt lit config sets the library path variable here (i.e. to ensure that just-built runtimes are used for tests, in the case of a standalone compiler-rt build), and that is currently used by the parent process but not the spawned ones.

My change only forwards the variable for Darwin (DYLD_LIBRARY_PATH), but we ought to also forward the variable for other platforms. However, it's not clear that there's any good way to plumb this into the test, since some platforms actually have multiple library path variables which would need to be forwarded (see: SunOS here). I considered adding a substitution variable for the library path variable, but that doesn't really work if there's multiple such variables.

@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Andrew Haberlandt (ndrewh)

Changes

This test explicitly sets the environment for a spawned process. Since the compiler-rt lit config sets the library path variable here (i.e. to ensure that just-built runtimes are used for tests), we should also forward it to the spawned process.

My change only forwards the variable for Darwin (DYLD_LIBRARY_PATH), but we ought to also forward the variable for other platforms. However, it's not clear that there's any good way to plumb this into the test, since some platforms actually have multiple library path variables which would need to be forwarded (see: SunOS here). I considered adding a substitution variable for the library path variable, but that doesn't really work if there's multiple such variables.


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

1 Files Affected:

  • (modified) compiler-rt/test/sanitizer_common/TestCases/Posix/posix_spawn.c (+8-1)
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/posix_spawn.c b/compiler-rt/test/sanitizer_common/TestCases/Posix/posix_spawn.c
index ea58b92af6097..e570a61f9ac9a 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Posix/posix_spawn.c
+++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/posix_spawn.c
@@ -7,6 +7,8 @@
 #include <spawn.h>
 #include <stdio.h>
 #include <sys/wait.h>
+#include <string.h>
+#include <stdlib.h>
 
 int main(int argc, char **argv) {
   if (argc > 1) {
@@ -23,11 +25,16 @@ int main(int argc, char **argv) {
       argv[0], "2", "3", "4", "2", "3", "4", "2", "3", "4",
       "2",     "3", "4", "2", "3", "4", "2", "3", "4", NULL,
   };
-  char *const env[] = {
+  const char *env[] = {
       "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B",
       "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", "A=B", NULL,
   };
 
+  char *lib_path = getenv("DYLD_LIRARY_PATH");
+  if (lib_path) {
+    env[0] = strdup(lib_path);
+  }
+
   pid_t pid;
   int s = posix_spawn(&pid, argv[0], &file_actions, &attr, args, env);
   assert(!s);

@github-actions
Copy link

🐧 Linux x64 Test Results

  • 5824 tests passed
  • 1319 tests skipped

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Do you have a motivating example for this? I'm struggling to understand why this is necessary.

@ndrewh
Copy link
Contributor Author

ndrewh commented Nov 20, 2025

@boomanaiden154 Without DYLD_LIBRARY_PATH, the spawned process can use a ASAN runtime other than the one that was just built. That other runtime library may not work at all, or may not be in the default search path. Either case can cause the spawned process to die before it makes it to main, thus failing the test.

The specific use-case is %run wrappers which can copy the test and libraries to a remote device and run with DYLD_LIBRARY_PATH set. However, I think this test could also cause issues with any standalone compiler-rt build, since DYLD_LIBRARY_PATH ensures that just-built runtimes are used rather than those in your compiler's resource directory or elsewhere on your system.

(I updated the PR description to hopefully clarify this)

I fucked up in my original patch since I lost the DYLD_LIBRARY_PATH= part. I've fixed that and now just walk environ directly.

Copy link
Contributor

@DanBlackwell DanBlackwell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR; please add a comment before the loop though for future readers.

@ndrewh ndrewh merged commit b5c0fcd into llvm:main Nov 20, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants