Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[windows][toolchain] Build sanitizers and builtins standalone for all SDKs #78861

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+64 −76
Open

[windows][toolchain] Build sanitizers and builtins standalone for all SDKs #78861

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3625597
[windows][toolchain] Build sanitizers and builtins standalone for all…
weliveindetail Jan 23, 2025
fe50fea
Mark failing Swift sanitizer tests as XFAIL instead of UNSUPPORTED
weliveindetail Jan 21, 2025
ee702d9
[dev] Temporarily enable Android x86_64 in CI
weliveindetail Jan 22, 2025
9f57c93
Configure target LLVM with stage-1 compilers and not the build machin…
weliveindetail Jan 24, 2025
9c61e36
Infer LLVM major version from version string of llvm-lit.py
weliveindetail Jan 24, 2025
86f4bcf
Drop LLVM_RUNTIMES_TARGET parameter from sanitizer config
weliveindetail Jan 24, 2025
7ec4021
Drop CMAKE_MT parameter from both configs
weliveindetail Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions cmake/caches/Windows-aarch64.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,6 @@ set(LLVM_DEFAULT_TARGET_TRIPLE aarch64-unknown-windows-msvc CACHE STRING "")
set(LLVM_APPEND_VC_REV NO CACHE BOOL "")
set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR YES CACHE BOOL "")
set(LLVM_ENABLE_PYTHON YES CACHE BOOL "")
set(LLVM_RUNTIME_TARGETS
aarch64-unknown-windows-msvc
CACHE STRING "")
foreach(target ${LLVM_RUNTIME_TARGETS})
set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES
compiler-rt
CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_MT mt CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_CRT NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_LIBFUZZER NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_PROFILE YES CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_SANITIZERS NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_XRAY NO CACHE BOOL "")
endforeach()

set(LLVM_TARGETS_TO_BUILD AArch64 ARM WebAssembly X86 CACHE STRING "")

Expand Down Expand Up @@ -167,7 +151,6 @@ set(LLVM_DISTRIBUTION_COMPONENTS
libclang
libclang-headers
LTO
runtimes
${LLVM_TOOLCHAIN_TOOLS}
${CLANG_TOOLS}
${LLD_TOOLS}
Expand Down
58 changes: 0 additions & 58 deletions cmake/caches/Windows-x86_64.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,62 +27,6 @@ set(LLVM_APPEND_VC_REV NO CACHE BOOL "")
set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR YES CACHE BOOL "")
set(LLVM_ENABLE_PYTHON YES CACHE BOOL "")

set(DEFAULT_BUILTIN_TARGETS
x86_64-unknown-windows-msvc
aarch64-unknown-windows-msvc)
# Build the android builtins if NDK path is provided.
if(NOT "$ENV{NDKPATH}" STREQUAL "")
list(APPEND DEFAULT_BUILTIN_TARGETS
aarch64-unknown-linux-android
x86_64-unknown-linux-android)
endif()

# The builtin targets are used to build the compiler-rt builtins.
set(LLVM_BUILTIN_TARGETS ${DEFAULT_BUILTIN_TARGETS} CACHE STRING "")

# The runtime targets are used to build the compiler-rt profile library.
set(LLVM_RUNTIME_TARGETS
x86_64-unknown-windows-msvc
aarch64-unknown-windows-msvc
CACHE STRING "")

foreach(target ${LLVM_RUNTIME_TARGETS})
set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES
compiler-rt
CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_MT mt CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
set(RUNTIMES_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_CRT NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_LIBFUZZER NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_ORC NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_PROFILE YES CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_SANITIZERS NO CACHE BOOL "")
set(RUNTIMES_${target}_COMPILER_RT_BUILD_XRAY NO CACHE BOOL "")
endforeach()

foreach(target ${LLVM_BUILTIN_TARGETS})
set(BUILTINS_${target}_CMAKE_MT mt CACHE STRING "")
if(${target} MATCHES windows-msvc)
set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
elseif(${target} MATCHES linux-android)
# Use a single 'linux' directory and arch-based lib names on Android.
set(BUILTINS_${target}_LLVM_ENABLE_PER_TARGET_RUNTIME_DIR NO CACHE BOOL "")
set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Android CACHE STRING "")
if(${target} MATCHES aarch64)
set(BUILTINS_${target}_CMAKE_ANDROID_ARCH_ABI arm64-v8a CACHE STRING "")
else()
set(BUILTINS_${target}_CMAKE_ANDROID_ARCH_ABI x86_64 CACHE STRING "")
endif()
set(BUILTINS_${target}_CMAKE_ANDROID_NDK $ENV{NDKPATH} CACHE PATH "")
set(BUILTINS_${target}_CMAKE_ANDROID_API 21 CACHE STRING "")
set(BUILTINS_${target}_CMAKE_C_COMPILER_TARGET "${target}21" CACHE STRING "")
set(BUILTINS_${target}_CMAKE_CXX_COMPILER_TARGET "${target}21" CACHE STRING "")
endif()
set(BUILTINS_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
endforeach()

set(LLVM_TARGETS_TO_BUILD AArch64 ARM WebAssembly X86 CACHE STRING "")

# Disable certain targets to reduce the configure time or to avoid configuration
Expand Down Expand Up @@ -207,8 +151,6 @@ set(LLVM_DISTRIBUTION_COMPONENTS
libclang
libclang-headers
LTO
builtins
runtimes
${LLVM_TOOLCHAIN_TOOLS}
${CLANG_TOOLS}
${LLD_TOOLS}
Expand Down
2 changes: 2 additions & 0 deletions test/Driver/sanitize_coverage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// XFAIL: OS=windows-msvc

// Different sanitizer coverage types
// RUN: %swiftc_driver -driver-print-jobs -sanitize-coverage=func -sanitize=address %s | %FileCheck -check-prefix=SANCOV_FUNC %s
// RUN: %swiftc_driver -driver-print-jobs -sanitize-coverage=bb -sanitize=address %s | %FileCheck -check-prefix=SANCOV_BB %s
Expand Down
1 change: 1 addition & 0 deletions test/IRGen/address_sanitizer_use_odr_indicator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// XFAIL: OS=windows-msvc
// REQUIRES: asan_runtime

// Default instrumentation that does not use ODR indicators
Expand Down
2 changes: 2 additions & 0 deletions test/Interpreter/indirect_enum.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// XFAIL: OS=windows-msvc

// RUN: %target-swiftc_driver %s -g -sanitize=address -o %t_asan-binary
// RUN: %target-codesign %t_asan-binary
// RUN: env ASAN_OPTIONS=detect_leaks=0 %target-run %t_asan-binary
Expand Down
1 change: 1 addition & 0 deletions test/Reflection/typeref_decoding_asan.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// UNSUPPORTED: OS=linux-gnu && CPU=aarch64
// XFAIL: OS=windows-msvc

// rdar://100805115
// UNSUPPORTED: CPU=arm64e
Expand Down
2 changes: 2 additions & 0 deletions test/Sanitizers/asan/asan.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// XFAIL: OS=windows-msvc

// RUN: %target-swiftc_driver %s -g -sanitize=address -o %t_asan-binary
// RUN: %target-codesign %t_asan-binary
// RUN: env %env-ASAN_OPTIONS=abort_on_error=0 not %target-run %t_asan-binary 2>&1 | %FileCheck %s
Expand Down
1 change: 1 addition & 0 deletions test/Sanitizers/sanitizer_coverage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// For now restrict this test to platforms where we know this test will pass
// REQUIRES: CPU=x86_64
// UNSUPPORTED: remote_run
// XFAIL: OS=windows-msvc

func sayHello() {
print("Hello")
Expand Down
2 changes: 2 additions & 0 deletions utils/build-windows-toolchain.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ powershell.exe -ExecutionPolicy RemoteSigned -File %~dp0build.ps1 ^
-ImageRoot %BuildRoot% ^
%SkipPackagingArg% ^
%TestArg% ^
-AndroidSDKs x86_64 ^
-WindowsSDKs X64 ^
-Stage %PackageRoot% ^
-Summary || (exit /b 1)

Expand Down
50 changes: 50 additions & 0 deletions utils/build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ enum TargetComponent {
Foundation
XCTest
Testing
ClangBuiltins
ClangRuntime
}

function Get-TargetProjectBinaryCache($Arch, [TargetComponent]$Project) {
Expand Down Expand Up @@ -1606,6 +1608,52 @@ function Build-LLVM([Platform]$Platform, $Arch) {
}
}

function Build-Sanitizers([Platform]$Platform, $Arch) {
$BareTarget = $Arch.LLVMTarget.Replace("$AndroidAPILevel", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that @andrurogerz is working on creating a ModuleTriple parameter in the architecture definition to help with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good, yeah this difference is nifty. The unified build used to pass it via LLVM_RUNTIMES_TARGET
here, but in the standalone build CMake reports it as unused. Will drop it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

$LLVMDir = "$(Get-TargetProjectBinaryCache $Arch LLVM)\lib\cmake\llvm"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be target? What happens if we don't have a target build of LLVM? (e.g. Android)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came up as a warning during configuration. In many cases standalone builds reach out to LLVM for configuration details or test dependencies. That should work because we do configure LLVM for Android targets. Let me double-check that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. It's required for testing and otherwise silences this warning: https://github.com/llvm/llvm-project/blob/57b48987f6c21e369e7bb1626dc79ca74aa34fdb/compiler-rt/cmake/Modules/CompilerRTUtils.cmake#L314

$InstallTo = "$($HostArch.ToolchainInstallRoot)\usr\lib\clang\19"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep it like that? I can probably get it from running stage-1 Clang, or lit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should extract this from the build, the resource dir changes on each rebranch, and this will complicate things.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done. I think asking llvm-config would usually be the right way, but for cross-compiles we cannot execute it. We are asking llvm-lit.py now. Seems more reliable than parsing a CMake config file or a header somewhere in the build-tree.


Build-CMakeProject `
-Src $SourceCache\llvm-project\compiler-rt\lib\builtins `
-Bin "$(Get-TargetProjectBinaryCache $Arch ClangBuiltins)" `
-InstallTo $InstallTo `
-Arch $Arch `
-Platform $Platform `
-UseBuiltCompilers C,CXX `
-BuildTargets "install-compiler-rt" `
-Defines (@{
CMAKE_MT = "mt";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, it's not. Dropped.

CMAKE_SYSTEM_NAME = $Platform.ToString();
LLVM_DIR = $LLVMDir;
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR = "YES";
COMPILER_RT_DEFAULT_TARGET_ONLY = "YES";
})

Build-CMakeProject `
-Src $SourceCache\llvm-project\compiler-rt `
-Bin "$(Get-TargetProjectBinaryCache $Arch ClangRuntime)" `
-InstallTo $InstallTo `
-Arch $Arch `
-Platform $Platform `
-UseBuiltCompilers C,CXX `
-BuildTargets "install-compiler-rt" `
-Defines (@{
CMAKE_MT = "mt";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped as well

CMAKE_SYSTEM_NAME = $Platform.ToString();
LLVM_DIR = $LLVMDir;
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR = "YES";
LLVM_RUNTIMES_TARGET = $BareTarget;
COMPILER_RT_DEFAULT_TARGET_ONLY = "YES";
COMPILER_RT_BUILD_BUILTINS = "NO";
COMPILER_RT_BUILD_CRT = "NO";
COMPILER_RT_BUILD_LIBFUZZER = "NO";
COMPILER_RT_BUILD_ORC = "NO";
COMPILER_RT_BUILD_XRAY = "NO";
COMPILER_RT_BUILD_PROFILE = "YES";
COMPILER_RT_BUILD_SANITIZERS = "YES";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could put this into a CMake cache file, but it won't get much simpler

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can see why it would not simplify much, but, it does make it more obvious that this is static configuration and not logic when you need a last second change to the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH the structure in https://github.com/swiftlang/swift/tree/main/cmake/caches forces us to add 7 new files. And they all have identical content. (Unless we outsource CMAKE_SYSTEM_NAME, but it won't make that much of a difference.) I guess it should be: LLVM-$Platform-$($Arch.LLVMName).cmake

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could look like this for the two arches I am testing right now: weliveindetail@windows-toolchain-sanitizers-standalone-with-caches

Not sure. Or should I break the scheme and make just one?

})
}

function Build-ZLib([Platform]$Platform, $Arch) {
$ArchName = $Arch.LLVMName

Expand Down Expand Up @@ -2822,6 +2870,7 @@ if (-not $SkipBuild) {
Invoke-BuildStep Build-FoundationMacros -Build Windows $BuildArch
Invoke-BuildStep Build-TestingMacros -Build Windows $BuildArch
Invoke-BuildStep Build-Foundation Windows $Arch
Invoke-BuildStep Build-Sanitizers Windows $Arch
Invoke-BuildStep Build-XCTest Windows $Arch
Invoke-BuildStep Build-Testing Windows $Arch
Invoke-BuildStep Write-PlatformInfoPlist $Arch
Expand All @@ -2840,6 +2889,7 @@ if (-not $SkipBuild) {
Invoke-BuildStep Build-Runtime Android $Arch
Invoke-BuildStep Build-Dispatch Android $Arch
Invoke-BuildStep Build-Foundation Android $Arch
Invoke-BuildStep Build-Sanitizers Android $Arch
Invoke-BuildStep Build-XCTest Android $Arch
Invoke-BuildStep Build-Testing Android $Arch
Invoke-BuildStep Write-PlatformInfoPlist $Arch
Expand Down