From 56619f32abcd35ca7c0e5595f0feb6dfac5905f5 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Sat, 25 Jul 2026 20:56:04 -0400 Subject: [PATCH 1/9] Fix overaligned argument --- src/abi.rs | 13 +++++++--- tests/run/overaligned_byval_arg.rs | 41 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 tests/run/overaligned_byval_arg.rs diff --git a/src/abi.rs b/src/abi.rs index 45fc5e3c4f6..2ae60e238ec 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -1,5 +1,5 @@ #[cfg(feature = "master")] -use gccjit::FnAttribute; +use gccjit::{FnAttribute, TypeAttribute}; use gccjit::{ToLValue, ToRValue, Type}; #[cfg(feature = "master")] use rustc_abi::{ArmCall, CanonAbi, InterruptKind, X86Call}; @@ -187,7 +187,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { let ty = cast.gcc_type(cx); apply_attrs(ty, &cast.attrs, argument_tys.len()) } - PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: true } => { + PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => { let x86_interrupt_first_arg = { #[cfg(feature = "master")] { @@ -211,7 +211,14 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { } else { // This is a "byval" argument, so we don't apply the `restrict` attribute on it. on_stack_param_indices.insert(argument_tys.len()); - arg.layout.gcc_type(cx) + let ty = arg.layout.gcc_type(cx); + #[cfg(feature = "master")] + if let Some(align) = attrs.pointee_align { + ty.add_attribute(TypeAttribute::Aligned(align.bytes() as u8)); + } + #[cfg(not(feature = "master"))] + let _ = attrs; + ty } } PassMode::Direct(attrs) => { diff --git a/tests/run/overaligned_byval_arg.rs b/tests/run/overaligned_byval_arg.rs new file mode 100644 index 00000000000..e20ec117329 --- /dev/null +++ b/tests/run/overaligned_byval_arg.rs @@ -0,0 +1,41 @@ +// Compiler: +// +// Run-time: +// status: 0 + +#![feature(no_core)] +#![no_std] +#![no_core] +#![no_main] + +extern crate mini_core; +use mini_core::*; + +#[repr(C)] +struct Big { + a: i64, + b: i64, + c: i64, +} + +#[repr(C, align(64))] +struct Aligned { + x: i32, +} + +#[inline(never)] +#[no_mangle] +extern "C" fn check(_b1: Big, a1: Aligned, _b2: Big, a2: Aligned) -> i32 { + if (&a1 as *const Aligned as usize) % 64 != 0 { + return 1; + } + if (&a2 as *const Aligned as usize) % 64 != 0 { + return 2; + } + 0 +} + +#[no_mangle] +extern "C" fn main(_argc: i32, _argv: *const *const u8) -> i32 { + check(Big { a: 1, b: 2, c: 3 }, Aligned { x: 42 }, Big { a: 4, b: 5, c: 6 }, Aligned { x: 43 }) +} From a7e1f0552e50d56b7274764d0948cfe0d4ada3d1 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Mon, 3 Aug 2026 17:36:18 -0400 Subject: [PATCH 2/9] Add -Wno-psabi to silent a warning --- src/gcc_util.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gcc_util.rs b/src/gcc_util.rs index d986dc68e67..56314dca5ef 100644 --- a/src/gcc_util.rs +++ b/src/gcc_util.rs @@ -193,6 +193,8 @@ pub fn new_context<'gcc>(sess: &Session) -> Context<'gcc> { context.add_command_line_option("-fno-strict-aliasing"); // NOTE: Rust relies on LLVM doing wrapping on overflow. context.add_command_line_option("-fwrapv"); + // NOTE: This is needed to hide a warning caused by the alignment fix on byval arguments. + context.add_command_line_option("-Wno-psabi"); if let Some(model) = sess.code_model() { use rustc_target::spec::CodeModel; From 20fbf8564ca6b70784c884050e4895c40244dbbd Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Mon, 3 Aug 2026 17:38:26 -0400 Subject: [PATCH 3/9] Update gccjit dependency --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- src/abi.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 060509e51a6..cca0e1e590e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,9 +56,9 @@ dependencies = [ [[package]] name = "gccjit" -version = "4.0.0" +version = "4.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be5dafc4e649cb4a363e95a5960ef50b0c6f1b8e136ff8eb2e928b40353b5d8b" +checksum = "859af1dd2815fd0f8ca97f5917a595f18c415692b07e58993a6ad34ff13204d5" dependencies = [ "gccjit_sys", ] diff --git a/Cargo.toml b/Cargo.toml index 63a20d46b9d..141949e9189 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ default = ["master"] [dependencies] object = { version = "0.37.0", default-features = false, features = ["std", "read"] } tempfile = "3.20" -gccjit = { version = "4.0.0", features = ["dlopen"] } +gccjit = { version = "4.1.0", features = ["dlopen"] } #gccjit = { git = "https://github.com/rust-lang/gccjit.rs", branch = "error-dlopen", features = ["dlopen"] } # Local copy. diff --git a/src/abi.rs b/src/abi.rs index 2ae60e238ec..445da17dc39 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -214,7 +214,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { let ty = arg.layout.gcc_type(cx); #[cfg(feature = "master")] if let Some(align) = attrs.pointee_align { - ty.add_attribute(TypeAttribute::Aligned(align.bytes() as u8)); + ty.add_attribute(TypeAttribute::Aligned(align.bytes() as u32)); } #[cfg(not(feature = "master"))] let _ = attrs; From 599ab528cae362e517d84d5b7af3786cd55417c5 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 4 Aug 2026 10:26:14 -0400 Subject: [PATCH 4/9] Add packed and aligned in type cache, add real test using C --- src/abi.rs | 24 ++++---- src/common.rs | 4 +- src/context.rs | 4 +- src/type_.rs | 71 ++++++++++++++++++++-- src/type_of.rs | 9 +-- tests/c/overaligned_byval_abi.c | 52 ++++++++++++++++ tests/lang_tests.rs | 97 +++++++++++++++++++++++++++--- tests/run/overaligned_byval_abi.rs | 89 +++++++++++++++++++++++++++ 8 files changed, 320 insertions(+), 30 deletions(-) create mode 100644 tests/c/overaligned_byval_abi.c create mode 100644 tests/run/overaligned_byval_abi.rs diff --git a/src/abi.rs b/src/abi.rs index 445da17dc39..ba798bab83f 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -1,5 +1,5 @@ #[cfg(feature = "master")] -use gccjit::{FnAttribute, TypeAttribute}; +use gccjit::FnAttribute; use gccjit::{ToLValue, ToRValue, Type}; #[cfg(feature = "master")] use rustc_abi::{ArmCall, CanonAbi, InterruptKind, X86Call}; @@ -73,7 +73,9 @@ impl GccType for CastTarget { args.push(cx.type_ix(rem_bytes * 8)); } - cx.type_struct(&args, false) + // A cast target describes registers, so its alignment is whatever GCC computes from + // them rather than the alignment of the Rust type being cast. + cx.type_struct(&args, false, None) } } @@ -187,7 +189,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { let ty = cast.gcc_type(cx); apply_attrs(ty, &cast.attrs, argument_tys.len()) } - PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => { + PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: true } => { let x86_interrupt_first_arg = { #[cfg(feature = "master")] { @@ -210,15 +212,15 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { cx.type_ptr_to(arg.layout.gcc_type(cx)) } else { // This is a "byval" argument, so we don't apply the `restrict` attribute on it. + // + // GCC picks the argument's stack slot from the alignment of this type, + // which `LayoutGccExt::gcc_type` sets from `layout.align.abi`. We + // deliberately do not use `attrs.pointee_align` here: when it differs + // from the type's alignment it describes the *slot*, not the type, and + // rustc already copies the argument to a sufficiently aligned alloca on + // whichever side needs it. on_stack_param_indices.insert(argument_tys.len()); - let ty = arg.layout.gcc_type(cx); - #[cfg(feature = "master")] - if let Some(align) = attrs.pointee_align { - ty.add_attribute(TypeAttribute::Aligned(align.bytes() as u32)); - } - #[cfg(not(feature = "master"))] - let _ = attrs; - ty + arg.layout.gcc_type(cx) } } PassMode::Direct(attrs) => { diff --git a/src/common.rs b/src/common.rs index 6f9d22885b2..5b89fc7aa63 100644 --- a/src/common.rs +++ b/src/common.rs @@ -288,7 +288,9 @@ impl<'gcc, 'tcx> ConstCodegenMethods for CodegenCx<'gcc, 'tcx> { fn const_struct(&self, values: &[RValue<'gcc>], packed: bool) -> RValue<'gcc> { let fields: Vec<_> = values.iter().map(|value| value.get_type()).collect(); // FIXME(antoyo): cache the type? It's anonymous, so probably not. - let typ = self.type_struct(&fields, packed); + // The alignment of a constant aggregate is the one GCC derives from its fields: the Rust + // layout this comes from is not available here. + let typ = self.type_struct(&fields, packed, None); let struct_type = typ.is_struct().expect("struct type"); self.context.new_struct_constructor(None, struct_type.as_type(), None, values) } diff --git a/src/context.rs b/src/context.rs index 19fbe37c27b..d971b7f32de 100644 --- a/src/context.rs +++ b/src/context.rs @@ -26,6 +26,7 @@ use rustc_target::spec::{HasTargetSpec, HasX86AbiOpt, Target, TlsModel, X86Abi}; use crate::abi::conv_to_fn_attribute; use crate::callee::get_fn; use crate::common::SignType; +use crate::type_::StructTypeKey; #[cfg_attr(not(feature = "master"), expect(dead_code))] pub struct CodegenCx<'gcc, 'tcx> { @@ -85,7 +86,8 @@ pub struct CodegenCx<'gcc, 'tcx> { pub types: RefCell, Option), Type<'gcc>>>, pub tcx: TyCtxt<'tcx>, - pub struct_types: RefCell>, Type<'gcc>>>, + /// Cache of the anonymous struct types. + pub struct_types: RefCell, Type<'gcc>>>, /// Cache instances of monomorphic and polymorphic items pub instances: RefCell, LValue<'gcc>>>, diff --git a/src/type_.rs b/src/type_.rs index f008be67e39..e0f6d72d8fd 100644 --- a/src/type_.rs +++ b/src/type_.rs @@ -101,9 +101,15 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { self.bool_type } - pub fn type_struct(&self, fields: &[Type<'gcc>], packed: bool) -> Type<'gcc> { - let types = fields.to_vec(); - if let Some(typ) = self.struct_types.borrow().get(fields) { + pub fn type_struct( + &self, + fields: &[Type<'gcc>], + packed: bool, + align: Option, + ) -> Type<'gcc> { + let align = normalize_struct_alignment(align); + let key = StructTypeKey { fields: fields.to_vec(), packed, align }; + if let Some(typ) = self.struct_types.borrow().get(&key) { return *typ; } let fields: Vec<_> = fields @@ -118,11 +124,59 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { #[cfg(feature = "master")] typ.add_attribute(TypeAttribute::Packed); } - self.struct_types.borrow_mut().insert(types, typ); + set_struct_alignment(typ, align); + self.struct_types.borrow_mut().insert(key, typ); typ } } +/// Identifies an anonymous struct type in `CodegenCx::struct_types`. +/// +/// Everything that can make two of them distinct has to be part of it. In particular the +/// alignment: two Rust types can have the same field list and still differ in alignment (for +/// instance `struct { a: u64, b: u64 }` with and without `repr(align(16))`), and they must not +/// end up sharing a GCC type. +#[derive(Clone, Eq, Hash, PartialEq)] +pub struct StructTypeKey<'gcc> { + pub fields: Vec>, + pub packed: bool, + pub align: Option, +} + +/// Discard an alignment that GCC would give the struct anyway. +/// +/// One byte is the minimum alignment of a GCC struct, so requesting it explicitly changes +/// nothing; mapping it to `None` keeps types that do not care about their alignment sharing a +/// single entry in `CodegenCx::struct_types`. +fn normalize_struct_alignment(align: Option) -> Option { + align.filter(|align| align.bytes() > 1) +} + +/// Give a struct type the alignment that Rust computed for it. +/// +/// GCC derives a struct's alignment from its field list, so a `repr(align(N))` that is larger +/// than what the fields require would otherwise be lost. That is not only a layout concern: the +/// ABI of a by-value ("byval") argument depends on it, since `ix86_function_arg_boundary` reads +/// `TYPE_ALIGN` to pick the argument's stack slot. An over-aligned aggregate whose GCC type has +/// lost its alignment is therefore passed at an offset a C caller does not agree on. +/// +/// This has to be set on the struct type itself. `Type::get_aligned` is not enough: it builds a +/// type *variant*, and the argument-passing code looks at `TYPE_MAIN_VARIANT` first, which +/// discards it. +/// +/// This never under-aligns a struct whose fields need more: GCC starts the record layout from +/// `TYPE_ALIGN` and the fields can only raise it. +#[cfg(feature = "master")] +fn set_struct_alignment(typ: Type<'_>, align: Option) { + if let Some(align) = normalize_struct_alignment(align) { + typ.add_attribute(TypeAttribute::Aligned(align.bytes() as u32)); + } +} + +/// Without the `master` feature, libgccjit has no way to set a type's alignment. +#[cfg(not(feature = "master"))] +fn set_struct_alignment(_typ: Type<'_>, _align: Option) {} + impl<'gcc, 'tcx> BaseTypeCodegenMethods for CodegenCx<'gcc, 'tcx> { fn type_i8(&self) -> Type<'gcc> { self.i8_type @@ -324,7 +378,13 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { self.type_array(self.type_from_integer(unit), size / unit_size) } - pub fn set_struct_body(&self, typ: Struct<'gcc>, fields: &[Type<'gcc>], packed: bool) { + pub fn set_struct_body( + &self, + typ: Struct<'gcc>, + fields: &[Type<'gcc>], + packed: bool, + align: Option, + ) { let fields: Vec<_> = fields .iter() .enumerate() @@ -335,6 +395,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { #[cfg(feature = "master")] typ.as_type().add_attribute(TypeAttribute::Packed); } + set_struct_alignment(typ.as_type(), align); } pub fn type_named_struct(&self, name: &str) -> Struct<'gcc> { diff --git a/src/type_of.rs b/src/type_of.rs index c6c32236ab4..da9a660e73d 100644 --- a/src/type_of.rs +++ b/src/type_of.rs @@ -82,6 +82,7 @@ fn uncached_gcc_type<'gcc, 'tcx>( layout.scalar_pair_element_gcc_type(cx, 1), ], false, + Some(layout.align.abi), ); } BackendRepr::Memory { .. } => {} @@ -130,10 +131,10 @@ fn uncached_gcc_type<'gcc, 'tcx>( let fill = cx.type_padding_filler(layout.size, layout.align.abi); let packed = false; match name { - None => cx.type_struct(&[fill], packed), + None => cx.type_struct(&[fill], packed, Some(layout.align.abi)), Some(ref name) => { let gcc_type = cx.type_named_struct(name); - cx.set_struct_body(gcc_type, &[fill], packed); + cx.set_struct_body(gcc_type, &[fill], packed, Some(layout.align.abi)); gcc_type.as_type() } } @@ -142,7 +143,7 @@ fn uncached_gcc_type<'gcc, 'tcx>( FieldsShape::Arbitrary { .. } => match name { None => { let (gcc_fields, packed) = struct_fields(cx, layout); - cx.type_struct(&gcc_fields, packed) + cx.type_struct(&gcc_fields, packed, Some(layout.align.abi)) } Some(ref name) => { let gcc_type = cx.type_named_struct(name); @@ -240,7 +241,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> { if let Some((deferred_ty, layout)) = defer { let (fields, packed) = struct_fields(cx, layout); - cx.set_struct_body(deferred_ty, &fields, packed); + cx.set_struct_body(deferred_ty, &fields, packed, Some(layout.align.abi)); } ty diff --git a/tests/c/overaligned_byval_abi.c b/tests/c/overaligned_byval_abi.c new file mode 100644 index 00000000000..e7575871e06 --- /dev/null +++ b/tests/c/overaligned_byval_abi.c @@ -0,0 +1,52 @@ +/* Reference side of `tests/run/overaligned_byval_abi.rs`, compiled by the real GCC. + * + * `Aligned` is an over-aligned aggregate passed by value ("byval"): the ABI places it in a stack + * slot aligned to its own alignment, not packed right after the preceding argument. cg_gcc used + * to build the GCC struct type from the field list alone, which dropped Rust's `repr(align(64))`, + * so it placed the argument at an offset nobody else agreed on. + * + * The two functions here check both directions: `c_take_both` is a GCC-built callee for a cg_gcc + * caller, and `c_call_rust` is a GCC-built caller for a cg_gcc callee. + * + * The checks are on the *values* received rather than on the address of the argument: which + * alignment the ABI gives a stack slot is target-specific, but caller and callee agreeing on it + * is not. A disagreement makes the arguments arrive as garbage. */ + +struct Big { + long a, b, c; +}; + +struct __attribute__((aligned(64))) Aligned { + int x; +}; + +/* Defined on the Rust side. */ +extern int rust_take_both(struct Big first, struct Aligned second, struct Big third, + struct Aligned fourth); + +/* Called from Rust: checks what a cg_gcc caller passed. */ +int c_take_both(struct Big first, struct Aligned second, struct Big third, + struct Aligned fourth) +{ + if (first.a != 1 || first.b != 2 || first.c != 3) + return 1; + if (second.x != 42) + return 2; + if (third.a != 4 || third.b != 5 || third.c != 6) + return 3; + if (fourth.x != 43) + return 4; + return 0; +} + +/* Called from Rust: passes the arguments the way the ABI says, for a cg_gcc callee to read. */ +int c_call_rust(void) +{ + struct Big first = {1, 2, 3}; + struct Big third = {4, 5, 6}; + struct Aligned second, fourth; + + second.x = 42; + fourth.x = 43; + return rust_take_both(first, second, third, fourth); +} diff --git a/tests/lang_tests.rs b/tests/lang_tests.rs index f3b4ad34bc9..5426d777bb4 100644 --- a/tests/lang_tests.rs +++ b/tests/lang_tests.rs @@ -7,6 +7,74 @@ use std::process::Command; use lang_tester::LangTester; use tempfile::TempDir; +/// Directory holding the C files that the `tests/run` tests can link against. +/// +/// A `tests/c/.c` is compiled by the real GCC and linked into `tests/run/.rs`. +/// This is what makes it possible to test the ABI: with cg_gcc on both sides of a call, caller +/// and callee agree even when they are both wrong, so a pure-Rust test cannot notice. A C +/// caller or callee built by GCC is an independent reference. +const C_TESTS_DIR: &str = "tests/c"; + +/// The m68k cross toolchain is not on the default `PATH` in CI. +// FIXME(antoyo): find a better way to add the PATH necessary locally. +const M68K_TOOLCHAIN_DIR: &str = "/opt/m68k-unknown-linux-gnu/bin"; + +fn target_path(test_target: &Option) -> Option { + test_target.as_ref().map(|_| { + let env_path = std::env::var("PATH").unwrap_or_default(); + format!("{}:{}", M68K_TOOLCHAIN_DIR, env_path) + }) +} + +/// Compile every C file in `tests/c` to an object file in `objects_dir`, named after the C file. +/// +/// The C files are compiled by the real GCC (the cross one when testing another target), not by +/// cg_gcc: they are the reference the Rust side is checked against. +fn compile_c_files(objects_dir: &Path, test_target: &Option) { + let c_tests_dir = Path::new(C_TESTS_DIR); + if !c_tests_dir.is_dir() { + return; + } + std::fs::create_dir_all(objects_dir).expect("create the directory for the C object files"); + + let compiler = match test_target { + Some(target) => format!("{}-gcc", target), + None => "gcc".to_string(), + }; + + for entry in std::fs::read_dir(c_tests_dir).expect("read the C tests directory") { + let source = entry.expect("directory entry").path(); + if source.extension().and_then(|extension| extension.to_str()) != Some("c") { + continue; + } + let object = c_object_path(objects_dir, &source); + + let mut command = Command::new(&compiler); + command.arg("-c"); + // Optimize: an unoptimized C caller can happen to agree with a wrong callee. + command.arg("-O1"); + // GCC notes that the ABI of over-aligned arguments changed in GCC 4.6. That is the ABI + // being tested here, so the note is expected rather than a problem. + command.arg("-Wno-psabi"); + command.arg("-o"); + command.arg(&object); + command.arg(&source); + if let Some(env_path) = target_path(test_target) { + command.env("PATH", env_path); + } + + let status = command + .status() + .unwrap_or_else(|error| panic!("failed to run `{}`: {}", compiler, error)); + assert!(status.success(), "failed to compile `{}`", source.display()); + } +} + +/// The object file that a test source links against, if any: `tests/c/x.c` for `tests/run/x.rs`. +fn c_object_path(objects_dir: &Path, source: &Path) -> PathBuf { + objects_dir.join(source.file_stem().expect("file_stem")).with_extension("o") +} + fn compile_and_run_cmds( compiler_args: Vec, test_target: &Option, @@ -18,10 +86,7 @@ fn compile_and_run_cmds( // Test command 2: run `tempdir/x`. if test_target.is_some() { - let mut env_path = std::env::var("PATH").unwrap_or_default(); - // FIXME(antoyo): find a better way to add the PATH necessary locally. - env_path = format!("/opt/m68k-unknown-linux-gnu/bin:{}", env_path); - compiler.env("PATH", env_path); + compiler.env("PATH", target_path(test_target).expect("target PATH")); let mut commands = vec![("Compiler", compiler)]; if test_mode.should_run() { @@ -84,6 +149,7 @@ impl TestMode { fn build_test_runner( tempdir: PathBuf, + c_objects_dir: PathBuf, current_dir: String, build_mode: BuildMode, test_kind: &str, @@ -159,6 +225,13 @@ fn build_test_runner( path.to_str().expect("to_str").into(), ]; + // Link against `tests/c/.c`, when the test has one. + let c_object = c_object_path(&c_objects_dir, path); + if c_object.exists() { + compiler_args.push("-C".into()); + compiler_args.push(format!("link-arg={}", c_object.display())); + } + if let Some(ref target) = test_target { compiler_args.extend_from_slice(&["--target".into(), target.into()]); @@ -203,9 +276,10 @@ fn build_test_runner( .run(); } -fn compile_tests(tempdir: PathBuf, current_dir: String) { +fn compile_tests(tempdir: PathBuf, c_objects_dir: PathBuf, current_dir: String) { build_test_runner( tempdir, + c_objects_dir, current_dir, BuildMode::Debug, "lang compile", @@ -221,9 +295,10 @@ fn compile_tests(tempdir: PathBuf, current_dir: String) { ); } -fn run_tests(tempdir: PathBuf, current_dir: String) { +fn run_tests(tempdir: PathBuf, c_objects_dir: PathBuf, current_dir: String) { build_test_runner( tempdir.clone(), + c_objects_dir.clone(), current_dir.clone(), BuildMode::Debug, "[DEBUG] lang run", @@ -233,6 +308,7 @@ fn run_tests(tempdir: PathBuf, current_dir: String) { ); build_test_runner( tempdir, + c_objects_dir, current_dir.to_string(), BuildMode::Release, "[RELEASE] lang run", @@ -248,6 +324,11 @@ fn main() { let current_dir = current_dir.to_str().expect("current dir").to_string(); let tempdir_path: PathBuf = tempdir.as_ref().into(); - compile_tests(tempdir_path.clone(), current_dir.clone()); - run_tests(tempdir_path, current_dir); + let c_objects_dir = tempdir_path.join("c-objects"); + // FIXME(antoyo): find a way to send this via a cli argument. + let test_target = std::env::var("CG_GCC_TEST_TARGET").ok(); + compile_c_files(&c_objects_dir, &test_target); + + compile_tests(tempdir_path.clone(), c_objects_dir.clone(), current_dir.clone()); + run_tests(tempdir_path, c_objects_dir, current_dir); } diff --git a/tests/run/overaligned_byval_abi.rs b/tests/run/overaligned_byval_abi.rs new file mode 100644 index 00000000000..78ee35f05ca --- /dev/null +++ b/tests/run/overaligned_byval_abi.rs @@ -0,0 +1,89 @@ +// Compiler: +// +// Run-time: +// status: 0 + +// Checks that cg_gcc passes an over-aligned by-value ("byval") argument where the platform ABI +// says it goes, by calling in both directions with `tests/c/overaligned_byval_abi.c`, which is +// compiled by the real GCC. +// +// `tests/run/overaligned_byval_arg.rs` covers the Rust-visible half of the same bug. It cannot +// cover this one: with cg_gcc on both sides of a call, caller and callee place the argument at +// the same wrong offset and agree with each other. +// +// Two over-aligned arguments are used rather than one so that the failure is deterministic. A +// backend that drops `align(64)` packs the arguments at offsets 0, 24, 88 and 112 of the argument +// area; 112 - 24 = 88 is not a multiple of 64, so the two of them cannot both land on a 64-byte +// boundary however the argument area itself is aligned. With a single over-aligned argument the +// frame often happens to be 64-aligned and the bug hides. +// +// Only the values received are checked, never the address an argument landed at: which alignment +// a target gives a by-value stack slot differs between targets, but the two sides of a call +// agreeing on it does not. `overaligned_byval_arg.rs` is where the alignment itself is asserted. + +#![feature(no_core)] +#![no_std] +#![no_core] +#![no_main] + +extern crate mini_core; +use mini_core::*; + +#[repr(C)] +struct Big { + a: i64, + b: i64, + c: i64, +} + +#[repr(C, align(64))] +struct Aligned { + x: i32, +} + +extern "C" { + fn c_take_both(first: Big, second: Aligned, third: Big, fourth: Aligned) -> i32; + fn c_call_rust() -> i32; +} + +// The callee for the GCC-built caller in `c_call_rust`. +// +// `#[no_mangle]` is not only about the symbol name: it makes the symbol externally visible, which +// pins the calling convention. Without it the function has internal linkage and GCC is free to +// clone it with a changed convention at `-O3` (the symbol comes out as `...constprop.0.isra.0`), +// so the arguments never travel through the stack slots and the release build passes spuriously. +#[no_mangle] +extern "C" fn rust_take_both(first: Big, second: Aligned, third: Big, fourth: Aligned) -> i32 { + if first.a as i32 != 1 || first.b as i32 != 2 || first.c as i32 != 3 { + return 5; + } + if second.x != 42 { + return 6; + } + if third.a as i32 != 4 || third.b as i32 != 5 || third.c as i32 != 6 { + return 7; + } + if fourth.x != 43 { + return 8; + } + 0 +} + +#[no_mangle] +extern "C" fn main(_argc: i32, _argv: *const *const u8) -> i32 { + // cg_gcc as the caller, GCC as the callee. + let result = unsafe { + c_take_both( + Big { a: 1, b: 2, c: 3 }, + Aligned { x: 42 }, + Big { a: 4, b: 5, c: 6 }, + Aligned { x: 43 }, + ) + }; + if result != 0 { + return result; + } + + // GCC as the caller, cg_gcc as the callee. + unsafe { c_call_rust() } +} From 80b9114ed632d61f29c999a84631fcfe085ce585 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Thu, 6 Aug 2026 12:58:34 -0400 Subject: [PATCH 5/9] Make type attributes less error-prone --- clippy.toml | 3 + src/abi.rs | 6 +- src/common.rs | 3 +- src/intrinsic/llvm.rs | 15 ++--- src/type_.rs | 127 ++++++++++++++++++++++++++---------------- src/type_of.rs | 18 +++--- 6 files changed, 104 insertions(+), 68 deletions(-) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000000..cf1593c6917 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +disallowed-methods = [ + { path = "gccjit::types::Type::add_attribute", reason = "go through `type_::apply_struct_attributes` instead: an attribute set directly on a type would not be part of the `CodegenCx::struct_types` cache key, so it would silently change every other use of that type" }, +] diff --git a/src/abi.rs b/src/abi.rs index ba798bab83f..fac5dfc3226 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -73,9 +73,9 @@ impl GccType for CastTarget { args.push(cx.type_ix(rem_bytes * 8)); } - // A cast target describes registers, so its alignment is whatever GCC computes from - // them rather than the alignment of the Rust type being cast. - cx.type_struct(&args, false, None) + // A cast target describes registers, so its layout is whatever GCC computes from them + // rather than the layout of the Rust type being cast. + cx.type_struct(&args, &[]) } } diff --git a/src/common.rs b/src/common.rs index 5b89fc7aa63..8d63f5a75a3 100644 --- a/src/common.rs +++ b/src/common.rs @@ -12,6 +12,7 @@ use rustc_session::PointerAuthSchema; use crate::consts::const_alloc_to_gcc; use crate::context::{CodegenCx, new_array_type}; +use crate::type_::struct_attributes; use crate::type_of::LayoutGccExt; impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { @@ -290,7 +291,7 @@ impl<'gcc, 'tcx> ConstCodegenMethods for CodegenCx<'gcc, 'tcx> { // FIXME(antoyo): cache the type? It's anonymous, so probably not. // The alignment of a constant aggregate is the one GCC derives from its fields: the Rust // layout this comes from is not available here. - let typ = self.type_struct(&fields, packed, None); + let typ = self.type_struct(&fields, &struct_attributes(packed, None)); let struct_type = typ.is_struct().expect("struct type"); self.context.new_struct_constructor(None, struct_type.as_type(), None, values) } diff --git a/src/intrinsic/llvm.rs b/src/intrinsic/llvm.rs index 6ad19d5af09..ef381715c1e 100644 --- a/src/intrinsic/llvm.rs +++ b/src/intrinsic/llvm.rs @@ -1,12 +1,11 @@ use std::borrow::Cow; -#[cfg(feature = "master")] -use gccjit::TypeAttribute; use gccjit::{CType, Context, Field, Function, FunctionPtrType, RValue, ToRValue, Type}; use rustc_codegen_ssa::traits::BuilderMethods; use crate::builder::Builder; use crate::context::{CodegenCx, new_array_type}; +use crate::type_::{StructAttribute, apply_struct_attributes}; fn encode_key_128_type<'a, 'gcc, 'tcx>( builder: &Builder<'a, 'gcc, 'tcx>, @@ -24,8 +23,7 @@ fn encode_key_128_type<'a, 'gcc, 'tcx>( "EncodeKey128Output", &[field1, field2, field3, field4, field5, field6, field7], ); - #[cfg(feature = "master")] - encode_type.as_type().add_attribute(TypeAttribute::Packed); + apply_struct_attributes(encode_type.as_type(), &[StructAttribute::Packed]); (encode_type.as_type(), field1, field2) } @@ -46,8 +44,7 @@ fn encode_key_256_type<'a, 'gcc, 'tcx>( "EncodeKey256Output", &[field1, field2, field3, field4, field5, field6, field7, field8], ); - #[cfg(feature = "master")] - encode_type.as_type().add_attribute(TypeAttribute::Packed); + apply_struct_attributes(encode_type.as_type(), &[StructAttribute::Packed]); (encode_type.as_type(), field1, field2) } @@ -59,8 +56,7 @@ fn aes_output_type<'a, 'gcc, 'tcx>( let field2 = builder.context.new_field(None, m128i, "field2"); let aes_output_type = builder.context.new_struct_type(None, "AesOutput", &[field1, field2]); let typ = aes_output_type.as_type(); - #[cfg(feature = "master")] - typ.add_attribute(TypeAttribute::Packed); + apply_struct_attributes(typ, &[StructAttribute::Packed]); (typ, field1, field2) } @@ -82,8 +78,7 @@ fn wide_aes_output_type<'a, 'gcc, 'tcx>( "WideAesOutput", &[field1, field2, field3, field4, field5, field6, field7, field8, field9], ); - #[cfg(feature = "master")] - aes_output_type.as_type().add_attribute(TypeAttribute::Packed); + apply_struct_attributes(aes_output_type.as_type(), &[StructAttribute::Packed]); (aes_output_type.as_type(), field1, field2) } diff --git a/src/type_.rs b/src/type_.rs index e0f6d72d8fd..cd0fb5aeda4 100644 --- a/src/type_.rs +++ b/src/type_.rs @@ -1,5 +1,6 @@ #[cfg(feature = "master")] use std::convert::TryInto; +use std::mem::discriminant; #[cfg(feature = "master")] use gccjit::{CType, TypeAttribute}; @@ -101,14 +102,9 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { self.bool_type } - pub fn type_struct( - &self, - fields: &[Type<'gcc>], - packed: bool, - align: Option, - ) -> Type<'gcc> { - let align = normalize_struct_alignment(align); - let key = StructTypeKey { fields: fields.to_vec(), packed, align }; + pub fn type_struct(&self, fields: &[Type<'gcc>], attributes: &[StructAttribute]) -> Type<'gcc> { + let key = + StructTypeKey { fields: fields.to_vec(), attributes: canonical_attributes(attributes) }; if let Some(typ) = self.struct_types.borrow().get(&key) { return *typ; } @@ -120,62 +116,104 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { }) .collect(); let typ = self.context.new_struct_type(None, "struct", &fields).as_type(); - if packed { - #[cfg(feature = "master")] - typ.add_attribute(TypeAttribute::Packed); - } - set_struct_alignment(typ, align); + // The attributes that are applied are the very ones the type is keyed on, so the two + // cannot drift apart. + apply_struct_attributes(typ, &key.attributes); self.struct_types.borrow_mut().insert(key, typ); typ } } +/// An attribute that can be set on a GCC struct type. +/// +/// This mirrors the subset of `gccjit::TypeAttribute` that cg_gcc needs, rather than using it +/// directly, because it must exist without the `master` feature and because it is what +/// `StructTypeKey` is keyed on. Adding a variant here is therefore all it takes to make a new +/// attribute part of the cache key: there is no second place to remember to update. +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub enum StructAttribute { + /// Alignment, in bytes. This can only ever raise a type's alignment: GCC starts the record + /// layout from `TYPE_ALIGN` and the fields can only push it further up. + Aligned(u32), + /// Lay the fields out without inserting padding between them. + Packed, +} + /// Identifies an anonymous struct type in `CodegenCx::struct_types`. /// -/// Everything that can make two of them distinct has to be part of it. In particular the -/// alignment: two Rust types can have the same field list and still differ in alignment (for -/// instance `struct { a: u64, b: u64 }` with and without `repr(align(16))`), and they must not -/// end up sharing a GCC type. +/// Two Rust types with the same field list can still need distinct GCC types — `struct { a: u64, +/// b: u64 }` with and without `repr(align(16))` produces the same fields — so every attribute has +/// to be part of the key. Holding them as one [`StructAttribute`] list rather than as separate +/// fields is what keeps that true when a new attribute is added. #[derive(Clone, Eq, Hash, PartialEq)] pub struct StructTypeKey<'gcc> { pub fields: Vec>, - pub packed: bool, - pub align: Option, + pub attributes: Vec, } -/// Discard an alignment that GCC would give the struct anyway. +/// The attributes a GCC struct needs in order to match the Rust layout it is built from. +/// +/// GCC derives a struct's alignment from its field list, so a `repr(align(N))` larger than what +/// the fields require would otherwise be lost. That is not only a layout concern: the ABI of a +/// by-value ("byval") argument depends on it, since `ix86_function_arg_boundary` reads +/// `TYPE_ALIGN` to pick the argument's stack slot. An over-aligned aggregate whose GCC type has +/// lost its alignment is passed at an offset a C caller does not agree on. /// -/// One byte is the minimum alignment of a GCC struct, so requesting it explicitly changes -/// nothing; mapping it to `None` keeps types that do not care about their alignment sharing a +/// An alignment of one byte is dropped: it is the minimum a GCC struct gets anyway, so asking for +/// it explicitly changes nothing, and dropping it keeps every alignment-indifferent type sharing a /// single entry in `CodegenCx::struct_types`. -fn normalize_struct_alignment(align: Option) -> Option { - align.filter(|align| align.bytes() > 1) +pub fn struct_attributes(packed: bool, align: Option) -> Vec { + let mut attributes = Vec::new(); + if packed { + attributes.push(StructAttribute::Packed); + } + if let Some(align) = align + && align.bytes() > 1 + { + attributes.push(StructAttribute::Aligned(align.bytes() as u32)); + } + attributes } -/// Give a struct type the alignment that Rust computed for it. +/// Put an attribute list into a canonical form so that it can be used as a cache key. /// -/// GCC derives a struct's alignment from its field list, so a `repr(align(N))` that is larger -/// than what the fields require would otherwise be lost. That is not only a layout concern: the -/// ABI of a by-value ("byval") argument depends on it, since `ix86_function_arg_boundary` reads -/// `TYPE_ALIGN` to pick the argument's stack slot. An over-aligned aggregate whose GCC type has -/// lost its alignment is therefore passed at an offset a C caller does not agree on. +/// Without this, `[Packed, Aligned(8)]` and `[Aligned(8), Packed]` would hash differently and mint +/// two GCC types for what is one Rust type. +fn canonical_attributes(attributes: &[StructAttribute]) -> Vec { + let mut attributes = attributes.to_vec(); + attributes.sort_unstable(); + attributes.dedup(); + debug_assert!( + attributes.windows(2).all(|pair| discriminant(&pair[0]) != discriminant(&pair[1])), + "contradictory struct attributes: {attributes:?}" + ); + attributes +} + +/// Set `attributes` on the struct type `typ`. /// -/// This has to be set on the struct type itself. `Type::get_aligned` is not enough: it builds a -/// type *variant*, and the argument-passing code looks at `TYPE_MAIN_VARIANT` first, which -/// discards it. +/// This is the only place allowed to call `Type::add_attribute`; `clippy.toml` forbids it +/// everywhere else. An attribute set on a type that `CodegenCx::struct_types` handed out would +/// change every other use of that type, so attributes have to be decided when the type is created +/// and be part of its cache key. Going through [`StructAttribute`] is what enforces that. /// -/// This never under-aligns a struct whose fields need more: GCC starts the record layout from -/// `TYPE_ALIGN` and the fields can only raise it. +/// Note that the alignment has to be set on the struct type itself: `Type::get_aligned` is not +/// enough, since it builds a type *variant* and the argument-passing code looks at +/// `TYPE_MAIN_VARIANT` first, which discards it. #[cfg(feature = "master")] -fn set_struct_alignment(typ: Type<'_>, align: Option) { - if let Some(align) = normalize_struct_alignment(align) { - typ.add_attribute(TypeAttribute::Aligned(align.bytes() as u32)); +#[allow(clippy::disallowed_methods)] +pub fn apply_struct_attributes(typ: Type<'_>, attributes: &[StructAttribute]) { + for attribute in attributes { + typ.add_attribute(match *attribute { + StructAttribute::Aligned(align) => TypeAttribute::Aligned(align), + StructAttribute::Packed => TypeAttribute::Packed, + }); } } -/// Without the `master` feature, libgccjit has no way to set a type's alignment. +/// Without the `master` feature, libgccjit cannot set attributes on a type. #[cfg(not(feature = "master"))] -fn set_struct_alignment(_typ: Type<'_>, _align: Option) {} +pub fn apply_struct_attributes(_typ: Type<'_>, _attributes: &[StructAttribute]) {} impl<'gcc, 'tcx> BaseTypeCodegenMethods for CodegenCx<'gcc, 'tcx> { fn type_i8(&self) -> Type<'gcc> { @@ -382,8 +420,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { &self, typ: Struct<'gcc>, fields: &[Type<'gcc>], - packed: bool, - align: Option, + attributes: &[StructAttribute], ) { let fields: Vec<_> = fields .iter() @@ -391,11 +428,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { .map(|(index, field)| self.context.new_field(None, *field, format!("field_{}", index))) .collect(); typ.set_fields(None, &fields); - if packed { - #[cfg(feature = "master")] - typ.as_type().add_attribute(TypeAttribute::Packed); - } - set_struct_alignment(typ.as_type(), align); + apply_struct_attributes(typ.as_type(), &canonical_attributes(attributes)); } pub fn type_named_struct(&self, name: &str) -> Struct<'gcc> { diff --git a/src/type_of.rs b/src/type_of.rs index da9a660e73d..409343c6351 100644 --- a/src/type_of.rs +++ b/src/type_of.rs @@ -17,7 +17,7 @@ use rustc_target::callconv::{CastTarget, FnAbi}; use crate::abi::{FnAbiGcc, FnAbiGccExt, GccType}; use crate::context::CodegenCx; -use crate::type_::struct_fields; +use crate::type_::{struct_attributes, struct_fields}; impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { fn type_from_unsigned_integer(&self, i: Integer) -> Type<'gcc> { @@ -81,8 +81,7 @@ fn uncached_gcc_type<'gcc, 'tcx>( layout.scalar_pair_element_gcc_type(cx, 0), layout.scalar_pair_element_gcc_type(cx, 1), ], - false, - Some(layout.align.abi), + &struct_attributes(false, Some(layout.align.abi)), ); } BackendRepr::Memory { .. } => {} @@ -130,11 +129,12 @@ fn uncached_gcc_type<'gcc, 'tcx>( FieldsShape::Primitive | FieldsShape::Union(_) => { let fill = cx.type_padding_filler(layout.size, layout.align.abi); let packed = false; + let attributes = struct_attributes(packed, Some(layout.align.abi)); match name { - None => cx.type_struct(&[fill], packed, Some(layout.align.abi)), + None => cx.type_struct(&[fill], &attributes), Some(ref name) => { let gcc_type = cx.type_named_struct(name); - cx.set_struct_body(gcc_type, &[fill], packed, Some(layout.align.abi)); + cx.set_struct_body(gcc_type, &[fill], &attributes); gcc_type.as_type() } } @@ -143,7 +143,7 @@ fn uncached_gcc_type<'gcc, 'tcx>( FieldsShape::Arbitrary { .. } => match name { None => { let (gcc_fields, packed) = struct_fields(cx, layout); - cx.type_struct(&gcc_fields, packed, Some(layout.align.abi)) + cx.type_struct(&gcc_fields, &struct_attributes(packed, Some(layout.align.abi))) } Some(ref name) => { let gcc_type = cx.type_named_struct(name); @@ -241,7 +241,11 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> { if let Some((deferred_ty, layout)) = defer { let (fields, packed) = struct_fields(cx, layout); - cx.set_struct_body(deferred_ty, &fields, packed, Some(layout.align.abi)); + cx.set_struct_body( + deferred_ty, + &fields, + &struct_attributes(packed, Some(layout.align.abi)), + ); } ty From d1525e2f206c2e91b6f271a46da6dfccf1b5aaf1 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Thu, 6 Aug 2026 21:44:53 -0400 Subject: [PATCH 6/9] Guard on max GCC alignment --- src/type_.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/type_.rs b/src/type_.rs index cd0fb5aeda4..c3f6304ddae 100644 --- a/src/type_.rs +++ b/src/type_.rs @@ -169,12 +169,23 @@ pub fn struct_attributes(packed: bool, align: Option) -> Vec 1 + && align.bytes() <= MAX_STRUCT_ALIGNMENT { attributes.push(StructAttribute::Aligned(align.bytes() as u32)); } attributes } +/// The largest alignment GCC accepts on a type, in bytes. +/// +/// This is `MAX_OFILE_ALIGNMENT / BITS_PER_UNIT` for ELF targets. Rust allows alignments up to +/// `1 << 29`, so a `repr(align(N))` beyond this simply cannot be expressed: asking for it makes +/// libgccjit fail the whole compilation with "requested alignment `N` exceeds maximum". Such a +/// type keeps whatever alignment GCC derives from its fields instead, which is what every type +/// got before alignments were set at all. See `tests/ui/abi/large-byval-align.rs`, which upstream +/// marks `ignore-backends: gcc` for this reason. +const MAX_STRUCT_ALIGNMENT: u64 = 1 << 28; + /// Put an attribute list into a canonical form so that it can be used as a cache key. /// /// Without this, `[Packed, Aligned(8)]` and `[Aligned(8), Packed]` would hash differently and mint From f525d92d0dec5f4a7fb4379743a6ca8fdc774eae Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Fri, 7 Aug 2026 10:18:17 -0400 Subject: [PATCH 7/9] Cleanup --- src/abi.rs | 9 --------- src/common.rs | 2 -- src/type_.rs | 27 +-------------------------- tests/lang_tests.rs | 11 +++-------- 4 files changed, 4 insertions(+), 45 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index fac5dfc3226..240bba0e752 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -73,8 +73,6 @@ impl GccType for CastTarget { args.push(cx.type_ix(rem_bytes * 8)); } - // A cast target describes registers, so its layout is whatever GCC computes from them - // rather than the layout of the Rust type being cast. cx.type_struct(&args, &[]) } } @@ -212,13 +210,6 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { cx.type_ptr_to(arg.layout.gcc_type(cx)) } else { // This is a "byval" argument, so we don't apply the `restrict` attribute on it. - // - // GCC picks the argument's stack slot from the alignment of this type, - // which `LayoutGccExt::gcc_type` sets from `layout.align.abi`. We - // deliberately do not use `attrs.pointee_align` here: when it differs - // from the type's alignment it describes the *slot*, not the type, and - // rustc already copies the argument to a sufficiently aligned alloca on - // whichever side needs it. on_stack_param_indices.insert(argument_tys.len()); arg.layout.gcc_type(cx) } diff --git a/src/common.rs b/src/common.rs index 8d63f5a75a3..a503c1b3451 100644 --- a/src/common.rs +++ b/src/common.rs @@ -289,8 +289,6 @@ impl<'gcc, 'tcx> ConstCodegenMethods for CodegenCx<'gcc, 'tcx> { fn const_struct(&self, values: &[RValue<'gcc>], packed: bool) -> RValue<'gcc> { let fields: Vec<_> = values.iter().map(|value| value.get_type()).collect(); // FIXME(antoyo): cache the type? It's anonymous, so probably not. - // The alignment of a constant aggregate is the one GCC derives from its fields: the Rust - // layout this comes from is not available here. let typ = self.type_struct(&fields, &struct_attributes(packed, None)); let struct_type = typ.is_struct().expect("struct type"); self.context.new_struct_constructor(None, struct_type.as_type(), None, values) diff --git a/src/type_.rs b/src/type_.rs index c3f6304ddae..9b2896838e7 100644 --- a/src/type_.rs +++ b/src/type_.rs @@ -116,8 +116,6 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { }) .collect(); let typ = self.context.new_struct_type(None, "struct", &fields).as_type(); - // The attributes that are applied are the very ones the type is keyed on, so the two - // cannot drift apart. apply_struct_attributes(typ, &key.attributes); self.struct_types.borrow_mut().insert(key, typ); typ @@ -132,8 +130,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { /// attribute part of the cache key: there is no second place to remember to update. #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub enum StructAttribute { - /// Alignment, in bytes. This can only ever raise a type's alignment: GCC starts the record - /// layout from `TYPE_ALIGN` and the fields can only push it further up. + /// Alignment, in bytes. Aligned(u32), /// Lay the fields out without inserting padding between them. Packed, @@ -152,16 +149,6 @@ pub struct StructTypeKey<'gcc> { } /// The attributes a GCC struct needs in order to match the Rust layout it is built from. -/// -/// GCC derives a struct's alignment from its field list, so a `repr(align(N))` larger than what -/// the fields require would otherwise be lost. That is not only a layout concern: the ABI of a -/// by-value ("byval") argument depends on it, since `ix86_function_arg_boundary` reads -/// `TYPE_ALIGN` to pick the argument's stack slot. An over-aligned aggregate whose GCC type has -/// lost its alignment is passed at an offset a C caller does not agree on. -/// -/// An alignment of one byte is dropped: it is the minimum a GCC struct gets anyway, so asking for -/// it explicitly changes nothing, and dropping it keeps every alignment-indifferent type sharing a -/// single entry in `CodegenCx::struct_types`. pub fn struct_attributes(packed: bool, align: Option) -> Vec { let mut attributes = Vec::new(); if packed { @@ -177,13 +164,6 @@ pub fn struct_attributes(packed: bool, align: Option) -> Vec Vec /// everywhere else. An attribute set on a type that `CodegenCx::struct_types` handed out would /// change every other use of that type, so attributes have to be decided when the type is created /// and be part of its cache key. Going through [`StructAttribute`] is what enforces that. -/// -/// Note that the alignment has to be set on the struct type itself: `Type::get_aligned` is not -/// enough, since it builds a type *variant* and the argument-passing code looks at -/// `TYPE_MAIN_VARIANT` first, which discards it. #[cfg(feature = "master")] #[allow(clippy::disallowed_methods)] pub fn apply_struct_attributes(typ: Type<'_>, attributes: &[StructAttribute]) { @@ -222,7 +198,6 @@ pub fn apply_struct_attributes(typ: Type<'_>, attributes: &[StructAttribute]) { } } -/// Without the `master` feature, libgccjit cannot set attributes on a type. #[cfg(not(feature = "master"))] pub fn apply_struct_attributes(_typ: Type<'_>, _attributes: &[StructAttribute]) {} diff --git a/tests/lang_tests.rs b/tests/lang_tests.rs index 5426d777bb4..ebce05a0597 100644 --- a/tests/lang_tests.rs +++ b/tests/lang_tests.rs @@ -10,9 +10,6 @@ use tempfile::TempDir; /// Directory holding the C files that the `tests/run` tests can link against. /// /// A `tests/c/.c` is compiled by the real GCC and linked into `tests/run/.rs`. -/// This is what makes it possible to test the ABI: with cg_gcc on both sides of a call, caller -/// and callee agree even when they are both wrong, so a pure-Rust test cannot notice. A C -/// caller or callee built by GCC is an independent reference. const C_TESTS_DIR: &str = "tests/c"; /// The m68k cross toolchain is not on the default `PATH` in CI. @@ -26,10 +23,7 @@ fn target_path(test_target: &Option) -> Option { }) } -/// Compile every C file in `tests/c` to an object file in `objects_dir`, named after the C file. -/// -/// The C files are compiled by the real GCC (the cross one when testing another target), not by -/// cg_gcc: they are the reference the Rust side is checked against. +/// Compile every C file in `tests/c` to an object file in `objects_dir`. fn compile_c_files(objects_dir: &Path, test_target: &Option) { let c_tests_dir = Path::new(C_TESTS_DIR); if !c_tests_dir.is_dir() { @@ -54,7 +48,7 @@ fn compile_c_files(objects_dir: &Path, test_target: &Option) { // Optimize: an unoptimized C caller can happen to agree with a wrong callee. command.arg("-O1"); // GCC notes that the ABI of over-aligned arguments changed in GCC 4.6. That is the ABI - // being tested here, so the note is expected rather than a problem. + // being tested in overaligned_byval_abi, so the note is expected rather than a problem. command.arg("-Wno-psabi"); command.arg("-o"); command.arg(&object); @@ -147,6 +141,7 @@ impl TestMode { } } +#[allow(clippy::too_many_arguments)] fn build_test_runner( tempdir: PathBuf, c_objects_dir: PathBuf, From 8ef525032677c4594b8959899613075a04b7a71a Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Fri, 7 Aug 2026 10:20:49 -0400 Subject: [PATCH 8/9] Remove spell checks in CI --- .cspell.json | 28 -------- .github/workflows/ci.yml | 7 -- tools/cspell_dicts/rust.txt | 3 - tools/cspell_dicts/rustc_codegen_gcc.txt | 83 ------------------------ 4 files changed, 121 deletions(-) delete mode 100644 .cspell.json delete mode 100644 tools/cspell_dicts/rust.txt delete mode 100644 tools/cspell_dicts/rustc_codegen_gcc.txt diff --git a/.cspell.json b/.cspell.json deleted file mode 100644 index a2856029c2c..00000000000 --- a/.cspell.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "allowCompoundWords": true, - "dictionaries": ["cpp", "rust-extra", "rustc_codegen_gcc"], - "dictionaryDefinitions": [ - { - "name": "rust-extra", - "path": "tools/cspell_dicts/rust.txt", - "addWords": true - }, - { - "name": "rustc_codegen_gcc", - "path": "tools/cspell_dicts/rustc_codegen_gcc.txt", - "addWords": true - } - ], - "files": [ - "src/**/*.rs" - ], - "ignorePaths": [ - "src/intrinsic/archs.rs", - "src/intrinsic/old_archs.rs", - "src/intrinsic/llvm.rs" - ], - "ignoreRegExpList": [ - "/(FIXME|NOTE)\\([^)]+\\)/", - "__builtin_\\w*" - ] -} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b76c79fd108..e58a1596bff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -126,13 +126,6 @@ jobs: - uses: actions/checkout@v4 - run: python tools/check_intrinsics_duplicates.py - spell_check: - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@v4 - - uses: crate-ci/typos@v1.32.0 - - uses: streetsidesoftware/cspell-action@v7 - build_system: runs-on: ubuntu-24.04 steps: diff --git a/tools/cspell_dicts/rust.txt b/tools/cspell_dicts/rust.txt deleted file mode 100644 index 15faacd53d5..00000000000 --- a/tools/cspell_dicts/rust.txt +++ /dev/null @@ -1,3 +0,0 @@ -lateout -repr -rmeta diff --git a/tools/cspell_dicts/rustc_codegen_gcc.txt b/tools/cspell_dicts/rustc_codegen_gcc.txt deleted file mode 100644 index bae8edc9ffd..00000000000 --- a/tools/cspell_dicts/rustc_codegen_gcc.txt +++ /dev/null @@ -1,83 +0,0 @@ -aapcs -addo -archs -ashl -ashr -cgcx -clzll -cmse -codegened -csky -ctfe -ctlz -ctpop -cttz -ctzll -flto -fmaximumf -fmuladd -fmuladdf -fminimumf -fmul -fptosi -fptosui -fptoui -fwrapv -gimple -hrtb -immediates -interner -liblto -llbb -llcx -llextra -llfn -lgcc -llmod -llresult -llret -ltrans -llty -llval -llvals -loong -lshr -masm -maximumf -maxnumf -mavx -mcmodel -minimumf -minnumf -miri -monomorphization -monomorphizations -monomorphized -monomorphizing -movnt -mulo -nvptx -pointee -powitf -reassoc -retag -riscv -rlib -roundevenf -rustc -sgpr -sitofp -sizet -spir -subo -sysv -tbaa -uitofp -unord -uninlined -utrunc -vgpr -xabort -xreg -xtensa -zext From 51584dda48b84c287003094dd3323754bf06d7d0 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Fri, 7 Aug 2026 10:38:46 -0400 Subject: [PATCH 9/9] Fix test on m68k --- tests/c/overaligned_byval_abi.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/c/overaligned_byval_abi.c b/tests/c/overaligned_byval_abi.c index e7575871e06..826a104f185 100644 --- a/tests/c/overaligned_byval_abi.c +++ b/tests/c/overaligned_byval_abi.c @@ -12,21 +12,23 @@ * alignment the ABI gives a stack slot is target-specific, but caller and callee agreeing on it * is not. A disagreement makes the arguments arrive as garbage. */ +#include + struct Big { - long a, b, c; + int64_t a, b, c; }; struct __attribute__((aligned(64))) Aligned { - int x; + int32_t x; }; /* Defined on the Rust side. */ -extern int rust_take_both(struct Big first, struct Aligned second, struct Big third, - struct Aligned fourth); +extern int32_t rust_take_both(struct Big first, struct Aligned second, struct Big third, + struct Aligned fourth); /* Called from Rust: checks what a cg_gcc caller passed. */ -int c_take_both(struct Big first, struct Aligned second, struct Big third, - struct Aligned fourth) +int32_t c_take_both(struct Big first, struct Aligned second, struct Big third, + struct Aligned fourth) { if (first.a != 1 || first.b != 2 || first.c != 3) return 1; @@ -40,7 +42,7 @@ int c_take_both(struct Big first, struct Aligned second, struct Big third, } /* Called from Rust: passes the arguments the way the ABI says, for a cg_gcc callee to read. */ -int c_call_rust(void) +int32_t c_call_rust(void) { struct Big first = {1, 2, 3}; struct Big third = {4, 5, 6};