Skip to content

Commit

Permalink
Auto merge of #135204 - RalfJung:win64-zst, r=SparrowLii
Browse files Browse the repository at this point in the history
fix handling of ZST in win64 ABI on windows-msvc targets

The Microsoft calling conventions do not really say anything about ZST since they do not seem to exist in MSVC. However, both GCC and clang allow passing ZST over  `__attribute__((ms_abi))` functions (which matches our `extern "win64" fn`) on `windows-gnu` targets, and therefore implicitly define a de-facto ABI for these types (and lucky enough they seem to define the same ABI). This ABI should be the same for windows-msvc and windows-gnu targets, so we use this as a hint for how to implement this ABI everywhere: we always pass ZST by-ref.

The best alternative would be to just reject compiling functions which cannot exist in MSVC, but that would be a breaking change.

Cc `@programmerjake` `@ChrisDenton`
Fixes #132893
  • Loading branch information
bors committed Jan 13, 2025
2 parents 3ff1b64 + 675a103 commit 7a202a9
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 271 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_abi/src/extern_abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ pub fn is_enabled(
s
}

/// Returns whether the ABI is stable to use.
///
/// Note that there is a separate check determining whether the ABI is even supported
/// on the current target; see `fn is_abi_supported` in `rustc_target::spec`.
pub fn is_stable(name: &str) -> Result<(), AbiDisabled> {
match name {
// Stable
Expand Down
29 changes: 14 additions & 15 deletions compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
use std::str::FromStr;
use std::{fmt, iter};

pub use rustc_abi::{Reg, RegKind};
pub use rustc_abi::{ExternAbi, Reg, RegKind};
use rustc_macros::HashStable_Generic;
use rustc_span::Symbol;

use crate::abi::{
self, AddressSpace, Align, BackendRepr, HasDataLayout, Pointer, Size, TyAbiInterface,
TyAndLayout,
};
use crate::spec::abi::Abi as SpecAbi;
use crate::spec::{self, HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};
use crate::spec::{HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};

mod aarch64;
mod amdgpu;
Expand Down Expand Up @@ -627,20 +626,20 @@ impl<'a, Ty: fmt::Display> fmt::Debug for FnAbi<'a, Ty> {
#[derive(Copy, Clone, Debug, HashStable_Generic)]
pub enum AdjustForForeignAbiError {
/// Target architecture doesn't support "foreign" (i.e. non-Rust) ABIs.
Unsupported { arch: Symbol, abi: spec::abi::Abi },
Unsupported { arch: Symbol, abi: ExternAbi },
}

impl<'a, Ty> FnAbi<'a, Ty> {
pub fn adjust_for_foreign_abi<C>(
&mut self,
cx: &C,
abi: spec::abi::Abi,
abi: ExternAbi,
) -> Result<(), AdjustForForeignAbiError>
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec + HasWasmCAbiOpt + HasX86AbiOpt,
{
if abi == spec::abi::Abi::X86Interrupt {
if abi == ExternAbi::X86Interrupt {
if let Some(arg) = self.args.first_mut() {
arg.pass_by_stack_offset(None);
}
Expand All @@ -651,12 +650,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
match &spec.arch[..] {
"x86" => {
let (flavor, regparm) = match abi {
spec::abi::Abi::Fastcall { .. } | spec::abi::Abi::Vectorcall { .. } => {
ExternAbi::Fastcall { .. } | ExternAbi::Vectorcall { .. } => {
(x86::Flavor::FastcallOrVectorcall, None)
}
spec::abi::Abi::C { .. }
| spec::abi::Abi::Cdecl { .. }
| spec::abi::Abi::Stdcall { .. } => {
ExternAbi::C { .. } | ExternAbi::Cdecl { .. } | ExternAbi::Stdcall { .. } => {
(x86::Flavor::General, cx.x86_abi_opt().regparm)
}
_ => (x86::Flavor::General, None),
Expand All @@ -666,8 +663,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
x86::compute_abi_info(cx, self, opts);
}
"x86_64" => match abi {
spec::abi::Abi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
spec::abi::Abi::Win64 { .. } => x86_win64::compute_abi_info(cx, self),
ExternAbi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
ExternAbi::Win64 { .. } | ExternAbi::Vectorcall { .. } => {
x86_win64::compute_abi_info(cx, self)
}
_ => {
if cx.target_spec().is_like_windows {
x86_win64::compute_abi_info(cx, self)
Expand Down Expand Up @@ -701,7 +700,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
"sparc" => sparc::compute_abi_info(cx, self),
"sparc64" => sparc64::compute_abi_info(cx, self),
"nvptx64" => {
if cx.target_spec().adjust_abi(abi, self.c_variadic) == spec::abi::Abi::PtxKernel {
if cx.target_spec().adjust_abi(abi, self.c_variadic) == ExternAbi::PtxKernel {
nvptx64::compute_ptx_kernel_abi_info(cx, self)
} else {
nvptx64::compute_abi_info(self)
Expand Down Expand Up @@ -730,7 +729,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
Ok(())
}

pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: SpecAbi)
pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: ExternAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
Expand Down Expand Up @@ -821,7 +820,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
// that's how we connect up to LLVM and it's unstable
// anyway, we control all calls to it in libstd.
BackendRepr::Vector { .. }
if abi != SpecAbi::RustIntrinsic && spec.simd_types_indirect =>
if abi != ExternAbi::RustIntrinsic && spec.simd_types_indirect =>
{
arg.make_indirect();
continue;
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_target/src/callconv/x86_win64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::spec::HasTargetSpec;

// Win64 ABI: https://docs.microsoft.com/en-us/cpp/build/parameter-passing

pub(crate) fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
let fixup = |a: &mut ArgAbi<'_, Ty>| {
match a.layout.backend_repr {
BackendRepr::Uninhabited | BackendRepr::Memory { sized: false } => {}
Expand Down Expand Up @@ -40,16 +40,18 @@ pub(crate) fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'
fixup(&mut fn_abi.ret);
}
for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
// x86_64-pc-windows-gnu doesn't ignore ZSTs.
if cx.target_spec().os == "windows"
&& cx.target_spec().env == "gnu"
&& arg.layout.is_zst()
{
arg.make_indirect_from_ignore();
}
if arg.is_ignore() && arg.layout.is_zst() {
// Windows ABIs do not talk about ZST since such types do not exist in MSVC.
// In that sense we can do whatever we want here, and maybe we should throw an error
// (but of course that would be a massive breaking change now).
// We try to match clang and gcc (which allow ZST is their windows-gnu targets), so we
// pass ZST via pointer indirection.
arg.make_indirect_from_ignore();
continue;
}
fixup(arg);
}
// FIXME: We should likely also do something about ZST return types, similar to above.
// However, that's non-trivial due to `()`.
// See <https://github.com/rust-lang/unsafe-code-guidelines/issues/552>.
}
15 changes: 10 additions & 5 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2815,12 +2815,17 @@ impl Target {
Abi::EfiApi if self.arch == "x86_64" => Abi::Win64 { unwind: false },
Abi::EfiApi => Abi::C { unwind: false },

// See commentary in `is_abi_supported`.
Abi::Stdcall { .. } | Abi::Thiscall { .. } if self.arch == "x86" => abi,
Abi::Stdcall { unwind } | Abi::Thiscall { unwind } => Abi::C { unwind },
Abi::Fastcall { .. } if self.arch == "x86" => abi,
// See commentary in `is_abi_supported`: we map these ABIs to "C" when they do not make sense.
Abi::Stdcall { .. } | Abi::Thiscall { .. } | Abi::Fastcall { .. }
if self.arch == "x86" =>
{
abi
}
Abi::Vectorcall { .. } if ["x86", "x86_64"].contains(&&self.arch[..]) => abi,
Abi::Fastcall { unwind } | Abi::Vectorcall { unwind } => Abi::C { unwind },
Abi::Stdcall { unwind }
| Abi::Thiscall { unwind }
| Abi::Fastcall { unwind }
| Abi::Vectorcall { unwind } => Abi::C { unwind },

// The Windows x64 calling convention we use for `extern "Rust"`
// <https://learn.microsoft.com/en-us/cpp/build/x64-software-conventions#register-volatility-and-preservation>
Expand Down
52 changes: 52 additions & 0 deletions tests/codegen/abi-win64-zst.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//@ compile-flags: -Z merge-functions=disabled

//@ revisions: windows-gnu
//@[windows-gnu] compile-flags: --target x86_64-pc-windows-gnu
//@[windows-gnu] needs-llvm-components: x86

//@ revisions: windows-msvc
//@[windows-msvc] compile-flags: --target x86_64-pc-windows-msvc
//@[windows-msvc] needs-llvm-components: x86

// Also test what happens when using a Windows ABI on Linux.
//@ revisions: linux
//@[linux] compile-flags: --target x86_64-unknown-linux-gnu
//@[linux] needs-llvm-components: x86

#![feature(no_core, lang_items, rustc_attrs, abi_vectorcall)]
#![no_core]
#![crate_type = "lib"]

#[lang = "sized"]
trait Sized {}

// Make sure the argument is always passed when explicitly requesting a Windows ABI.
// Our goal here is to match clang: <https://clang.godbolt.org/z/Wr4jMWq3P>.

// CHECK: define win64cc void @pass_zst_win64(ptr {{[^,]*}})
#[no_mangle]
extern "win64" fn pass_zst_win64(_: ()) {}

// CHECK: define x86_vectorcallcc void @pass_zst_vectorcall(ptr {{[^,]*}})
#[no_mangle]
extern "vectorcall" fn pass_zst_vectorcall(_: ()) {}

// windows-gnu: define void @pass_zst_fastcall(ptr {{[^,]*}})
// windows-msvc: define void @pass_zst_fastcall(ptr {{[^,]*}})
#[no_mangle]
#[cfg(windows)] // "fastcall" is not valid on 64bit Linux
extern "fastcall" fn pass_zst_fastcall(_: ()) {}

// The sysv64 ABI ignores ZST.

// CHECK: define x86_64_sysvcc void @pass_zst_sysv64()
#[no_mangle]
extern "sysv64" fn pass_zst_sysv64(_: ()) {}

// For `extern "C"` functions, ZST are ignored on Linux put passed on Windows.

// linux: define void @pass_zst_c()
// windows-msvc: define void @pass_zst_c(ptr {{[^,]*}})
// windows-gnu: define void @pass_zst_c(ptr {{[^,]*}})
#[no_mangle]
extern "C" fn pass_zst_c(_: ()) {}
24 changes: 0 additions & 24 deletions tests/ui/abi/win64-zst.rs

This file was deleted.

69 changes: 0 additions & 69 deletions tests/ui/abi/win64-zst.x86_64-linux.stderr

This file was deleted.

80 changes: 0 additions & 80 deletions tests/ui/abi/win64-zst.x86_64-windows-gnu.stderr

This file was deleted.

Loading

0 comments on commit 7a202a9

Please sign in to comment.