Skip to content

Commit

Permalink
Rollup merge of rust-lang#125854 - beetrees:zst-arg-abi, r=estebank
Browse files Browse the repository at this point in the history 
Move ZST ABI handling to `rustc_target`

Currently, target specific handling of ZST function call ABI (specifically passing them indirectly instead of ignoring them) is handled in `rustc_ty_utils`, whereas all other target specific function call ABI handling is located in `rustc_target`. This PR moves the ZST handling to `rustc_target` so that all the target-specific function call ABI handling is in one place. In the process of doing so, this PR fixes rust-lang#125850 by ensuring that ZST arguments are always correctly ignored in the x86-64 `"sysv64"` ABI; any code which would be affected by this fix would have ICEd before this PR. Tests are also added using `#[rustc_abi(debug)]` to ensure this behaviour does not regress.

Fixes rust-lang#125850
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Aug 18, 2024
2 parents 6de928d + b1493ba commit 8fa089b
Show file tree
Hide file tree
Showing 19 changed files with 766 additions and 43 deletions.
13 changes: 9 additions & 4 deletions compiler/rustc_target/src/abi/call/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
pub fn make_indirect(&mut self) {
match self.mode {
PassMode::Direct(_) | PassMode::Pair(_, _) => {
self.mode = Self::indirect_pass_mode(&self.layout);
self.make_indirect_force();
}
PassMode::Indirect { attrs: _, meta_attrs: _, on_stack: false } => {
// already indirect
Expand All @@ -652,6 +652,11 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
}
}

/// Same as make_indirect, but doesn't check the current `PassMode`.
pub fn make_indirect_force(&mut self) {
self.mode = Self::indirect_pass_mode(&self.layout);
}

/// Pass this argument indirectly, by placing it at a fixed stack offset.
/// This corresponds to the `byval` LLVM argument attribute.
/// This is only valid for sized arguments.
Expand Down Expand Up @@ -871,10 +876,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
}
"x86_64" => match abi {
spec::abi::Abi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
spec::abi::Abi::Win64 { .. } => x86_win64::compute_abi_info(self),
spec::abi::Abi::Win64 { .. } => x86_win64::compute_abi_info(cx, self),
_ => {
if cx.target_spec().is_like_windows {
x86_win64::compute_abi_info(self)
x86_win64::compute_abi_info(cx, self)
} else {
x86_64::compute_abi_info(cx, self)
}
Expand All @@ -898,7 +903,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
"csky" => csky::compute_abi_info(self),
"mips" | "mips32r6" => mips::compute_abi_info(cx, self),
"mips64" | "mips64r6" => mips64::compute_abi_info(cx, self),
"powerpc" => powerpc::compute_abi_info(self),
"powerpc" => powerpc::compute_abi_info(cx, self),
"powerpc64" => powerpc64::compute_abi_info(cx, self),
"s390x" => s390x::compute_abi_info(cx, self),
"msp430" => msp430::compute_abi_info(self),
Expand Down
20 changes: 14 additions & 6 deletions compiler/rustc_target/src/abi/call/powerpc.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::abi::call::{ArgAbi, FnAbi};
use crate::spec::HasTargetSpec;

fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
if ret.layout.is_aggregate() {
Expand All @@ -8,23 +9,30 @@ fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
}
}

fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
fn classify_arg<Ty>(cx: &impl HasTargetSpec, arg: &mut ArgAbi<'_, Ty>) {
if arg.is_ignore() {
// powerpc-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
if cx.target_spec().os == "linux"
&& matches!(&*cx.target_spec().env, "gnu" | "musl" | "uclibc")
&& arg.layout.is_zst()
{
arg.make_indirect_force();
}
return;
}
if arg.layout.is_aggregate() {
arg.make_indirect();
} else {
arg.extend_integer_width_to(32);
}
}

pub fn compute_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
pub fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
if !fn_abi.ret.is_ignore() {
classify_ret(&mut fn_abi.ret);
}

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}
classify_arg(arg);
classify_arg(cx, arg);
}
}
18 changes: 13 additions & 5 deletions compiler/rustc_target/src/abi/call/s390x.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use crate::abi::call::{ArgAbi, FnAbi, Reg};
use crate::abi::{HasDataLayout, TyAbiInterface};
use crate::spec::HasTargetSpec;

fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
if !ret.layout.is_aggregate() && ret.layout.size.bits() <= 64 {
Expand All @@ -15,12 +16,22 @@ fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
C: HasDataLayout + HasTargetSpec,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if arg.is_ignore() {
// s390x-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
if cx.target_spec().os == "linux"
&& matches!(&*cx.target_spec().env, "gnu" | "musl" | "uclibc")
&& arg.layout.is_zst()
{
arg.make_indirect_force();
}
return;
}
if !arg.layout.is_aggregate() && arg.layout.size.bits() <= 64 {
arg.extend_integer_width_to(64);
return;
Expand All @@ -46,16 +57,13 @@ where
pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
C: HasDataLayout + HasTargetSpec,
{
if !fn_abi.ret.is_ignore() {
classify_ret(&mut fn_abi.ret);
}

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}
classify_arg(cx, arg);
}
}
12 changes: 10 additions & 2 deletions compiler/rustc_target/src/abi/call/sparc64.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::abi::call::{
ArgAbi, ArgAttribute, ArgAttributes, ArgExtension, CastTarget, FnAbi, Reg, Uniform,
};
use crate::abi::{self, HasDataLayout, Scalar, Size, TyAbiInterface, TyAndLayout};
use crate::spec::HasTargetSpec;

#[derive(Clone, Debug)]
pub struct Sdata {
Expand Down Expand Up @@ -211,15 +212,22 @@ where
pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
C: HasDataLayout + HasTargetSpec,
{
if !fn_abi.ret.is_ignore() {
classify_arg(cx, &mut fn_abi.ret, Size::from_bytes(32));
}

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
// sparc64-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
if cx.target_spec().os == "linux"
&& matches!(&*cx.target_spec().env, "gnu" | "musl" | "uclibc")
&& arg.layout.is_zst()
{
arg.make_indirect_force();
}
return;
}
classify_arg(cx, arg, Size::from_bytes(16));
}
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_target/src/abi/call/x86_win64.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::abi::call::{ArgAbi, FnAbi, Reg};
use crate::abi::{Abi, Float, Primitive};
use crate::spec::HasTargetSpec;

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

pub fn compute_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
pub fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
let fixup = |a: &mut ArgAbi<'_, Ty>| {
match a.layout.abi {
Abi::Uninhabited | Abi::Aggregate { sized: false } => {}
Expand Down Expand Up @@ -37,6 +38,13 @@ pub fn compute_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
}
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_force();
}
continue;
}
fixup(arg);
Expand Down
27 changes: 2 additions & 25 deletions compiler/rustc_ty_utils/src/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ fn fn_abi_new_uncached<'tcx>(
let conv = conv_from_spec_abi(cx.tcx(), sig.abi, sig.c_variadic);

let mut inputs = sig.inputs();
let extra_args = if sig.abi == RustCall {
let extra_args = if sig.abi == SpecAbi::RustCall {
assert!(!sig.c_variadic && extra_args.is_empty());

if let Some(input) = sig.inputs().last() {
Expand All @@ -608,18 +608,6 @@ fn fn_abi_new_uncached<'tcx>(
extra_args
};

let target = &cx.tcx.sess.target;
let target_env_gnu_like = matches!(&target.env[..], "gnu" | "musl" | "uclibc");
let win_x64_gnu = target.os == "windows" && target.arch == "x86_64" && target.env == "gnu";
let linux_s390x_gnu_like =
target.os == "linux" && target.arch == "s390x" && target_env_gnu_like;
let linux_sparc64_gnu_like =
target.os == "linux" && target.arch == "sparc64" && target_env_gnu_like;
let linux_powerpc_gnu_like =
target.os == "linux" && target.arch == "powerpc" && target_env_gnu_like;
use SpecAbi::*;
let rust_abi = matches!(sig.abi, RustIntrinsic | Rust | RustCall);

let is_drop_in_place =
fn_def_id.is_some_and(|def_id| cx.tcx.is_lang_item(def_id, LangItem::DropInPlace));

Expand Down Expand Up @@ -659,18 +647,7 @@ fn fn_abi_new_uncached<'tcx>(
});

if arg.layout.is_zst() {
// For some forsaken reason, x86_64-pc-windows-gnu
// doesn't ignore zero-sized struct arguments.
// The same is true for {s390x,sparc64,powerpc}-unknown-linux-{gnu,musl,uclibc}.
if is_return
|| rust_abi
|| (!win_x64_gnu
&& !linux_s390x_gnu_like
&& !linux_sparc64_gnu_like
&& !linux_powerpc_gnu_like)
{
arg.mode = PassMode::Ignore;
}
arg.mode = PassMode::Ignore;
}

Ok(arg)
Expand Down
5 changes: 5 additions & 0 deletions src/tools/compiletest/src/command-list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,12 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"ignore-nvptx64-nvidia-cuda",
"ignore-openbsd",
"ignore-pass",
"ignore-powerpc",
"ignore-remote",
"ignore-riscv64",
"ignore-s390x",
"ignore-sgx",
"ignore-sparc64",
"ignore-spirv",
"ignore-stable",
"ignore-stage1",
Expand Down Expand Up @@ -123,6 +125,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"ignore-x86",
"ignore-x86_64",
"ignore-x86_64-apple-darwin",
"ignore-x86_64-pc-windows-gnu",
"ignore-x86_64-unknown-linux-gnu",
"incremental",
"known-bug",
Expand Down Expand Up @@ -191,7 +194,9 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"only-msvc",
"only-nightly",
"only-nvptx64",
"only-powerpc",
"only-riscv64",
"only-s390x",
"only-sparc",
"only-sparc64",
"only-stable",
Expand Down
67 changes: 67 additions & 0 deletions tests/ui/abi/c-zst.other-linux.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
error: fn_abi_of(pass_zst) = FnAbi {
args: [
ArgAbi {
layout: TyAndLayout {
ty: (),
layout: Layout {
size: Size(0 bytes),
align: AbiAndPrefAlign {
abi: $SOME_ALIGN,
pref: $SOME_ALIGN,
},
abi: Aggregate {
sized: true,
},
fields: Arbitrary {
offsets: [],
memory_index: [],
},
largest_niche: None,
variants: Single {
index: 0,
},
max_repr_align: None,
unadjusted_abi_align: $SOME_ALIGN,
},
},
mode: Ignore,
},
],
ret: ArgAbi {
layout: TyAndLayout {
ty: (),
layout: Layout {
size: Size(0 bytes),
align: AbiAndPrefAlign {
abi: $SOME_ALIGN,
pref: $SOME_ALIGN,
},
abi: Aggregate {
sized: true,
},
fields: Arbitrary {
offsets: [],
memory_index: [],
},
largest_niche: None,
variants: Single {
index: 0,
},
max_repr_align: None,
unadjusted_abi_align: $SOME_ALIGN,
},
},
mode: Ignore,
},
c_variadic: false,
fixed_count: 1,
conv: C,
can_unwind: false,
}
--> $DIR/c-zst.rs:27:1
|
LL | extern "C" fn pass_zst(_: ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

Loading

0 comments on commit 8fa089b

Please sign in to comment.