[test] Fix asan/scudo -shared-libsan tests with -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on

[MaskRay]

Summary

On x86_64-unknown-linux-gnu, -m32 tests set LD_LIBRARY_PATH to
config.compiler_rt_libdir ($build/lib/clang/14.0.0/lib/x86_64-unknown-linux-gnu)
instead of i386-unknown-linux-gnu, so -shared-libsan executables
cannot find their runtime (e.g. TestCases/replaceable_new_delete.cpp).

Detect -m32 and -m64 in config.target_cflags, and adjust config.compiler_rt_libdir.

[delcypher]

I'm not a fan of this change because it breaks the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS feature in a really non-intuitive way because the presence of the -m32 or -m64 flags causes the value of COMPILER_RT_TEST_STANDALONE_BUILD_LIBS to be ignored.

Given that it looks like you're trying to patch the value of config.compiler_rt_libdir it suggests to me that this isn't being set correctly in the CMake code. The initial value of config.compiler_rt_libdir (i.e. before this lit configuration file tries to change it) should always point to the directory where the build system will place those files. Can we fix your issue using that approach instead?

Seeing as you said you didn't understand the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS option in https://reviews.llvm.org/D101681 here's a quick summary.

The whole point of the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS option is to make it explicit which libraries to test when doing a standalone build of compiler-rt. There are two choices when doing a standalone build,

• the runtime libraries that are part of the toolchain used for the CMake configure of compiler-rt (COMPILER_RT_TEST_STANDALONE_BUILD_LIBS=OFF). This made is very useful for testing arbitrary toolchains (i.e. you can take an arbitrary build of clang and run the Sanitizer test suites against that build of clang). In this mode we don't actually build any libraries, we are assuming they've already been built and are included in the toolchain used by the CMake configure.

OR

• the runtime libraries built by the standalone build of compiler-rt (COMPILER_RT_TEST_STANDALONE_BUILD_LIBS=ON).

In non standalone configurations (the common case) these happen to be the same. However, in the standalone case it's possible for these to be different. The compiler-rt test suites don't really handle this distinction very well:

• The majority of the Sanitizer tests assume clang knows where to find the library so changing config.compiler_rt_libdir has no effect. So these test suites don't support COMPILER_RT_TEST_STANDALONE_BUILD_LIBS=ON, currently a warning is emitted when that's the case but it's supposed to be an error.
• The builtin tests explicitly tell clang where to find the library by using config.compiler_rt_libdir. So these test suites support COMPILER_RT_TEST_STANDALONE_BUILD_LIBS being set to either value.

compiler-rt/test/lit.common.cfg.py

58–60

Why do you need to add config.target_c_flags here?.

get_path_from_clang() already sets the target triple. Isn't that enough?

[MaskRay]

In D108859#2970532, @delcypher wrote:

I'm not a fan of this change because it breaks the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS feature in a really non-intuitive way because the presence of the -m32 or -m64 flags causes the value of COMPILER_RT_TEST_STANDALONE_BUILD_LIBS to be ignored.

Given that it looks like you're trying to patch the value of config.compiler_rt_libdir it suggests to me that this isn't being set correctly in the CMake code. The initial value of config.compiler_rt_libdir (i.e. before this lit configuration file tries to change it) should always point to the directory where the build system will place those files. Can we fix your issue using that approach instead?

The Linux multilib scheme tests both i386-unknown-linux-gnu and x86_64-unknown-linux-gnu on x86_64.

compiler-rt/unittests/lit.common.unit.configured.in sets config.compiler_rt_libdir = "@COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR@".

COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR has only one value. how to make config.compiler_rt_libdir $build/lib/clang/14.0.0/lib/i386-unknown-linux-gnu for $build/projects/compiler-rt/test/asan/I386LinuxConfig/lit.site.cfg.py?

[MaskRay]

compiler-rt/test/lit.common.cfg.py

58–60

config.target_cflags is needed to get -m32.

-m32 (not included by get_path_from_clang()) can effectively change the --target= specified triple.

[MaskRay]

Ping @delcypher

[MaskRay]

In D108859#2975557, @MaskRay wrote:

Ping @delcypher

Ping @delcypher

[MaskRay]

Ping other reviewers.

[delcypher]

In D108859#2970538, @MaskRay wrote:

In D108859#2970532, @delcypher wrote:

I'm not a fan of this change because it breaks the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS feature in a really non-intuitive way because the presence of the -m32 or -m64 flags causes the value of COMPILER_RT_TEST_STANDALONE_BUILD_LIBS to be ignored.

Given that it looks like you're trying to patch the value of config.compiler_rt_libdir it suggests to me that this isn't being set correctly in the CMake code. The initial value of config.compiler_rt_libdir (i.e. before this lit configuration file tries to change it) should always point to the directory where the build system will place those files. Can we fix your issue using that approach instead?

The Linux multilib scheme tests both i386-unknown-linux-gnu and x86_64-unknown-linux-gnu on x86_64.

Is that something that's actually supported? The only compiler-rt configuration that supports building and testing multiple architectures in the same build that I know of is Apple's build. I think we need to establish if what you're doing is actually supported.

compiler-rt/unittests/lit.common.unit.configured.in sets config.compiler_rt_libdir = "@COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR@".

COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR has only one value. how to make config.compiler_rt_libdir $build/lib/clang/14.0.0/lib/i386-unknown-linux-gnu for $build/projects/compiler-rt/test/asan/I386LinuxConfig/lit.site.cfg.py?

Looking at how config.compiler_rt_libdir gets set now leads to

function(configure_compiler_rt_lit_site_cfg input output)
  set_llvm_build_mode()

  get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} output_dir)

  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_TEST_COMPILER ${COMPILER_RT_TEST_COMPILER})
  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR ${output_dir})

  configure_lit_site_cfg(${input} ${output})
endfunction()

There are a few going on here:

• get_compiler_rt_output_dir() sets the value for output_dir
• COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR is set to be a modified version of output_dir that supports multi configuration builds. setup_llvm_build_mode() sets LLVM_BUILD_MODE set_llvm_build_mode() will cause LLVM_BUILD_MODE to be set to . or %(build_mode)s. The %(build_mode)s substitution relies on the doing the actually substitution when the lit test suite configuration happens (i.e. config.compiler_rt_libdir = config.compiler_rt_libdir % lit_config.params) see https://reviews.llvm.org/D38471. This complexity exists because at CMake configure time there isn't a single build configuration.

For your particular problem, I think the problem here is that configure_compiler_rt_lit_site_cfg hardcodes the architecture to be COMPILER_RT_DEFAULT_TARGET_ARCH. What you probably want to do is change the configure_compiler_rt_lit_site_cfg function to take an architecture argument so at the use sites of configure_compiler_rt_lit_site_cfg can specify the architecture if you are generating multiple test suites for different architectures.

This on it's own doesn't look like its enough because the function doesn't appear to go over architectures in a loop

configure_compiler_rt_lit_site_cfg(
  ${CMAKE_CURRENT_SOURCE_DIR}/lit.common.unit.configured.in
  ${CMAKE_CURRENT_BINARY_DIR}/lit.common.unit.configured)

So it's unclear to me how you're having multiple lit configurations generated for each architecture in a single build.

If you're not having multiple lit configurations (i.e. you're only building for i386) then looking at the CMake code suggests you need to change COMPILER_RT_DEFAULT_TARGET_ARCH. I think configuring with -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=i386-unknown-linux-gnu should set COMPILER_RT_DEFAULT_TARGET_ARCH appropriately.

[MaskRay]

In D108859#2992539, @delcypher wrote:

In D108859#2970538, @MaskRay wrote:

In D108859#2970532, @delcypher wrote:

I'm not a fan of this change because it breaks the COMPILER_RT_TEST_STANDALONE_BUILD_LIBS feature in a really non-intuitive way because the presence of the -m32 or -m64 flags causes the value of COMPILER_RT_TEST_STANDALONE_BUILD_LIBS to be ignored.

Given that it looks like you're trying to patch the value of config.compiler_rt_libdir it suggests to me that this isn't being set correctly in the CMake code. The initial value of config.compiler_rt_libdir (i.e. before this lit configuration file tries to change it) should always point to the directory where the build system will place those files. Can we fix your issue using that approach instead?

The Linux multilib scheme tests both i386-unknown-linux-gnu and x86_64-unknown-linux-gnu on x86_64.

Is that something that's actually supported? The only compiler-rt configuration that supports building and testing multiple architectures in the same build that I know of is Apple's build. I think we need to establish if what you're doing is actually supported.

It's supported. On an x86_64 Linux, including compiler-rt in LLVM_ENABLE_PROJECTS will test both x86_64 and i386, probably when the host compiler supports -m32.
This is the multilib scheme which is the default on Debian/Ubuntu and their derivatives.
Supporting -m32 is fairly common for other Linux distributions.

Actually I believe -m32 on x86_64 is the only way sanitizers's i386 ports are currently tested on build bots. AFAIK there is no dedicate build bot running i386 Linux.

The be clear, I just want to fix the tests so that D107799 can be re-commited.
The fact that these i386-unknown-linux-gnu tests (through -m32) actually run is the motivation I have to fix this.

If you're not having multiple lit configurations (i.e. you're only building for i386) then looking at the CMake code suggests you need to change COMPILER_RT_DEFAULT_TARGET_ARCH. I think configuring with -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=i386-unknown-linux-gnu should set COMPILER_RT_DEFAULT_TARGET_ARCH appropriately.

No. The Linux setup doesn't need to touch any of these -DCOMPILER_RT_*.

Running two builds with different -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE is not desired on Linux x86_64.

I don't know what macOS does on this front, but what I said is the Linux x86_64 reality.

[MaskRay]

Ping

[MaskRay]

@delcypher If COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is really a macOS thing, please give ELF users a way to proceed.

[delcypher]

In D108859#3000110, @MaskRay wrote:

@delcypher If COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is really a macOS thing, please give ELF users a way to proceed.

COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is not a macOS specific thing at all.

To proceed you need to either

Investigate fixing COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR to be set correctly in CMake for your configuration so that when the lit configuration file is generated it is correct for your configuration. This would be my preference because everything you've said so far suggests this value is wrong for your configuration, so fixing it the wrong information at its source seems like the right fix.

OR if that isn't possible (please explain why)

Change compiler-rt/test/lit.common.cfg.py to modify config.compiler_rt_libdir but do that before the logic under # Ask the compiler for the path to libraries it is going to use... runs so that config.test_standalone_build_libs is respected regardless of what is in config.target_c_flags. Your current patch breaks the semantics of config.test_standalone_build_libs and also makes the code harder to read which is why I don't approve of the patch in its current form.

[MaskRay]

better fix

[MaskRay]

When the build directory is /tmp/RelA, /tmp/RelA/projects/compiler-rt/test/lit.common.configured says config.compiler_rt_libdir = "/tmp/RelA/./lib/clang/14.0.0/lib/x86_64-unknown-linux-gnu"

The x86_64 asan archive is at /tmp/RelA/lib/clang/14.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.a and the i386 archive is at /tmp/RelA/./lib/clang/14.0.0/lib/i386-unknown-linux-gnu/libclang_rt.asan.a.

COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR is the native library directory "/tmp/RelA/./lib/clang/14.0.0/lib/x86_64-unknown-linux-gnu".
For multilib -m32, adjusting the trailing path component seems to make the most sense.

---

I cannot find any useful documentation about "standalone build" but I guess I figured it out:

# my ~/llvm is llvm-project
% cd /tmp/out/play
% cmake -GNinja -H~/llvm/compiler-rt -B/tmp/out/play -DCMAKE_C_COMPILER=/tmp/RelA/bin/clang -DCMAKE_CXX_COMPILER=/tmp/RelA/bin/clang++ -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu -DCOMPILER_RT_INCLUDE_TESTS=on -DCOMPILER_RT_TEST_STANDALONE_BUILD_LIBS=on
% ninja check-asan

All tests passed. So this patch makes -DCOMPILER_RT_TEST_STANDALONE_BUILD_LIBS=on work as well, though I don't understand why ... we want to support the extra (standalone) configuration mode for tests.
Test a clang built at slightly different commit to test runtime stability?

For some reason I need to specify -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu, otherwise it would be incorrect (x86_64-pc-linux-gnu).

[vitalybuka]

I don't understand why ... we want to support the extra (standalone) configuration mode for tests.

GCC uses these libs as well

compiler-rt/test/lit.common.cfg.py

except subprocess.CalledProcessError as e:
       if allow_failure:
         lit_config.warning(msg)
       else:
         lit_config.fatal(msg)
     return path, clang_cmd
 
   # Try using `-print-runtime-dir`. This is only supported by very new versions of Clang.
   # so allow failure here.
-  runtime_dir, clang_cmd = get_path_from_clang(['-print-runtime-dir'], allow_failure=True)
+  runtime_dir, clang_cmd = get_path_from_clang(shlex.split(config.target_cflags)
+                                               + ['-print-runtime-dir'],
+                                               allow_failure=True)
   if runtime_dir:
     if os.path.exists(runtime_dir):
       return os.path.realpath(runtime_dir)
     # TODO(dliew): This should be a fatal error but it seems to trip the `llvm-clang-win-x-aarch64`
     # bot which is likely misconfigured
     lit_config.warning(
       f'Path reported by clang does not exist: \"{runtime_dir}\". '
       f'This path was found by running {clang_cmd}.'

 elif compiler_id == 'GNU':
   config.cxx_mode_flags = ["-x c++"]
   config.debug_info_flags = ["-g"]
 else:
   lit_config.fatal("Unsupported compiler id: %r" % compiler_id)
 # Add compiler ID to the list of available features.
 config.available_features.add(compiler_id)
 
+# 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.
+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)
+
 # Ask the compiler for the path to libraries it is going to use. If this
 # doesn't match config.compiler_rt_libdir then it means we might be testing the
 # compiler's own runtime libraries rather than the ones we just built.
 # Warn about about this and handle appropriately.
 compiler_libdir = find_compiler_libdir()
 if compiler_libdir:
   compiler_rt_libdir_real = os.path.realpath(config.compiler_rt_libdir)
   if compiler_libdir != compiler_rt_libdir_real: