[libcxx][libcxxabi][libunwind] Honor CMAKE_SYSROOT in tests

[phosek]

LGTM

[abrachet]

Rebase + use new formatting of py file

[abrachet]

@ldionne do you have any objections, or would we be safe to land this?

[phosek]

@ldionne Ping?

[arichardson]

I ran into a similar issue and posted https://github.com/llvm/llvm-project/pull/65517.

[ldionne]

I don't think this is the path we want to take. We shouldn't be setting -isysroot manually in these configs, I've always seen it as technical debt that we did. What's your motivation for doing this?

[ldionne]

I just did some digging and the reason for setting -isysroot manually in these config files is basically that CMake will resolve clang++ to the result of xcrun --find clang++ when it sets CMAKE_CXX_COMPILER: https://gitlab.kitware.com/cmake/cmake/-/issues/19180. Without this issue, we wouldn't be setting -isysroot manually at all.

[ldionne]

See https://github.com/llvm/llvm-project/pull/66265

[phosek]

We always use the just built Clang (built using the bootstrap build) even on Darwin, which is not recognized by xcrun --find clang, so we need to manually pass -isysroot.

[ldionne]

I believe the latest incarnation of https://github.com/llvm/llvm-project/pull/66265 fixes your issues with the bootstrapping build as well. The reason why I want to go down that route instead of doing this patch is that we should aim to *simplify* how the compiler is invoked and make it closer to what end users will do, instead of adding more and more flags to make it do what we want. This is important because we strive to test-as-we-ship as much as we can, to avoid issues where our test suite doesn't really reflect the reality of how users use the library. If you agree that https://github.com/llvm/llvm-project/pull/66265 solves your problem, let's abandon this.

libcxx/test/CMakeLists.txt

 endif()
 
 if (CMAKE_CXX_COMPILER_TARGET)
   serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
 else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
+if (CMAKE_SYSROOT)
+  serialize_lit_param(sysroot "\"${CMAKE_SYSROOT}\"")
+endif()
+
 if (LLVM_USE_SANITIZER)
   serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
 endif()
 
 foreach(param IN LISTS LIBCXX_TEST_PARAMS)
   string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")
   string(REGEX REPLACE "(.+)=(.+)" "\\2" value "${param}")
   serialize_lit_param("${name}" "\"${value}\"")

libcxx/test/configs/llvm-libc++-shared.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a shared library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
+local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-pthread' + (' -isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else '')
+    '-pthread' + ('{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else '')
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -Wl,-rpath,%{lib} -lc++'
 ))
 config.substitutions.append(('%{exec}',

libcxx/test/configs/llvm-libc++-static.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a static library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
+local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-pthread' + (' -isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else '')
+    '-pthread' + ('{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else '')
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -lc++ -lc++abi'
 ))
 config.substitutions.append(('%{exec}',

libcxx/utils/libcxx/test/params.py

Parameter(
         help="Whether to enable backwards-compatibility transitive includes when running the tests. This "
         "is provided to ensure that the trimmed-down version of libc++ does not bit-rot in between "
         "points at which we bulk-remove transitive includes.",
         actions=lambda enabled: [] if enabled else [
             AddFeature("transitive-includes-disabled"),
             AddCompileFlag("-D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES"),
         ],
     ),
+    Parameter(name='sysroot', type=str, default='',
+        help="sysroot to use",
+        actions=lambda sysroot: [AddCompileFlag(f"--sysroot={sysroot}")] if sysroot else []),
 ]
 # fmt: on

libcxxabi/test/CMakeLists.txt

 endif()
 
 if (CMAKE_CXX_COMPILER_TARGET)
   serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
 else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
+if (CMAKE_SYSROOT)
+  serialize_lit_param(sysroot "\"${CMAKE_SYSROOT}\"")
+endif()
+
 foreach(param IN LISTS LIBCXXABI_TEST_PARAMS)
   string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")
   string(REGEX REPLACE "(.+)=(.+)" "\\2" value "${param}")
   serialize_lit_param("${name}" "\"${value}\"")
 endforeach()
 
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/configs/cmake-bridge.cfg.in"
                "${CMAKE_CURRENT_BINARY_DIR}/cmake-bridge.cfg"

libcxxabi/test/configs/llvm-libc++abi-shared.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++abi
 # using a shared library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
+local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
+    '{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{include} -I %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -I %{libcxx}/test/support -I %{libcxx}/src -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -Wl,-rpath,%{lib} -lc++ -lc++abi -pthread'
 ))
 config.substitutions.append(('%{exec}',

libcxxabi/test/configs/llvm-libc++abi-static.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++abi
 # using a static library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
+local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
+    '{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{include} -I %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -I %{libcxx}/test/support -I %{libcxx}/src -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -lc++ -lc++abi -pthread'
 ))
 config.substitutions.append(('%{exec}',

libunwind/test/configs/llvm-libunwind-merged.cfg.in

 
 if '@CMAKE_SYSTEM_NAME@' == 'Linux':
     link_flags.append('-Wl,--export-dynamic')
 
 # Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
 compile_flags.append('-funwind-tables')
 
 local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
+    '{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
 ))
 config.substitutions.append(('%{link_flags}',
     '-L %{{lib}} -Wl,-rpath,%{{lib}} -lc++ -ldl {}'.format(' '.join(link_flags))
 ))
 config.substitutions.append(('%{exec}',

libunwind/test/configs/llvm-libunwind-shared.cfg.in

 
 if '@CMAKE_SYSTEM_NAME@' == 'Linux':
     link_flags.append('-Wl,--export-dynamic')
 
 # Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
 compile_flags.append('-funwind-tables')
 
 local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
+    '{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
 ))
 config.substitutions.append(('%{link_flags}',
     '-L %{{lib}} -Wl,-rpath,%{{lib}} -lunwind -ldl {}'.format(' '.join(link_flags))
 ))
 config.substitutions.append(('%{exec}',

libunwind/test/configs/llvm-libunwind-static.cfg.in

 
 if '@CMAKE_SYSTEM_NAME@' == 'Linux':
     link_flags.append('-Wl,--export-dynamic')
 
 # Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
 compile_flags.append('-funwind-tables')
 
 local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
+sysroot_flag = '--sysroot=' if '@CMAKE_SYSROOT@' else '-isysroot '
 config.substitutions.append(('%{flags}',
-    '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
+    '{}{}'.format(sysroot_flag, local_sysroot) if local_sysroot else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
 ))
 config.substitutions.append(('%{link_flags}',
     '%{{lib}}/libunwind.a -ldl {}'.format(' '.join(link_flags))
 ))
 config.substitutions.append(('%{exec}',