Skip to content

Commit

Permalink
Rollup merge of #115708 - RalfJung:homogeneous, r=davidtwco
Browse files Browse the repository at this point in the history
fix homogeneous_aggregate not ignoring some ZST

This is an ABI-breaking change, because it fixes bugs in our ABI code. I'm not sure what that means for this PR, we don't really have a process for such changes, do we? I can only hope nobody relied on the old buggy behavior.

Fixes #115664
  • Loading branch information
matthiaskrgr authored Sep 11, 2023
2 parents 5a2b589 + 254e13d commit 279e257
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 12 deletions.
13 changes: 9 additions & 4 deletions compiler/rustc_target/src/abi/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
/// only a single type (e.g., `(u32, u32)`). Such aggregates are often
/// special-cased in ABIs.
///
/// Note: We generally ignore fields of zero-sized type when computing
/// this value (see #56877).
/// Note: We generally ignore 1-ZST fields when computing this value (see #56877).
///
/// This is public so that it can be used in unit tests, but
/// should generally only be relevant to the ABI details of
Expand Down Expand Up @@ -441,12 +440,18 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
let mut total = start;

for i in 0..layout.fields.count() {
let field = layout.field(cx, i);
if field.is_1zst() {
// No data here and no impact on layout, can be ignored.
// (We might be able to also ignore all aligned ZST but that's less clear.)
continue;
}

if !is_union && total != layout.fields.offset(i) {
// This field isn't just after the previous one we considered, abort.
return Err(Heterogeneous);
}

let field = layout.field(cx, i);

result = result.merge(field.homogeneous_aggregate(cx)?)?;

// Keep track of the offset (without padding).
Expand Down
10 changes: 2 additions & 8 deletions tests/ui/abi/compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,15 @@ test_transparent!(zst, Zst);
test_transparent!(unit, ());
test_transparent!(pair, (i32, f32)); // mixing in some floats since they often get special treatment
test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit
test_transparent!(triple_f32, (f32, f32, f32)); // homogeneous case
test_transparent!(triple_f64, (f64, f64, f64));
test_transparent!(tuple, (i32, f32, i64, f64));
test_transparent!(empty_array, [u32; 0]);
test_transparent!(empty_1zst_array, [u8; 0]);
test_transparent!(small_array, [i32; 2]); // chosen to fit into 64bit
test_transparent!(large_array, [i32; 16]);
test_transparent!(enum_, Option<i32>);
test_transparent!(enum_niched, Option<&'static i32>);
// Pure-float types that are not ScalarPair seem to be tricky.
// FIXME: <https://github.com/rust-lang/rust/issues/115664>
#[cfg(not(any(target_arch = "arm", target_arch = "aarch64")))]
mod tricky {
use super::*;
test_transparent!(triple_f32, (f32, f32, f32));
test_transparent!(triple_f64, (f64, f64, f64));
}

// RFC 3391 <https://rust-lang.github.io/rfcs/3391-result_ffi_guarantees.html>.
macro_rules! test_nonnull {
Expand Down
44 changes: 44 additions & 0 deletions tests/ui/layout/homogeneous-aggr-transparent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#![feature(rustc_attrs)]
#![feature(transparent_unions)]
use std::marker::PhantomData;

// Regression test for #115664. We want to ensure that `repr(transparent)` wrappers do not affect
// the result of `homogeneous_aggregate`.

type Tuple = (f32, f32, f32);

struct Zst;

#[repr(transparent)]
struct Wrapper1<T>(T);
#[repr(transparent)]
struct Wrapper2<T>((), Zst, T);
#[repr(transparent)]
struct Wrapper3<T>(T, [u8; 0], PhantomData<u64>);
#[repr(transparent)]
union WrapperUnion<T: Copy> {
nothing: (),
something: T,
}

#[rustc_layout(homogeneous_aggregate)]
pub type Test0 = Tuple;
//~^ ERROR homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))

#[rustc_layout(homogeneous_aggregate)]
pub type Test1 = Wrapper1<Tuple>;
//~^ ERROR homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))

#[rustc_layout(homogeneous_aggregate)]
pub type Test2 = Wrapper2<Tuple>;
//~^ ERROR homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))

#[rustc_layout(homogeneous_aggregate)]
pub type Test3 = Wrapper3<Tuple>;
//~^ ERROR homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))

#[rustc_layout(homogeneous_aggregate)]
pub type Test4 = WrapperUnion<Tuple>;
//~^ ERROR homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))

fn main() {}
32 changes: 32 additions & 0 deletions tests/ui/layout/homogeneous-aggr-transparent.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))
--> $DIR/homogeneous-aggr-transparent.rs:25:1
|
LL | pub type Test0 = Tuple;
| ^^^^^^^^^^^^^^

error: homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))
--> $DIR/homogeneous-aggr-transparent.rs:29:1
|
LL | pub type Test1 = Wrapper1<Tuple>;
| ^^^^^^^^^^^^^^

error: homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))
--> $DIR/homogeneous-aggr-transparent.rs:33:1
|
LL | pub type Test2 = Wrapper2<Tuple>;
| ^^^^^^^^^^^^^^

error: homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))
--> $DIR/homogeneous-aggr-transparent.rs:37:1
|
LL | pub type Test3 = Wrapper3<Tuple>;
| ^^^^^^^^^^^^^^

error: homogeneous_aggregate: Ok(Homogeneous(Reg { kind: Float, size: Size(4 bytes) }))
--> $DIR/homogeneous-aggr-transparent.rs:41:1
|
LL | pub type Test4 = WrapperUnion<Tuple>;
| ^^^^^^^^^^^^^^

error: aborting due to 5 previous errors

0 comments on commit 279e257

Please sign in to comment.