Fix incorrect compiler type with `USE_CLANG_CL` on windows. (#858) This change affects the auto configured toolchains which was setting the incorrect toolchain type when `--repo_env=USE_CLANG_CL=1` is set. Closes #858 COPYBARA_INTEGRATE_REVIEW=https://github.com/bazelbuild/rules_cc/pull/858 from UebelAndre:dump 4eb3b2d0e86c7168e00ab754bb3eca6a4315f56f PiperOrigin-RevId: 975960065 Change-Id: I603889f541f0123ec4076948c69ad4f32b93e2a6
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 041247d..3c68838 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml
@@ -59,7 +59,6 @@ build_targets: - "//..." - buildifier: version: latest @@ -267,6 +266,22 @@ coverage_flags: *windows_coverage_flags coverage_targets: *test_targets + # Same target set as the `windows` task, with the auto-configured toolchain + # switched over to clang-cl.exe/lld-link.exe/llvm-lib.exe instead of MSVC. + # //tests/compiler_settings static_asserts that the toolchain's declared + # compiler matches the one actually running, so this job cannot silently + # degrade into a copy of the MSVC job. + windows_clang_cl: + name: Windows clang-cl (Bazel 9) + bazel: 9.x + platform: windows + build_flags: + - "--repo_env=USE_CLANG_CL=1" + build_targets: *build_targets + test_flags: + - "--repo_env=USE_CLANG_CL=1" + test_targets: *test_targets + ubuntu_bzlmod: name: Ubuntu 20.04 (Bazel 9, bzlmod) bazel: 9.x
diff --git a/cc/private/rules_impl/cc_static_library.bzl b/cc/private/rules_impl/cc_static_library.bzl index 054c8f5..41de852 100644 --- a/cc/private/rules_impl/cc_static_library.bzl +++ b/cc/private/rules_impl/cc_static_library.bzl
@@ -273,8 +273,11 @@ <h5 id="cc_static_library_symbol_check_toolchain">Toolchain support for <code>symbol_check</code></h5> <p>The auto-configured C++ toolchains shipped with Bazel support the -<code>symbol_check</code> feature on all platforms. Custom toolchains can add support for -it in one of two ways:</p> +<code>symbol_check</code> feature on all platforms, except on Windows when the toolchain +has been switched to clang-cl via <code>USE_CLANG_CL=1</code>: the check relies on a +<code>lib.exe</code> flag that <code>llvm-lib.exe</code> does not implement, so no duplicate +symbol check is performed there. Custom toolchains can add support for it in one of two +ways:</p> <ul> <li>Implementing the <code>ACTION_NAMES.validate_static_library</code> action and enabling it with the <code>symbol_check</code> feature. The tool set in the action is
diff --git a/cc/private/toolchain/BUILD.windows.tpl b/cc/private/toolchain/BUILD.windows.tpl index 4ce9022..178b1bf 100644 --- a/cc/private/toolchain/BUILD.windows.tpl +++ b/cc/private/toolchain/BUILD.windows.tpl
@@ -315,7 +315,7 @@ cc_toolchain_config( name = "msvc_x64", cpu = "x64_windows", - compiler = "msvc-cl", + compiler = "%{compiler}", host_system_name = "local", target_system_name = "local", target_libc = "ucrt", @@ -390,7 +390,7 @@ cc_toolchain_config( name = "msvc_x64_x86", cpu = "x64_windows", - compiler = "msvc-cl", + compiler = "%{compiler}", host_system_name = "local", target_system_name = "local", target_libc = "ucrt", @@ -465,7 +465,7 @@ cc_toolchain_config( name = "msvc_x64_arm", cpu = "x64_windows", - compiler = "msvc-cl", + compiler = "%{compiler}", host_system_name = "local", target_system_name = "local", target_libc = "ucrt", @@ -540,7 +540,7 @@ cc_toolchain_config( name = "msvc_arm64", cpu = "x64_windows", - compiler = "msvc-cl", + compiler = "%{compiler}", host_system_name = "local", target_system_name = "local", target_libc = "ucrt",
diff --git a/cc/private/toolchain/windows_cc_configure.bzl b/cc/private/toolchain/windows_cc_configure.bzl index fcefce1..235ebbf 100644 --- a/cc/private/toolchain/windows_cc_configure.bzl +++ b/cc/private/toolchain/windows_cc_configure.bzl
@@ -728,6 +728,11 @@ build_tools["CL"] = find_llvm_tool(repository_ctx, llvm_path, "clang-cl.exe") build_tools["ML"] = find_msvc_tool(repository_ctx, vc_path, "ml64.exe", "x64") + + # LLVM has no dumpbin.exe equivalent that the MSVC toolchain shape can + # use, so source it from MSVC. _find_missing_vc_tools above already + # verified it exists for this target architecture. + build_tools["DUMPBIN"] = find_msvc_tool(repository_ctx, vc_path, "dumpbin.exe", target_arch) build_tools["LINK"] = find_llvm_tool(repository_ctx, llvm_path, "lld-link.exe") if not build_tools["LINK"]: build_tools["LINK"] = find_msvc_tool(repository_ctx, vc_path, "link.exe", "x64") @@ -942,6 +947,13 @@ ) template_vars = dict() + + # USE_CLANG_CL=1 keeps the MSVC toolchain shape but swaps in clang-cl, so + # all four msvc_* configs must report what actually runs -- otherwise + # select()ing on //cc/compiler:msvc-cl feeds cl.exe flags to clang-cl. + # Set outside _get_msvc_vars so the error-stub configs it returns early for + # are labelled the same as the working ones. + template_vars["%{compiler}"] = "clang-cl" if _use_clang_cl(repository_ctx) else "msvc-cl" msvc_vars_x64 = _get_msvc_vars(repository_ctx, paths, "x64") template_vars.update(msvc_vars_x64) template_vars.update(_get_clang_cl_vars(repository_ctx, paths, msvc_vars_x64, "x64"))
diff --git a/cc/private/toolchain/windows_cc_toolchain_config.bzl b/cc/private/toolchain/windows_cc_toolchain_config.bzl index 99fce63..de141cf 100644 --- a/cc/private/toolchain/windows_cc_toolchain_config.bzl +++ b/cc/private/toolchain/windows_cc_toolchain_config.bzl
@@ -1332,15 +1332,23 @@ implies = ["msvc_compile_env", "msvc_link_env"], ) - symbol_check_feature = feature( - name = "symbol_check", - flag_sets = [ - flag_set( - actions = [ACTION_NAMES.cpp_link_static_library], - flag_groups = [flag_group(flags = ["/WX:4006"])], - ), - ], - ) + # /WX:4006 promotes lib.exe's "symbol already defined" warning to an + # error. llvm-lib.exe has no equivalent and parses the flag as an input + # file name, so the archive fails with "/WX:4006: no such file or + # directory". Leave the feature undefined there instead of defining one + # that quietly checks nothing; cc_static_library requests `symbol_check` + # and tolerates the toolchain not offering it. + symbol_check_feature = None + if ctx.attr.compiler == "msvc-cl": + symbol_check_feature = feature( + name = "symbol_check", + flag_sets = [ + flag_set( + actions = [ACTION_NAMES.cpp_link_static_library], + flag_groups = [flag_group(flags = ["/WX:4006"])], + ), + ], + ) features = [ no_legacy_features_feature, @@ -1397,8 +1405,9 @@ no_windows_export_all_symbols_feature, supports_dynamic_linker_feature, supports_interface_shared_libraries_feature, - symbol_check_feature, ] + if symbol_check_feature: + features.append(symbol_check_feature) else: targets_windows_feature = feature( name = "targets_windows",
diff --git a/tests/alwayslink_srcs/registerer.c b/tests/alwayslink_srcs/registerer.c index 9979304..e6a61a6 100644 --- a/tests/alwayslink_srcs/registerer.c +++ b/tests/alwayslink_srcs/registerer.c
@@ -15,13 +15,15 @@ #include "registered.h" // Static constructor that registers on load. -#if defined(_MSC_VER) +#if defined(__GNUC__) || defined(__clang__) +__attribute__((constructor)) static void registerer_init(void) { + set_registered(); +} +#elif defined(_MSC_VER) static void registerer_init(void) { set_registered(); } #pragma section(".CRT$XCU", read) __declspec(allocate(".CRT$XCU")) static void (*registerer_init_)(void) = registerer_init; #else -__attribute__((constructor)) static void registerer_init(void) { - set_registered(); -} +#error "No static constructor mechanism known for this compiler" #endif
diff --git a/tests/compiler_settings/main.cc b/tests/compiler_settings/main.cc index 35b088c..eca61df 100644 --- a/tests/compiler_settings/main.cc +++ b/tests/compiler_settings/main.cc
@@ -17,6 +17,53 @@ #define STRINGIFY(x) #x #define TO_STRING(x) STRINGIFY(x) +#ifdef COMPILER + +namespace { + +constexpr bool StrEq(const char* a, const char* b) { + return *a == *b && (*a == '\0' || StrEq(a + 1, b + 1)); +} + +// The compiler //cc/compiler:compiler reports, i.e. the `compiler` attribute +// of the resolved cc_toolchain_config. +constexpr const char* kDeclaredCompiler = TO_STRING(COMPILER); + +#if defined(__clang__) +constexpr bool kIsClang = true; +#else +constexpr bool kIsClang = false; +#endif + +// clang-cl emulates the MSVC command line and ABI, so it defines _MSC_VER as +// well as __clang__. cl.exe defines only _MSC_VER. +#if defined(_MSC_VER) +constexpr bool kIsMsvcCompatible = true; +#else +constexpr bool kIsMsvcCompatible = false; +#endif + +// Cross-check the declared compiler against what really compiled this file, so +// a toolchain cannot claim one compiler while running another. This is what +// catches USE_CLANG_CL=1 swapping the toolchain over to clang-cl while it +// still declares itself msvc-cl. +// +// Stated as implications keyed off the declared name rather than deriving an +// expected name from the macros: that way only the two labels this repo emits +// are constrained, and any other value -- a third-party toolchain's, or one +// that does not match a //cc/compiler config_setting at all -- is left alone +// instead of failing the build. +static_assert(!StrEq(kDeclaredCompiler, "msvc-cl") || !kIsClang, + "Toolchain declares msvc-cl, but clang compiled this file."); +static_assert(!StrEq(kDeclaredCompiler, "clang-cl") || + (kIsClang && kIsMsvcCompatible), + "Toolchain declares clang-cl, but clang in MSVC-compatible mode " + "is not what compiled this file."); + +} // namespace + +#endif // COMPILER + int main() { std::cout << "Hello, " << TO_STRING(COMPILER) << "!" << std::endl; }