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

[Dan Liew via Phabricator via llvm-commits]

delcypher requested changes to this revision.
delcypher added a comment.
This revision now requires changes to proceed.

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.



================
Comment at: compiler-rt/test/lit.common.configured.in:11
 set_default("target_cflags", "@COMPILER_RT_TEST_COMPILER_CFLAGS@")
+set_default("sysroot", "@CMAKE_SYSROOT@")
+set_default("osx_sysroot", "@CMAKE_OSX_SYSROOT@")
----------------
This will be the sysroot for macOS which is the wrong sysroot for all other Apple platforms.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126936/new/

https://reviews.llvm.org/D126936