From 3f73f85c1fa3a7d60871b211a16393df04a0fd61 Mon Sep 17 00:00:00 2001 From: Amy Kwan Date: Wed, 15 Jan 2025 14:49:16 -0500 Subject: [PATCH] [AIX] Lint on structs that have a different alignment in AIX's C ABI --- compiler/rustc_lint/messages.ftl | 2 + compiler/rustc_lint/src/lints.rs | 4 + compiler/rustc_lint/src/types.rs | 134 +++++++++++++++- tests/ui/layout/reprc-power-alignment.rs | 152 +++++++++++++++++++ tests/ui/layout/reprc-power-alignment.stderr | 112 ++++++++++++++ tests/ui/lint/clashing-extern-fn.rs | 1 + tests/ui/lint/clashing-extern-fn.stderr | 22 +-- 7 files changed, 411 insertions(+), 16 deletions(-) create mode 100644 tests/ui/layout/reprc-power-alignment.rs create mode 100644 tests/ui/layout/reprc-power-alignment.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 7522e21d0ef09..868ffdc14238d 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -970,6 +970,8 @@ lint_unused_result = unused result of type `{$ty}` lint_use_let_underscore_ignore_suggestion = use `let _ = ...` to ignore the expression or result +lint_uses_power_alignment = repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + lint_variant_size_differences = enum variant is more than three times larger ({$largest} bytes) than the next largest diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index ac995b59caff1..32af126f364f9 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1692,6 +1692,10 @@ pub(crate) struct OverflowingLiteral<'a> { pub lit: String, } +#[derive(LintDiagnostic)] +#[diag(lint_uses_power_alignment)] +pub(crate) struct UsesPowerAlignment; + #[derive(LintDiagnostic)] #[diag(lint_unused_comparisons)] pub(crate) struct UnusedComparisons; diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 3bd27a224e77c..1aa75814fe666 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -1,14 +1,15 @@ use std::iter; use std::ops::ControlFlow; -use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, Variants, WrappingRange}; +use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, VariantIdx, Variants, WrappingRange}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_middle::bug; use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton}; use rustc_middle::ty::{ - self, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, + self, Adt, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, + TypeVisitableExt, }; use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass}; use rustc_span::def_id::LocalDefId; @@ -23,7 +24,7 @@ use crate::lints::{ AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons, - UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, + UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, UsesPowerAlignment, VariantSizeDifferencesDiag, }; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; @@ -727,7 +728,60 @@ declare_lint! { "proper use of libc types in foreign item definitions" } -declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS]); +declare_lint! { + /// The `uses_power_alignment` lint detects specific `repr(C)` + /// aggregates on AIX. + /// In its platform C ABI, AIX uses the "power" (as in PowerPC) alignment + /// rule (detailed in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment), + /// which can also be set for XLC by `#pragma align(power)` or + /// `-qalign=power`. Aggregates with a floating-point type as the + /// recursively first field (as in "at offset 0") modify the layout of + /// *subsequent* fields of the associated structs to use an alignment value + /// where the floating-point type is aligned on a 4-byte boundary. + /// + /// The power alignment rule for structs needed for C compatibility is + /// unimplementable within `repr(C)` in the compiler without building in + /// handling of references to packed fields and infectious nested layouts, + /// so a warning is produced in these situations. + /// + /// ### Example + /// + /// ```rust,ignore + /// #[repr(C)] + /// pub struct Floats { + /// a: f64, + /// b: u8, + /// c: f64, + /// } + /// ``` + /// + /// This will produce: + /// + /// ```text + /// warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + /// --> :5:3 + /// | + /// 5 | c: f64, + /// | ^^^^^^ + /// | + /// = note: `#[warn(uses_power_alignment)]` on by default + /// ``` + /// + /// ### Explanation + /// + /// The power alignment rule specifies that the above struct has the + /// following alignment: + /// - offset_of!(Floats, a) == 0 + /// - offset_of!(Floats, b) == 8 + /// - offset_of!(Floats, c) == 12 + /// However, rust currently aligns `c` at offset_of!(Floats, c) == 16. + /// Thus, a warning should be produced for the above struct in this case. + USES_POWER_ALIGNMENT, + Warn, + "Structs do not follow the power alignment rule under repr(C)" +} + +declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS, USES_POWER_ALIGNMENT]); #[derive(Clone, Copy)] pub(crate) enum CItemKind { @@ -1539,6 +1593,71 @@ impl ImproperCTypesDefinitions { vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false); } } + + fn check_arg_for_power_alignment<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + ) -> bool { + // Structs (under repr(C)) follow the power alignment rule if: + // - the first field of the struct is a floating-point type that + // is greater than 4-bytes, or + // - the first field of the struct is an aggregate whose + // recursively first field is a floating-point type greater than + // 4 bytes. + if cx.tcx.sess.target.os != "aix" { + return false; + } + if ty.is_floating_point() && ty.primitive_size(cx.tcx).bytes() > 4 { + return true; + } else if let Adt(adt_def, _) = ty.kind() + && adt_def.is_struct() + { + let struct_variant = adt_def.variant(VariantIdx::ZERO); + // Within a nested struct, all fields are examined to correctly + // report if any fields after the nested struct within the + // original struct are misaligned. + for struct_field in &struct_variant.fields { + let field_ty = cx.tcx.type_of(struct_field.did).instantiate_identity(); + if self.check_arg_for_power_alignment(cx, field_ty) { + return true; + } + } + } + return false; + } + + fn check_struct_for_power_alignment<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + item: &'tcx hir::Item<'tcx>, + ) { + let adt_def = cx.tcx.adt_def(item.owner_id.to_def_id()); + if adt_def.repr().c() + && !adt_def.repr().packed() + && cx.tcx.sess.target.os == "aix" + && !adt_def.all_fields().next().is_none() + { + let struct_variant_data = item.expect_struct().0; + for (index, ..) in struct_variant_data.fields().iter().enumerate() { + // Struct fields (after the first field) are checked for the + // power alignment rule, as fields after the first are likely + // to be the fields that are misaligned. + if index != 0 { + let first_field_def = struct_variant_data.fields()[index]; + let def_id = first_field_def.def_id; + let ty = cx.tcx.type_of(def_id).instantiate_identity(); + if self.check_arg_for_power_alignment(cx, ty) { + cx.emit_span_lint( + USES_POWER_ALIGNMENT, + first_field_def.span, + UsesPowerAlignment, + ); + } + } + } + } + } } /// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in @@ -1562,8 +1681,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { } // See `check_fn`.. hir::ItemKind::Fn { .. } => {} + // Structs are checked based on if they follow the power alignment + // rule (under repr(C)). + hir::ItemKind::Struct(..) => { + self.check_struct_for_power_alignment(cx, item); + } // See `check_field_def`.. - hir::ItemKind::Union(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Enum(..) => {} + hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {} // Doesn't define something that can contain a external type to be checked. hir::ItemKind::Impl(..) | hir::ItemKind::TraitAlias(..) diff --git a/tests/ui/layout/reprc-power-alignment.rs b/tests/ui/layout/reprc-power-alignment.rs new file mode 100644 index 0000000000000..f6c1df55988ba --- /dev/null +++ b/tests/ui/layout/reprc-power-alignment.rs @@ -0,0 +1,152 @@ +//@ check-pass +//@ compile-flags: --target powerpc64-ibm-aix +//@ needs-llvm-components: powerpc +//@ add-core-stubs +#![feature(no_core)] +#![no_core] +#![no_std] + +extern crate minicore; +use minicore::*; + +#[warn(uses_power_alignment)] + +#[repr(C)] +pub struct Floats { + a: f64, + b: u8, + c: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + d: f32, +} + +pub struct Floats2 { + a: f64, + b: u32, + c: f64, +} + +#[repr(C)] +pub struct Floats3 { + a: f32, + b: f32, + c: i64, +} + +#[repr(C)] +pub struct Floats4 { + a: u64, + b: u32, + c: f32, +} + +#[repr(C)] +pub struct Floats5 { + a: f32, + b: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + c: f32, +} + +#[repr(C)] +pub struct FloatAgg1 { + x: Floats, + y: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} + +#[repr(C)] +pub struct FloatAgg2 { + x: i64, + y: Floats, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} + +#[repr(C)] +pub struct FloatAgg3 { + x: FloatAgg1, + // NOTE: the "power" alignment rule is infectious to nested struct fields. + y: FloatAgg2, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + z: FloatAgg2, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} + +#[repr(C)] +pub struct FloatAgg4 { + x: FloatAgg1, + y: FloatAgg2, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} + +#[repr(C)] +pub struct FloatAgg5 { + x: FloatAgg1, + y: FloatAgg2, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + z: FloatAgg3, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} + +#[repr(C)] +pub struct FloatAgg6 { + x: i64, + y: Floats, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + z: u8, +} + +#[repr(C)] +pub struct FloatAgg7 { + x: i64, + y: Floats, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + z: u8, + zz: f32, +} + +#[repr(C)] +pub struct A { + d: f64, +} +#[repr(C)] +pub struct B { + a: A, + f: f32, + d: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} +#[repr(C)] +pub struct C { + c: u8, + b: B, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} +#[repr(C)] +pub struct D { + x: f64, +} +#[repr(C)] +pub struct E { + x: i32, + d: D, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} +#[repr(C)] +pub struct F { + a: u8, + b: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} +#[repr(C)] +pub struct G { + a: u8, + b: u8, + c: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + d: f32, + e: f64, //~ WARNING repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type +} +// Should not warn on #[repr(packed)]. +#[repr(packed)] +pub struct H { + a: u8, + b: u8, + c: f64, + d: f32, + e: f64, +} +#[repr(C, packed)] +pub struct I { + a: u8, + b: u8, + c: f64, + d: f32, + e: f64, +} + +fn main() { } diff --git a/tests/ui/layout/reprc-power-alignment.stderr b/tests/ui/layout/reprc-power-alignment.stderr new file mode 100644 index 0000000000000..18664e4d655d3 --- /dev/null +++ b/tests/ui/layout/reprc-power-alignment.stderr @@ -0,0 +1,112 @@ +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:18:5 + | +LL | c: f64, + | ^^^^^^ + | +note: the lint level is defined here + --> $DIR/reprc-power-alignment.rs:12:8 + | +LL | #[warn(uses_power_alignment)] + | ^^^^^^^^^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:45:5 + | +LL | b: f64, + | ^^^^^^ + | + = note: `#[warn(uses_power_alignment)]` on by default + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:52:5 + | +LL | y: f64, + | ^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:58:5 + | +LL | y: Floats, + | ^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:65:5 + | +LL | y: FloatAgg2, + | ^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:66:5 + | +LL | z: FloatAgg2, + | ^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:72:5 + | +LL | y: FloatAgg2, + | ^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:78:5 + | +LL | y: FloatAgg2, + | ^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:79:5 + | +LL | z: FloatAgg3, + | ^^^^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:85:5 + | +LL | y: Floats, + | ^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:92:5 + | +LL | y: Floats, + | ^^^^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:105:3 + | +LL | d: f64, + | ^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:110:3 + | +LL | b: B, + | ^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:119:3 + | +LL | d: D, + | ^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:124:3 + | +LL | b: f64, + | ^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:130:5 + | +LL | c: f64, + | ^^^^^^ + +warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + --> $DIR/reprc-power-alignment.rs:132:5 + | +LL | e: f64, + | ^^^^^^ + +warning: 17 warnings emitted + diff --git a/tests/ui/lint/clashing-extern-fn.rs b/tests/ui/lint/clashing-extern-fn.rs index a12fe81eecdaf..9bbb20246df9d 100644 --- a/tests/ui/lint/clashing-extern-fn.rs +++ b/tests/ui/lint/clashing-extern-fn.rs @@ -248,6 +248,7 @@ mod sameish_members { mod same_sized_members_clash { mod a { + #[allow(uses_power_alignment)] #[repr(C)] struct Point3 { x: f32, diff --git a/tests/ui/lint/clashing-extern-fn.stderr b/tests/ui/lint/clashing-extern-fn.stderr index b30dd476a1d74..48dd1adbc1fa2 100644 --- a/tests/ui/lint/clashing-extern-fn.stderr +++ b/tests/ui/lint/clashing-extern-fn.stderr @@ -1,5 +1,5 @@ warning: `extern` block uses type `Option`, which is not FFI-safe - --> $DIR/clashing-extern-fn.rs:482:55 + --> $DIR/clashing-extern-fn.rs:483:55 | LL | fn hidden_niche_transparent_no_niche() -> Option; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -9,7 +9,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option>>`, which is not FFI-safe - --> $DIR/clashing-extern-fn.rs:486:46 + --> $DIR/clashing-extern-fn.rs:487:46 | LL | fn hidden_niche_unsafe_cell() -> Option>>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -151,7 +151,7 @@ LL | fn draw_point(p: Point); found `unsafe extern "C" fn(sameish_members::b::Point)` warning: `origin` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:269:13 + --> $DIR/clashing-extern-fn.rs:270:13 | LL | fn origin() -> Point3; | ---------------------- `origin` previously declared here @@ -163,7 +163,7 @@ LL | fn origin() -> Point3; found `unsafe extern "C" fn() -> same_sized_members_clash::b::Point3` warning: `transparent_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:292:13 + --> $DIR/clashing-extern-fn.rs:293:13 | LL | fn transparent_incorrect() -> T; | -------------------------------- `transparent_incorrect` previously declared here @@ -175,7 +175,7 @@ LL | fn transparent_incorrect() -> isize; found `unsafe extern "C" fn() -> isize` warning: `missing_return_type` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:310:13 + --> $DIR/clashing-extern-fn.rs:311:13 | LL | fn missing_return_type() -> usize; | ---------------------------------- `missing_return_type` previously declared here @@ -187,7 +187,7 @@ LL | fn missing_return_type(); found `unsafe extern "C" fn()` warning: `non_zero_usize` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:328:13 + --> $DIR/clashing-extern-fn.rs:329:13 | LL | fn non_zero_usize() -> core::num::NonZero; | ------------------------------------------------- `non_zero_usize` previously declared here @@ -199,7 +199,7 @@ LL | fn non_zero_usize() -> usize; found `unsafe extern "C" fn() -> usize` warning: `non_null_ptr` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:330:13 + --> $DIR/clashing-extern-fn.rs:331:13 | LL | fn non_null_ptr() -> core::ptr::NonNull; | ----------------------------------------------- `non_null_ptr` previously declared here @@ -211,7 +211,7 @@ LL | fn non_null_ptr() -> *const usize; found `unsafe extern "C" fn() -> *const usize` warning: `option_non_zero_usize_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:424:13 + --> $DIR/clashing-extern-fn.rs:425:13 | LL | fn option_non_zero_usize_incorrect() -> usize; | ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here @@ -223,7 +223,7 @@ LL | fn option_non_zero_usize_incorrect() -> isize; found `unsafe extern "C" fn() -> isize` warning: `option_non_null_ptr_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:426:13 + --> $DIR/clashing-extern-fn.rs:427:13 | LL | fn option_non_null_ptr_incorrect() -> *const usize; | --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here @@ -235,7 +235,7 @@ LL | fn option_non_null_ptr_incorrect() -> *const isize; found `unsafe extern "C" fn() -> *const isize` warning: `hidden_niche_transparent_no_niche` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:482:13 + --> $DIR/clashing-extern-fn.rs:483:13 | LL | fn hidden_niche_transparent_no_niche() -> usize; | ------------------------------------------------ `hidden_niche_transparent_no_niche` previously declared here @@ -247,7 +247,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option Option` warning: `hidden_niche_unsafe_cell` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:486:13 + --> $DIR/clashing-extern-fn.rs:487:13 | LL | fn hidden_niche_unsafe_cell() -> usize; | --------------------------------------- `hidden_niche_unsafe_cell` previously declared here