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

[asan][cmake][test] Fix finding dynamic asan runtime lib #100083

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Jul 23, 2024

In a runtimes build on Solaris/amd64, there are two failues:

  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest

This happens when lit enumerates the tests with --gtest_list_tests --gtest_filter=-*DISABLED_*. The error is twofold:

  • The LD_LIBRARY_PATH* variables point at the 64-bit directory (lib/clang/19/lib/x86_64-pc-solaris2.11) for a 32-bit test:
    ld.so.1: Asan-i386-calls-Dynamic-Test: fatal: /var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so: wrong ELF class: ELFCLASS64  
    
  • While the tests are linked with -Wl,-rpath, that path always is the 64-bit directory again.

Accordingly, the fix consists of two parts:

  • The code in compiler-rt/test/asan/Unit/lit.site.cfg.py.in to adjust the LD_LIBRARY_PATH* variables is guarded by a config.target_arch != config.host_arch condition. This is wrong in two ways:
    • The adjustment is always needed independent of the host arch. This is what compiler-rt/test/lit.common.cfg.py already does.
    • Besides, config.host_arch is ultimately set from CMAKE_HOST_SYSTEM_PROCESSOR. On Linux/x86_64, this is x86_64 (uname -m) while on Solaris/amd64 it's i386 (uname -p), explaining why the transformation is skipped on Solaris, but not on Linux.
  • Besides, RPATH needs to be set to the correct subdirectory, so instead of using the default arch in compiler-rt/CMakeLists.txt, this patch moves the code to a function which takes the test's arch into account.

Tested on amd64-pc-solaris2.11 and x86_64-pc-linux-gnu.

In a `runtimes` build on Solaris/amd64, there are two failues:
```
  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest
```
This happens when `lit` enumerates the tests with `--gtest_list_tests
--gtest_filter=-*DISABLED_*`.  The error is twofold:

- The `LD_LIBRARY_PATH*` variables point at the 64-bit directory
  (`lib/clang/19/lib/x86_64-pc-solaris2.11`) for a 32-bit test:
  ```
  ld.so.1: Asan-i386-calls-Dynamic-Test: fatal: /var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so: wrong ELF class: ELFCLASS64
  ```
- While the tests are linked with `-Wl,-rpath`, that path always is the
  64-bit directory again.

Accordingly, the fix consists of two parts:
- The code in `compiler-rt/test/asan/Unit/lit.site.cfg.py.in` to adjust the
  `LD_LIBRARY_PATH*` variables is guarded by a `config.target_arch !=
  config.host_arch` condition.  This is wrong in two ways:
  - The adjustment is always needed independent of the host arch.  This is
    what `compiler-rt/test/lit.common.cfg.py` already does.
  - Besides, `config.host_arch` is ultimately set from
    `CMAKE_HOST_SYSTEM_PROCESSOR`.  On Linux/x86_64, this is `x86_64`
    (`uname -m`) while on Solaris/amd64 it's `i386` (`uname -p`),
    explaining why the transformation is skipped on Solaris, but not on Linux.
- Besides, `RPATH` needs to be set to the correct subdirectory, so instead
  of using the default arch in `compiler-rt/CMakeLists.txt`, this patch
  moves the code to a function which takes the test's arch into account.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 23, 2024

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

Author: Rainer Orth (rorth)

Changes

In a runtimes build on Solaris/amd64, there are two failues:

  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest

This happens when lit enumerates the tests with --gtest_list_tests --gtest_filter=-*DISABLED_*. The error is twofold:

  • The LD_LIBRARY_PATH* variables point at the 64-bit directory (lib/clang/19/lib/x86_64-pc-solaris2.11) for a 32-bit test:
    ld.so.1: Asan-i386-calls-Dynamic-Test: fatal: /var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so: wrong ELF class: ELFCLASS64  
    
  • While the tests are linked with -Wl,-rpath, that path always is the 64-bit directory again.

Accordingly, the fix consists of two parts:

  • The code in compiler-rt/test/asan/Unit/lit.site.cfg.py.in to adjust the LD_LIBRARY_PATH* variables is guarded by a config.target_arch != config.host_arch condition. This is wrong in two ways:
    • The adjustment is always needed independent of the host arch. This is what compiler-rt/test/lit.common.cfg.py already does.
    • Besides, config.host_arch is ultimately set from CMAKE_HOST_SYSTEM_PROCESSOR. On Linux/x86_64, this is x86_64 (uname -m) while on Solaris/amd64 it's i386 (uname -p), explaining why the transformation is skipped on Solaris, but not on Linux.
  • Besides, RPATH needs to be set to the correct subdirectory, so instead of using the default arch in compiler-rt/CMakeLists.txt, this patch moves the code to a function which takes the test's arch into account.

Tested on amd64-pc-solaris2.11 and x86_64-pc-linux-gnu.


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

4 Files Affected:

  • (modified) compiler-rt/CMakeLists.txt (-4)
  • (modified) compiler-rt/cmake/config-ix.cmake (+13)
  • (modified) compiler-rt/lib/asan/tests/CMakeLists.txt (+3-1)
  • (modified) compiler-rt/test/asan/Unit/lit.site.cfg.py.in (+1-1)
diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index 65063e0057bbc..96e432e2c97a7 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -604,10 +604,6 @@ if (COMPILER_RT_TEST_STANDALONE_BUILD_LIBS)
   if ("${COMPILER_RT_TEST_COMPILER_ID}" MATCHES "Clang")
     list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS "-resource-dir=${COMPILER_RT_OUTPUT_DIR}")
   endif()
-  get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} rtlib_dir)
-  if (NOT WIN32)
-    list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS "-Wl,-rpath,${rtlib_dir}")
-  endif()
 endif()
 
 if(COMPILER_RT_USE_LLVM_UNWINDER)
diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 3a151772e268a..dad557af2ae8c 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -267,6 +267,19 @@ function(get_target_link_flags_for_arch arch out_var)
   endif()
 endfunction()
 
+# Returns a list of architecture specific dynamic ldflags in @out_var list.
+function(get_dynamic_link_flags_for_arch arch out_var)
+  list(FIND COMPILER_RT_SUPPORTED_ARCH ${arch} ARCH_INDEX)
+  if(ARCH_INDEX EQUAL -1)
+    message(FATAL_ERROR "Unsupported architecture: ${arch}")
+  else()
+    get_compiler_rt_output_dir(${arch} rtlib_dir)
+    if (NOT WIN32)
+      set(${out_var} "-Wl,-rpath,${rtlib_dir}" PARENT_SCOPE)
+    endif()
+  endif()
+endfunction()
+
 # Returns a compiler and CFLAGS that should be used to run tests for the
 # specific architecture.  When cross-compiling, this is controled via
 # COMPILER_RT_TEST_COMPILER and COMPILER_RT_TEST_COMPILER_CFLAGS.
diff --git a/compiler-rt/lib/asan/tests/CMakeLists.txt b/compiler-rt/lib/asan/tests/CMakeLists.txt
index 7abd4c89ac6bc..b489bb99aeff3 100644
--- a/compiler-rt/lib/asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/asan/tests/CMakeLists.txt
@@ -206,13 +206,15 @@ function(add_asan_tests arch test_runtime)
           -Wl,-nodefaultlib:libcmt,-defaultlib:msvcrt,-defaultlib:oldnames
         )
     else()
+      set(DYNAMIC_LINK_FLAGS)
+      get_dynamic_link_flags_for_arch(${arch} DYNAMIC_LINK_FLAGS)
 
       # Otherwise, reuse ASAN_INST_TEST_OBJECTS.
       add_compiler_rt_test(AsanDynamicUnitTests "${dynamic_test_name}" "${arch}"
         SUBDIR "${CONFIG_NAME_DYNAMIC}"
         OBJECTS ${ASAN_INST_TEST_OBJECTS}
         DEPS asan ${ASAN_INST_TEST_OBJECTS}
-        LINK_FLAGS ${ASAN_DYNAMIC_UNITTEST_INSTRUMENTED_LINK_FLAGS} ${TARGET_LINK_FLAGS}
+        LINK_FLAGS ${ASAN_DYNAMIC_UNITTEST_INSTRUMENTED_LINK_FLAGS} ${TARGET_LINK_FLAGS} ${DYNAMIC_LINK_FLAGS}
         )
     endif()
   endif()
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 638e1dedfc1d2..ac652b53dcb9d 100644
--- a/compiler-rt/test/asan/Unit/lit.site.cfg.py.in
+++ b/compiler-rt/test/asan/Unit/lit.site.cfg.py.in
@@ -53,7 +53,7 @@ config.test_source_root = config.test_exec_root
 # 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 and config.target_arch != config.host_arch:
+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':

@rorth rorth added the cmake Build system in general and CMake in particular label Jul 23, 2024
Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

This looks correct to me and should also fix similar failures that I've seen on FreeBSD before. Please wait for another positive review before meeting though.

@rorth
Copy link
Collaborator Author

rorth commented Jul 23, 2024

Will do, thanks. I'll file an Issue in the meantime: I'd like to get this into the 19.x branch if possible.

@rorth rorth merged commit c34d673 into llvm:main Jul 24, 2024
11 checks passed
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 24, 2024
In a `runtimes` build on Solaris/amd64, there are two failues:
```
  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest
```
This happens when `lit` enumerates the tests with `--gtest_list_tests
--gtest_filter=-*DISABLED_*`. The error is twofold:

- The `LD_LIBRARY_PATH*` variables point at the 64-bit directory
(`lib/clang/19/lib/x86_64-pc-solaris2.11`) for a 32-bit test:
  ```
ld.so.1: Asan-i386-calls-Dynamic-Test: fatal:
/var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so:
wrong ELF class: ELFCLASS64
  ```
- While the tests are linked with `-Wl,-rpath`, that path always is the
64-bit directory again.

Accordingly, the fix consists of two parts:
- The code in `compiler-rt/test/asan/Unit/lit.site.cfg.py.in` to adjust
the `LD_LIBRARY_PATH*` variables is guarded by a `config.target_arch !=
config.host_arch` condition. This is wrong in two ways:
- The adjustment is always needed independent of the host arch. This is
what `compiler-rt/test/lit.common.cfg.py` already does.
- Besides, `config.host_arch` is ultimately set from
`CMAKE_HOST_SYSTEM_PROCESSOR`. On Linux/x86_64, this is `x86_64` (`uname
-m`) while on Solaris/amd64 it's `i386` (`uname -p`), explaining why the
transformation is skipped on Solaris, but not on Linux.
- Besides, `RPATH` needs to be set to the correct subdirectory, so
instead of using the default arch in `compiler-rt/CMakeLists.txt`, this
patch moves the code to a function which takes the test's arch into
account.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

(cherry picked from commit c34d673)
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
In a `runtimes` build on Solaris/amd64, there are two failues:
```
  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest
```
This happens when `lit` enumerates the tests with `--gtest_list_tests
--gtest_filter=-*DISABLED_*`. The error is twofold:

- The `LD_LIBRARY_PATH*` variables point at the 64-bit directory
(`lib/clang/19/lib/x86_64-pc-solaris2.11`) for a 32-bit test:
  ``` 
ld.so.1: Asan-i386-calls-Dynamic-Test: fatal:
/var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so:
wrong ELF class: ELFCLASS64
  ```
- While the tests are linked with `-Wl,-rpath`, that path always is the
64-bit directory again.

Accordingly, the fix consists of two parts:
- The code in `compiler-rt/test/asan/Unit/lit.site.cfg.py.in` to adjust
the `LD_LIBRARY_PATH*` variables is guarded by a `config.target_arch !=
config.host_arch` condition. This is wrong in two ways:
- The adjustment is always needed independent of the host arch. This is
what `compiler-rt/test/lit.common.cfg.py` already does.
- Besides, `config.host_arch` is ultimately set from
`CMAKE_HOST_SYSTEM_PROCESSOR`. On Linux/x86_64, this is `x86_64` (`uname
-m`) while on Solaris/amd64 it's `i386` (`uname -p`), explaining why the
transformation is skipped on Solaris, but not on Linux.
- Besides, `RPATH` needs to be set to the correct subdirectory, so
instead of using the default arch in `compiler-rt/CMakeLists.txt`, this
patch moves the code to a function which takes the test's arch into
account.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250675
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 29, 2024
In a `runtimes` build on Solaris/amd64, there are two failues:
```
  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest
```
This happens when `lit` enumerates the tests with `--gtest_list_tests
--gtest_filter=-*DISABLED_*`. The error is twofold:

- The `LD_LIBRARY_PATH*` variables point at the 64-bit directory
(`lib/clang/19/lib/x86_64-pc-solaris2.11`) for a 32-bit test:
  ```
ld.so.1: Asan-i386-calls-Dynamic-Test: fatal:
/var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so:
wrong ELF class: ELFCLASS64
  ```
- While the tests are linked with `-Wl,-rpath`, that path always is the
64-bit directory again.

Accordingly, the fix consists of two parts:
- The code in `compiler-rt/test/asan/Unit/lit.site.cfg.py.in` to adjust
the `LD_LIBRARY_PATH*` variables is guarded by a `config.target_arch !=
config.host_arch` condition. This is wrong in two ways:
- The adjustment is always needed independent of the host arch. This is
what `compiler-rt/test/lit.common.cfg.py` already does.
- Besides, `config.host_arch` is ultimately set from
`CMAKE_HOST_SYSTEM_PROCESSOR`. On Linux/x86_64, this is `x86_64` (`uname
-m`) while on Solaris/amd64 it's `i386` (`uname -p`), explaining why the
transformation is skipped on Solaris, but not on Linux.
- Besides, `RPATH` needs to be set to the correct subdirectory, so
instead of using the default arch in `compiler-rt/CMakeLists.txt`, this
patch moves the code to a function which takes the test's arch into
account.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

(cherry picked from commit c34d673)
Harini0924 pushed a commit to Harini0924/llvm-project that referenced this pull request Aug 1, 2024
In a `runtimes` build on Solaris/amd64, there are two failues:
```
  AddressSanitizer-Unit :: ./Asan-i386-calls-Dynamic-Test/failed_to_discover_tests_from_gtest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Dynamic-Test/failed_to_discover_tests_from_gtest
```
This happens when `lit` enumerates the tests with `--gtest_list_tests
--gtest_filter=-*DISABLED_*`. The error is twofold:

- The `LD_LIBRARY_PATH*` variables point at the 64-bit directory
(`lib/clang/19/lib/x86_64-pc-solaris2.11`) for a 32-bit test:
  ``` 
ld.so.1: Asan-i386-calls-Dynamic-Test: fatal:
/var/llvm/local-amd64-release-stage2-A-flang-clang18-runtimes/tools/clang/stage2-bins/./lib/../lib/clang/19/lib/x86_64-pc-solaris2.11/libclang_rt.asan.so:
wrong ELF class: ELFCLASS64
  ```
- While the tests are linked with `-Wl,-rpath`, that path always is the
64-bit directory again.

Accordingly, the fix consists of two parts:
- The code in `compiler-rt/test/asan/Unit/lit.site.cfg.py.in` to adjust
the `LD_LIBRARY_PATH*` variables is guarded by a `config.target_arch !=
config.host_arch` condition. This is wrong in two ways:
- The adjustment is always needed independent of the host arch. This is
what `compiler-rt/test/lit.common.cfg.py` already does.
- Besides, `config.host_arch` is ultimately set from
`CMAKE_HOST_SYSTEM_PROCESSOR`. On Linux/x86_64, this is `x86_64` (`uname
-m`) while on Solaris/amd64 it's `i386` (`uname -p`), explaining why the
transformation is skipped on Solaris, but not on Linux.
- Besides, `RPATH` needs to be set to the correct subdirectory, so
instead of using the default arch in `compiler-rt/CMakeLists.txt`, this
patch moves the code to a function which takes the test's arch into
account.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular compiler-rt:asan Address sanitizer compiler-rt:sanitizer compiler-rt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants