bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.19k stars 4.06k forks source link

Compiling assembly with gcc-style args broken using clang-cl on Windows #24152

Open fhanau opened 1 day ago

fhanau commented 1 day ago

Description of the bug:

As of Bazel 7.3.1, the standard clang-cl Windows toolchain supports compiling assembly code using clang-cl.exe (as it does with C and C++ code). As of 7.4.0, the clang-cl toolchain uses MASM instead of clang-cl itself to compile assembly code. This breaks our build configuration as we use gcc-style compiler flags (e.g. -Wno-macro-redefined) which are not supported by MASM. The most straightforward way to fix this would be to revert #23406 so that clang-cl.exe is used again for assembly, contrary to what is stated there it is capable of compiling assembly code. This resolves the inconsistency of using MSVC tools for assembly and clang-cl for everything else in the clang-cl toolchain; having the tools be consistent is crucial for being able to compile the same files with the same compiler flags on both Unix and Windows using clang/clang-cl. Of course, this would break developers who depend on using MASM for some assembly files (as reported in the original issue motivating the change, #23128). Ideally, there would be some way to configure what assembler is being used without having to define a custom toolchain; and a means to not have C/C++ compiler flags apply to compiling assembly (we need -Wno-macro-redefined to avoid excessive warnings for C++ code on Windows, but don't actually need it when compiling assembly).

Which category does this issue belong to?

C++ Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

We assume that Windows and the clang-cl toolchain are being used. Create an empty dummy.S file alongside a basic assembly target:

cc_library(
    name = "dummy",
    srcs = ["dummy.S"],
)

Then compile the target with a gcc-style compiler flag, e.g. bazel build --copt=-Wno-macro-redefined //:dummy. This works with Bazel 7.3.1, but fails with Bazel 7.4.0 with the following error. //:x64_windows-clang-cl refers to a simple platform definition for clang-cl setting the @bazel_tools//tools/cpp:clang-cl constraint. The output reflects this being executed in a CI environment for our project.

ERROR: D:/a/<path>/tests/BUILD.bazel:28:11: Compiling <path>/tests/dummy.S failed: (Exit 1): ml64.exe failed: error executing CppCompile command (from target //<path>/tests:dummy) 
MASM : warning A4018:invalid command-line option : /bigobj
  cd /d C:/tmp/hs7imhxj/execroot/<path>
  SET INCLUDE=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\include;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\ATLMFC\include;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include;C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um;C:\Program Files\LLVM\lib\clang\18\include
    SET PATH=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\bin\HostX64\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\VC\VCPackages;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Current\bin\Roslyn;C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\x64\;C:\Program Files (x86)\HTML Help Workshop;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Team Tools\DiagnosticsHub\Collector;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\Extensions\Microsoft\CodeCoverage.Console;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\\x64;C:\Program Files (x86)\Windows Kits\10\bin\\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\\MSBuild\Current\Bin\amd64;C:\Windows\Microsoft.NET\Framework64\v4.0.30319;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\Tools\;;C:\Windows\system32;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\Llvm\x64\bin;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\VC\Linux\bin\ConnectionManagerExe;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\vcpkg
    SET ***
    SET TEMP=C:\Users\RUNNER~1\AppData\Local\Temp
    SET TMP=C:\Users\RUNNER~1\AppData\Local\Temp
    SET VSLANG=1033
  C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\bin\HostX64\x64\ml64.exe @bazel-out/x64_windows-fastbuild/bin/<path>/tests/_objs/dummy/dummy.obj.params
# Configuration: ed49adc2ea3b3e3521585f328ebde607[70]<redacted>
# Execution platform: //:x64_windows-clang-cl
Microsoft (R) Macro Assembler (x64) Version 14.41.34123.0
Copyright (C) Microsoft Corporation.  All rights reserved.

MASM : warning A4018:invalid command-line option : /Zm500
MASM : warning A4018:invalid command-line option : /Z500
MASM : warning A4018:invalid command-line option : /Z00
MASM : warning A4018:invalid command-line option : /Z0
MASM : warning A4018:invalid command-line option : /EHsc
MASM : warning A4018:invalid command-line option : /wd4351
MASM : warning A4018:invalid command-line option : /wd4291
MASM : warning A4018:invalid command-line option : /wd4250
MASM : warning A4018:invalid command-line option : /wd4996
MASM : warning A4018:invalid command-line option : /showIncludes
MASM : fatal error A1013:invalid numerical command-line argument : /W

The params file dummy.obj.params suggests that this failed with the first argument with a leading dash, which leads to the A1013 error.

/nologo
/DCOMPILER_MSVC
/DNOMINMAX
/D_WIN32_WINNT=0x0601
/D_CRT_SECURE_NO_DEPRECATE
/D_CRT_SECURE_NO_WARNINGS
/bigobj
/Zm500
/EHsc
/wd4351
/wd4291
/wd4250
/wd4996
/I.
/Ibazel-out/x64_windows-fastbuild/bin
/showIncludes
-Wno-macro-redefined
/Fobazel-out/x64_windows-fastbuild/bin/<path>/tests/_objs/dummy/dummy.obj
/c
<path>/tests/dummy.S

Since the action failed due to MASM not accepting gcc-style arguments like -Wno-macro-redefined, the switch to MASM in 7.4.0 is likely responsible here.

Which operating system are you running Bazel on?

Windows Server 2022

What is the output of bazel info release?

release 7.4.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

N/A

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

I did not bisect since I only observed this on Windows CI, but believe that this is being caused by #23406 where the clang-cl toolchain starts using MASM to compile assembly instead of clang-cl which is used for other compile actions. The PR asserts that "clang-cl is not intended to handle assembly", but we have been compiling assembly code on Windows with Bazel 7.3.1 and thus clang-cl without any issues; using MASM instead breaks compilation.

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

meteorcloudy commented 1 day ago

@michaelsiegrist Can you take a look at this issue? Otherwise, we should probably revert e5a083d47e7174357997c851c20521e03a9c80ce in 7.4.1, 8.0.0, and Bazel@HEAD

meteorcloudy commented 1 day ago

@bazel-io fork 7.4.1

meteorcloudy commented 1 day ago

@bazel-io fork 8.0.0