Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sanitizer][test] Unify LD_LIBRARY_PATH handling #111498

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Oct 8, 2024

When testing on Linux/sparc64 with a runtimes build, the UBSan-Standalone-sparc :: TestCases/Misc/Linux/sigaction.cpp test FAILs:

runtimes/runtimes-bins/compiler-rt/test/ubsan/Standalone-sparc/TestCases/Misc/Linux/Output/sigaction.cpp.tmp: error while loading shared libraries: libclang_rt.ubsan_standalone.so: wrong ELF class: ELFCLASS64

It turns out SPARC needs the same LD_LIBRARY_PATH handling as x86.

This is what this patch does, at the same time noticing that the current duplication between lit.common.cfg.py and asan/Unit/lit.site.cfg.py.in isn't necessary.

Tested on sparc64-unknown-linux-gnu and x86_64-pc-linux-gnu.

When testing on Linux/sparc64 with a `runtimes` build, the
`UBSan-Standalone-sparc :: TestCases/Misc/Linux/sigaction.cpp` test
`FAIL`s:
```
runtimes/runtimes-bins/compiler-rt/test/ubsan/Standalone-sparc/TestCases/Misc/Linux/Output/sigaction.cpp.tmp: error while loading shared libraries: libclang_rt.ubsan_standalone.so: wrong ELF class: ELFCLASS64
```
It turns out SPARC needs the same `LD_LIBRARY_PATH` handling as x86.

This is what this patch does, at the same time noticing that the current
duplication between `lit.common.cfg.py` and `asan/Unit/lit.site.cfg.py.in`
isn't necessary.

Tested on `sparc64-unknown-linux-gnu` and `x86_64-pc-linux-gnu`.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 8, 2024

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

Author: Rainer Orth (rorth)

Changes

When testing on Linux/sparc64 with a runtimes build, the UBSan-Standalone-sparc :: TestCases/Misc/Linux/sigaction.cpp test FAILs:

runtimes/runtimes-bins/compiler-rt/test/ubsan/Standalone-sparc/TestCases/Misc/Linux/Output/sigaction.cpp.tmp: error while loading shared libraries: libclang_rt.ubsan_standalone.so: wrong ELF class: ELFCLASS64

It turns out SPARC needs the same LD_LIBRARY_PATH handling as x86.

This is what this patch does, at the same time noticing that the current duplication between lit.common.cfg.py and asan/Unit/lit.site.cfg.py.in isn't necessary.

Tested on sparc64-unknown-linux-gnu and x86_64-pc-linux-gnu.


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

2 Files Affected:

  • (modified) compiler-rt/test/asan/Unit/lit.site.cfg.py.in (-33)
  • (modified) compiler-rt/test/lit.common.cfg.py (+11-11)
diff --git a/compiler-rt/test/asan/Unit/lit.site.cfg.py.in b/compiler-rt/test/asan/Unit/lit.site.cfg.py.in
index ac652b53dcb9da..69313142ad58e4 100644
--- a/compiler-rt/test/asan/Unit/lit.site.cfg.py.in
+++ b/compiler-rt/test/asan/Unit/lit.site.cfg.py.in
@@ -8,25 +8,6 @@ import shlex
 # Load common config for all compiler-rt unit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/unittests/lit.common.unit.configured")
 
-def push_ld_library_path(config, new_path):
-  new_ld_library_path = os.path.pathsep.join(
-    (new_path, config.environment.get('LD_LIBRARY_PATH', '')))
-  config.environment['LD_LIBRARY_PATH'] = new_ld_library_path
-
-  if platform.system() == 'FreeBSD':
-    new_ld_32_library_path = os.path.pathsep.join(
-      (new_path, config.environment.get('LD_32_LIBRARY_PATH', '')))
-    config.environment['LD_32_LIBRARY_PATH'] = new_ld_32_library_path
-
-  if platform.system() == 'SunOS':
-    new_ld_library_path_32 = os.path.pathsep.join(
-      (new_path, config.environment.get('LD_LIBRARY_PATH_32', '')))
-    config.environment['LD_LIBRARY_PATH_32'] = new_ld_library_path_32
-
-    new_ld_library_path_64 = os.path.pathsep.join(
-      (new_path, config.environment.get('LD_LIBRARY_PATH_64', '')))
-    config.environment['LD_LIBRARY_PATH_64'] = new_ld_library_path_64
-
 # Setup config name.
 config.name = 'AddressSanitizer-Unit'
 
@@ -48,19 +29,5 @@ config.test_exec_root = os.path.join("@COMPILER_RT_BINARY_DIR@",
 
 config.test_source_root = config.test_exec_root
 
-# When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on, the initial value of
-# config.compiler_rt_libdir (COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR) has the
-# host triple as the trailing path component. The value is incorrect for i386
-# tests on x86_64 hosts and vice versa. Adjust config.compiler_rt_libdir
-# accordingly.
-if config.enable_per_target_runtime_dir:
-    if config.target_arch == 'i386':
-        config.compiler_rt_libdir = re.sub(r'/x86_64(?=-[^/]+$)', '/i386', config.compiler_rt_libdir)
-    elif config.target_arch == 'x86_64':
-        config.compiler_rt_libdir = re.sub(r'/i386(?=-[^/]+$)', '/x86_64', config.compiler_rt_libdir)
-
-# Set LD_LIBRARY_PATH to pick dynamic runtime up properly.
-push_ld_library_path(config, config.compiler_rt_libdir)
-
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
diff --git a/compiler-rt/test/lit.common.cfg.py b/compiler-rt/test/lit.common.cfg.py
index c533c7e9a70476..caf298b93ea493 100644
--- a/compiler-rt/test/lit.common.cfg.py
+++ b/compiler-rt/test/lit.common.cfg.py
@@ -161,18 +161,18 @@ def push_dynamic_library_lookup_path(config, new_path):
 
 # When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on, the initial value of
 # config.compiler_rt_libdir (COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR) has the
-# triple as the trailing path component. The value is incorrect for -m32/-m64.
-# Adjust config.compiler_rt accordingly.
+# host triple as the trailing path component. The value is incorrect for 32-bit
+# tests on 64-bit hosts and vice versa. Adjust config.compiler_rt_libdir
+# accordingly.
 if config.enable_per_target_runtime_dir:
-    if "-m32" in shlex.split(config.target_cflags):
-        config.compiler_rt_libdir = re.sub(
-            r"/x86_64(?=-[^/]+$)", "/i386", config.compiler_rt_libdir
-        )
-    elif "-m64" in shlex.split(config.target_cflags):
-        config.compiler_rt_libdir = re.sub(
-            r"/i386(?=-[^/]+$)", "/x86_64", config.compiler_rt_libdir
-        )
-
+    if config.target_arch == 'i386':
+        config.compiler_rt_libdir = re.sub(r'/x86_64(?=-[^/]+$)', '/i386', config.compiler_rt_libdir)
+    elif config.target_arch == 'x86_64':
+        config.compiler_rt_libdir = re.sub(r'/i386(?=-[^/]+$)', '/x86_64', config.compiler_rt_libdir)
+    if config.target_arch == 'sparc':
+        config.compiler_rt_libdir = re.sub(r'/sparcv9(?=-[^/]+$)', '/sparc', config.compiler_rt_libdir)
+    elif config.target_arch == 'sparcv9':
+        config.compiler_rt_libdir = re.sub(r'/sparc(?=-[^/]+$)', '/sparcv9', config.compiler_rt_libdir)
 
 # Check if the test compiler resource dir matches the local build directory
 # (which happens with -DLLVM_ENABLE_PROJECTS=clang;compiler-rt) or if we are

Copy link

github-actions bot commented Oct 8, 2024

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r a3a253d3c7780977077dd46493917b1949c0166d...2f35de6675746ac94e742e0925f61ffd20abe500 compiler-rt/test/lit.common.cfg.py
View the diff from darker here.
--- lit.common.cfg.py	2024-10-08 08:20:42.000000 +0000
+++ lit.common.cfg.py	2024-10-08 08:25:19.371346 +0000
@@ -163,18 +163,26 @@
 # config.compiler_rt_libdir (COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR) has the
 # host triple as the trailing path component. The value is incorrect for 32-bit
 # tests on 64-bit hosts and vice versa. Adjust config.compiler_rt_libdir
 # accordingly.
 if config.enable_per_target_runtime_dir:
-    if config.target_arch == 'i386':
-        config.compiler_rt_libdir = re.sub(r'/x86_64(?=-[^/]+$)', '/i386', config.compiler_rt_libdir)
-    elif config.target_arch == 'x86_64':
-        config.compiler_rt_libdir = re.sub(r'/i386(?=-[^/]+$)', '/x86_64', config.compiler_rt_libdir)
-    if config.target_arch == 'sparc':
-        config.compiler_rt_libdir = re.sub(r'/sparcv9(?=-[^/]+$)', '/sparc', config.compiler_rt_libdir)
-    elif config.target_arch == 'sparcv9':
-        config.compiler_rt_libdir = re.sub(r'/sparc(?=-[^/]+$)', '/sparcv9', config.compiler_rt_libdir)
+    if config.target_arch == "i386":
+        config.compiler_rt_libdir = re.sub(
+            r"/x86_64(?=-[^/]+$)", "/i386", config.compiler_rt_libdir
+        )
+    elif config.target_arch == "x86_64":
+        config.compiler_rt_libdir = re.sub(
+            r"/i386(?=-[^/]+$)", "/x86_64", config.compiler_rt_libdir
+        )
+    if config.target_arch == "sparc":
+        config.compiler_rt_libdir = re.sub(
+            r"/sparcv9(?=-[^/]+$)", "/sparc", config.compiler_rt_libdir
+        )
+    elif config.target_arch == "sparcv9":
+        config.compiler_rt_libdir = re.sub(
+            r"/sparc(?=-[^/]+$)", "/sparcv9", config.compiler_rt_libdir
+        )
 
 # Check if the test compiler resource dir matches the local build directory
 # (which happens with -DLLVM_ENABLE_PROJECTS=clang;compiler-rt) or if we are
 # using an installed clang to test compiler-rt standalone. In the latter case
 # we may need to override the resource dir to match the path of the just-built

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.

2 participants