[CMake] Use COMPILER_RT_BUILD_CRT in the condition for test

[phosek]

Summary

Unlike COMPILER_RT_HAS_CRT this handles the case where CRT is available
but has been disabled by setting COMPILER_RT_BUILD_CRT. This addresses
an issue reported on D153989.

[smeenai]

compiler-rt/lib/builtins/CMakeLists.txt

905

HAS_CRT is the internal check and BUILD_CRT is user-configurable, right? Could we simplify this by ensuring that BUILD_CRT is always false is HAS_CRT is false (perhaps erroring out if the user explicitly asked it to be built when it's unavailable)?

[phosek]

compiler-rt/lib/builtins/CMakeLists.txt

905

That's what cmake_dependent_option on the line above was intended achieve. The problem is that there are some LLVM bots that use incremental builds and have COMPILER_RT_BUILD_CRT=ON in their CMake cache. I was hoping to force a clean build on these bots, but it looks like the Buildbot login is currently broken :(

[smeenai]

compiler-rt/lib/builtins/CMakeLists.txt

905

Ah, that's subtle. If I'm reading https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html correctly though, we'd overwrite the cache value in this file by creating a local variable. That wouldn't help test/builtins/CMakeLists.txt though.

[phosek]

compiler-rt/lib/builtins/CMakeLists.txt

905

A potential alternative would be to add unset(COMPILER_RT_BUILD_CRT CACHE) above cmake_dependent_option with TODO and then delete in in a follow up change. WDYT?

[smeenai]

compiler-rt/lib/builtins/CMakeLists.txt

905

Yeah, I think that's cleaner and conveys the issue better.

[arichardson]

compiler-rt/lib/builtins/CMakeLists.txt

901

This also appears to unset COMPILER_RT_BUILD_CRT when set on the command line. I was surprised to see the CRT builds being run for my builtins-only build config (I noticed because one of the tests was failing despite having REQUIRES: crt.

I pass -DCOMPILER_RT_BUILD_CRT=OFF to cmake. Adding some prints around this statement shows the following:

-- before unset(): COMPILER_RT_BUILD_CRT=OFF
-- after unset: COMPILER_RT_BUILD_CRT=
-- after cmake_dependent_option COMPILER_RT_BUILD_CRT=ON

compiler-rt/lib/builtins/CMakeLists.txt

if(COMPILER_RT_BUILD_STANDALONE_LIBATOMIC)
   if(OS_NAME MATCHES "AIX")
     archive_aix_libatomic(clang_rt.atomic libatomic
                           ARCHS ${BUILTIN_SUPPORTED_ARCH}
                           PARENT_TARGET builtins-standalone-atomic)
   endif()
   add_dependencies(compiler-rt builtins-standalone-atomic)
 endif()
 
+# TODO: COMPILER_RT_BUILD_CRT used to be a cached variable so we need to unset
+# it first so cmake_dependent_option can set the local variable of the same
+# name. This statement can be removed in the future.
+unset(COMPILER_RT_BUILD_CRT CACHE)
+
 cmake_dependent_option(COMPILER_RT_BUILD_CRT "Build crtbegin.o/crtend.o" ON "COMPILER_RT_HAS_CRT" OFF)
 
-if(COMPILER_RT_BUILD_CRT)
+if (COMPILER_RT_BUILD_CRT)
   add_compiler_rt_component(crt)
 
   option(COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY "Use eh_frame in crtbegin.o/crtend.o" ON)
 
   include(CheckSectionExists)
   check_section_exists(".init_array" COMPILER_RT_HAS_INITFINI_ARRAY
     SOURCE "volatile int x;\n__attribute__((constructor)) void f(void) {x = 0;}\nint main(void) { return 0; }\n")
 

compiler-rt/test/builtins/CMakeLists.txt

 set(BUILTINS_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 set(BUILTINS_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} builtins)
 set(BUILTINS_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/TestCases)
 
 # Test cases.
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
 )
 
 #Unit tests.
 
 include(builtin-config-ix)
 
-if (COMPILER_RT_HAS_CRT)
+if (COMPILER_RT_BUILD_CRT)
   list(APPEND BUILTINS_TEST_DEPS crt)
 endif()
+pythonize_bool(COMPILER_RT_BUILD_CRT)
 
 # Indicate if this is an MSVC environment.
 pythonize_bool(MSVC)
 
 # Indicate if the compiler for the builtins library was MSVC. If the builtins
 # compiler was clang-cl, we will enable some features that the host compiler
 # will not, like C99 _Complex and int128.
 set(BUILTINS_IS_MSVC OFF)

compiler-rt/test/builtins/Unit/ctor_dtor.c

-// REQUIRES: linux
+// REQUIRES: crt
 
 // RUN: %clang -fno-use-init-array -g -c %s -o %t.o
 // RUN: %clang -o %t -no-pie -nostdlib %crt1 %crti %crtbegin %t.o -lc %libgcc %crtend %crtn
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #include <stdio.h>
 #include <stdlib.h>
 

compiler-rt/test/builtins/Unit/dso_handle.cpp

-// REQUIRES: linux
+// REQUIRES: crt
 
 // RUN: %clangxx -g -fno-exceptions -DCRT_SHARED -c %s -fPIC -o %tshared.o
 // RUN: %clangxx -g -fno-exceptions -c %s -fPIC -o %t.o
 // RUN: %clangxx -g -shared -o %t.so -nostdlib %crti %crtbegin %tshared.o %libstdcxx -lc -lm %libgcc %crtend %crtn
 // RUN: %clangxx -g -o %t -fno-pic -no-pie -nostdlib %crt1 %crti %crtbegin %t.o %libstdcxx -lc -lm %libgcc %t.so %crtend %crtn
 // RUN: %run %t 2>&1 | FileCheck %s
 
 // UNSUPPORTED: target={{(arm|aarch64).*}}

compiler-rt/test/builtins/Unit/lit.cfg.py

else:
     base_lib = os.path.join(
         config.compiler_rt_libdir, "libclang_rt.builtins%s.a" % config.target_suffix
     )
     if sys.platform in ["win32"] and execute_external:
         # Don't pass dosish path separator to msys bash.exe.
         base_lib = base_lib.replace("\\", "/")
     config.substitutions.append(("%librt ", base_lib + " -lc -lm "))
 
-    if config.host_os == "Linux":
-        base_obj = os.path.join(
-            config.compiler_rt_libdir, "clang_rt.%%s%s.o" % config.target_suffix
-        )
-        if sys.platform in ["win32"] and execute_external:
-            # Don't pass dosish path separator to msys bash.exe.
-            base_obj = base_obj.replace("\\", "/")
+builtins_build_crt = get_required_attr(config, "builtins_build_crt")
+if builtins_build_crt:
 
-        config.substitutions.append(("%crtbegin", base_obj % "crtbegin"))
-        config.substitutions.append(("%crtend", base_obj % "crtend"))
 
-        config.substitutions.append(("%crt1", get_library_path("crt1.o")))
-        config.substitutions.append(("%crti", get_library_path("crti.o")))
-        config.substitutions.append(("%crtn", get_library_path("crtn.o")))
 
-        config.substitutions.append(("%libgcc", get_libgcc_file_name()))
 
-        config.substitutions.append(
-            ("%libstdcxx", "-l" + config.sanitizer_cxx_lib.lstrip("lib"))
-        )
 
+    config.available_features.add("crt")
+
 builtins_source_dir = os.path.join(
     get_required_attr(config, "compiler_rt_src_root"), "lib", "builtins"
 )
 if sys.platform in ["win32"] and execute_external:
     # Don't pass dosish path separator to msys bash.exe.
     builtins_source_dir = builtins_source_dir.replace("\\", "/")
 builtins_lit_source_dir = get_required_attr(config, "builtins_lit_source_dir")
 

compiler-rt/test/builtins/Unit/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 config.name_suffix = "@BUILTINS_TEST_CONFIG_SUFFIX@"
 config.builtins_lit_source_dir = "@BUILTINS_LIT_SOURCE_DIR@/Unit"
 config.target_cflags = "@BUILTINS_TEST_TARGET_CFLAGS@"
 config.target_arch = "@BUILTINS_TEST_TARGET_ARCH@"
 config.sanitizer_cxx_lib = "@SANITIZER_TEST_CXX_LIBNAME@"
+config.builtins_build_crt = @COMPILER_RT_BUILD_CRT_PYBOOL@
 config.is_msvc = @MSVC_PYBOOL@
 config.builtins_is_msvc = @BUILTINS_IS_MSVC_PYBOOL@
 config.builtins_lit_source_features = "@BUILTINS_LIT_SOURCE_FEATURES@"
 config.test_suite_supports_overriding_runtime_lib_path = True
 
 # Load common config for all compiler-rt lit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
 
 # Load tool-specific config that would do the real work.
 lit_config.load_config(config, "@BUILTINS_LIT_SOURCE_DIR@/Unit/lit.cfg.py")