[compiler-rt] Handle target and sysroot flags in tests

[phosek]

Summary

Pass the target and sysroot flags from the build to lit and make sure
they are included in the test cflags.

[yln]

Looks sensible to me, but I don't understand the implications here.

Can you explain why this is needed/what doesn't work right now?
@phosek

[phosek]

In D126936#3556967, @yln wrote:

Looks sensible to me, but I don't understand the implications here.

Can you explain why this is needed/what doesn't work right now?
@phosek

In our case we build against an explicit sysroot or SDK, even when the host matches the target. This is to ensure that we don't accidentally introduce dependency on libraries (or their versions) that are only present on certain hosts. We noticed recently that test binaries don't follow the CMake configuration when we saw test failures that only affected some hosts. The goal of this change is to make the behavior CMake and lit more consistent.

[delcypher]

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

699

I haven't verified this but I think this very likely to break testing for Apple platforms. I don't think config.target_triple is set correctly. IIRC we rely on the target triple being inferred from the SDK and arch being used. This isn't a good situation to be in, explicit would definitely be better.

701

We should check but I doubt this is configured correctly. It's probably correct for macOS but broken for other Apple platforms because I suspect config.sysroot might always be set to a macOS SDK path.

[delcypher]

The change is right in "spirit" but I think the current implementation will likely break things (or will work by accident). There is a fundamental difference between how compiler-rt is built for Apple platforms and other platforms. For Apple platforms we build all platforms and targets in a single CMake configure (IIUC other platforms have a CMake configure correspond to a single platform and arch). So there is no single sysroot or target triple.

The way this is currently handled for the the lit testing is we generate a lit test suite for each OS and architecture we want, e.g. from a template like

https://github.com/llvm/llvm-project/blob/main/compiler-rt/test/asan/lit.site.cfg.py.in

I think what the patch does here is fine to be the "default" (although I might prefer it to be set to something that causes on error by default on Apple platforms) but we need to make sure for all the generated testsuites that we override config.target_triple and config.sysroot to be the right thing.

We might also want to remote the sysroot from being set in the target_cflags because we already do that e.g.

# Autogenerated from /Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/test/asan/lit.site.cfg.py.in
# Do not edit!

# Allow generated file to be relocatable.
from pathlib import Path
def path(p):
    if not p: return ''
    return str((Path(__file__).parent / p).resolve())


# Tool-specific config options.
config.name_suffix = "-arm64-ios"
config.target_cflags = "-arch arm64 -stdlib=libc++ -miphoneos-version-min=9.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.4.sdk -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta "
config.clang = "/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug/./bin/clang"
config.bits = "64"
config.arm_thumb = ""
config.apple_platform = "ios"
config.apple_platform_min_deployment_target_flag = "-miphoneos-version-min"
config.asan_dynamic = True
config.target_arch = "arm64"

...

We might want to create an Apple "platform/arch" specific "lit.common.configured.in" files that the existing testsuites (e.g. ASan) can include so we don't have to duplicate the logic across all the different test suites.

compiler-rt/test/lit.common.configured.in

11

This will be the sysroot for macOS which is the wrong sysroot for all other Apple platforms.

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

 else:
   config.available_features.add("memprof-shadow-scale-3")
 
 if config.expensive_checks:
   config.available_features.add("expensive_checks")
 
 # Propagate the LLD/LTO into the clang config option, so nothing else is needed.
 run_wrapper = []
-target_cflags = [getattr(config, 'target_cflags', None)]
+target_cflags = []
 extra_cflags = []
 
+# Handle target triple and sysroot.
+if config.target_triple:
+  extra_cflags += ["--target=%s" % config.target_triple]
+if config.sysroot:
+  extra_cflags += ["--sysroot=%s" % config.sysroot]
+if config.osx_sysroot:
+  extra_cflags += ["-isysroot %s" % config.osx_sysroot]
+
+target_cflags.append(getattr(config, 'target_cflags', None))
+
 if config.use_lto and config.lto_supported:
   extra_cflags += config.lto_flags
 elif config.use_lto and (not config.lto_supported):
   config.unsupported = True
 
 if config.use_lld and config.has_lld and not config.use_lto:
   extra_cflags += ["-fuse-ld=lld"]
 elif config.use_lld and (not config.has_lld):

compiler-rt/test/lit.common.configured.in

 @LIT_SITE_CFG_IN_HEADER@
 
 # Set attribute value if it is unset.
 def set_default(attr, value):
   if not getattr(config, attr, None):
     setattr(config, attr, value)
 
 # Generic config options for all compiler-rt lit tests.
 set_default("target_triple", "@COMPILER_RT_DEFAULT_TARGET_TRIPLE@")
 set_default("target_cflags", "@COMPILER_RT_TEST_COMPILER_CFLAGS@")
+set_default("sysroot", "@CMAKE_SYSROOT@")
+set_default("osx_sysroot", "@CMAKE_OSX_SYSROOT@")
 set_default("host_arch", "@HOST_ARCH@")
 set_default("target_arch", "@COMPILER_RT_DEFAULT_TARGET_ARCH@")
 set_default("host_os", "@HOST_OS@")
 set_default("llvm_build_mode", "@LLVM_BUILD_MODE@")
 set_default("llvm_src_root", "@LLVM_MAIN_SRC_DIR@")
 set_default("llvm_obj_root", "@LLVM_BINARY_DIR@")
 set_default("compiler_rt_src_root", "@COMPILER_RT_SOURCE_DIR@")
 set_default("compiler_rt_obj_root", "@COMPILER_RT_BINARY_DIR@")