Skip to content

Commit 42a6dd2

Browse files
compiler: Remove power alignment lint
The "power alignment" lint was based on a false premise: rustc assumed LLVM's built-in AIX datalayout was correct for given types. Unfortunately, C compilers, like clang, implemented a different layout. As LLVM's wrong layout was taken as gospel, a fix seemed impossible. This resulted in the creation of the "power alignment" lint as a compromise, as it did not require invasively changing layout algorithms. Eventually in llvm-project's @b7c0452a9a3d895b14ec7c5735e4e4ddc311edb3 LLVM's AIX layout was fixed. Now rustc will have mostly-correct layouts on AIX since rust's dea8b8b We may need a new lint that captures the remaining C layout problems for AIX and other targets as well, but that is a larger conversation. For now, remove the incorrect lint.
1 parent 021fc25 commit 42a6dd2

3 files changed

Lines changed: 3 additions & 162 deletions

File tree

compiler/rustc_lint/messages.ftl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,5 @@ lint_useless_ptr_null_checks_fn_ret = returned pointer of `{$fn_name}` call is n
10631063
lint_useless_ptr_null_checks_ref = references are not nullable, so checking them for null will always return false
10641064
.label = expression has type `{$orig_ty}`
10651065
1066-
lint_uses_power_alignment = repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type
1067-
10681066
lint_variant_size_differences =
10691067
enum variant is more than three times larger ({$largest} bytes) than the next largest

compiler/rustc_lint/src/lints.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,10 +1775,6 @@ pub(crate) struct TooLargeCharCast {
17751775
pub literal: u128,
17761776
}
17771777

1778-
#[derive(LintDiagnostic)]
1779-
#[diag(lint_uses_power_alignment)]
1780-
pub(crate) struct UsesPowerAlignment;
1781-
17821778
#[derive(LintDiagnostic)]
17831779
#[diag(lint_unused_comparisons)]
17841780
pub(crate) struct UnusedComparisons;

compiler/rustc_lint/src/types/improper_ctypes.rs

Lines changed: 3 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,22 @@ use std::iter;
22
use std::ops::ControlFlow;
33

44
use bitflags::bitflags;
5-
use rustc_abi::VariantIdx;
65
use rustc_data_structures::fx::FxHashSet;
76
use rustc_errors::DiagMessage;
87
use rustc_hir::def::CtorKind;
98
use rustc_hir::intravisit::VisitorExt;
109
use rustc_hir::{self as hir, AmbigArg};
1110
use rustc_middle::bug;
1211
use rustc_middle::ty::{
13-
self, Adt, AdtDef, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
14-
TypeVisitableExt,
12+
self, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
1513
};
1614
use rustc_session::{declare_lint, declare_lint_pass};
1715
use rustc_span::def_id::LocalDefId;
1816
use rustc_span::{Span, sym};
19-
use rustc_target::spec::Os;
2017
use tracing::debug;
2118

2219
use super::repr_nullable_ptr;
23-
use crate::lints::{ImproperCTypes, UsesPowerAlignment};
20+
use crate::lints::ImproperCTypes;
2421
use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
2522

2623
declare_lint! {
@@ -77,65 +74,9 @@ declare_lint! {
7774
"proper use of libc types in foreign item definitions"
7875
}
7976

80-
declare_lint! {
81-
/// The `uses_power_alignment` lint detects specific `repr(C)`
82-
/// aggregates on AIX.
83-
/// In its platform C ABI, AIX uses the "power" (as in PowerPC) alignment
84-
/// rule (detailed in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment),
85-
/// which can also be set for XLC by `#pragma align(power)` or
86-
/// `-qalign=power`. Aggregates with a floating-point type as the
87-
/// recursively first field (as in "at offset 0") modify the layout of
88-
/// *subsequent* fields of the associated structs to use an alignment value
89-
/// where the floating-point type is aligned on a 4-byte boundary.
90-
///
91-
/// Effectively, subsequent floating-point fields act as-if they are `repr(packed(4))`. This
92-
/// would be unsound to do in a `repr(C)` type without all the restrictions that come with
93-
/// `repr(packed)`. Rust instead chooses a layout that maintains soundness of Rust code, at the
94-
/// expense of incompatibility with C code.
95-
///
96-
/// ### Example
97-
///
98-
/// ```rust,ignore (fails on non-powerpc64-ibm-aix)
99-
/// #[repr(C)]
100-
/// pub struct Floats {
101-
/// a: f64,
102-
/// b: u8,
103-
/// c: f64,
104-
/// }
105-
/// ```
106-
///
107-
/// This will produce:
108-
///
109-
/// ```text
110-
/// warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type
111-
/// --> <source>:5:3
112-
/// |
113-
/// 5 | c: f64,
114-
/// | ^^^^^^
115-
/// |
116-
/// = note: `#[warn(uses_power_alignment)]` on by default
117-
/// ```
118-
///
119-
/// ### Explanation
120-
///
121-
/// The power alignment rule specifies that the above struct has the
122-
/// following alignment:
123-
/// - offset_of!(Floats, a) == 0
124-
/// - offset_of!(Floats, b) == 8
125-
/// - offset_of!(Floats, c) == 12
126-
///
127-
/// However, Rust currently aligns `c` at `offset_of!(Floats, c) == 16`.
128-
/// Using offset 12 would be unsound since `f64` generally must be 8-aligned on this target.
129-
/// Thus, a warning is produced for the above struct.
130-
USES_POWER_ALIGNMENT,
131-
Warn,
132-
"Structs do not follow the power alignment rule under repr(C)"
133-
}
134-
13577
declare_lint_pass!(ImproperCTypesLint => [
13678
IMPROPER_CTYPES,
13779
IMPROPER_CTYPES_DEFINITIONS,
138-
USES_POWER_ALIGNMENT
13980
]);
14081

14182
/// Check a variant of a non-exhaustive enum for improper ctypes
@@ -174,75 +115,6 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool {
174115
!matches!(variant.ctor_kind(), Some(CtorKind::Const))
175116
}
176117

177-
/// Per-struct-field function that checks if a struct definition follows
178-
/// the Power alignment Rule (see the `check_struct_for_power_alignment` function).
179-
fn check_arg_for_power_alignment<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
180-
let tcx = cx.tcx;
181-
assert!(tcx.sess.target.os == Os::Aix);
182-
// Structs (under repr(C)) follow the power alignment rule if:
183-
// - the first field of the struct is a floating-point type that
184-
// is greater than 4-bytes, or
185-
// - the first field of the struct is an aggregate whose
186-
// recursively first field is a floating-point type greater than
187-
// 4 bytes.
188-
if ty.is_floating_point() && ty.primitive_size(tcx).bytes() > 4 {
189-
return true;
190-
} else if let Adt(adt_def, _) = ty.kind()
191-
&& adt_def.is_struct()
192-
&& adt_def.repr().c()
193-
&& !adt_def.repr().packed()
194-
&& adt_def.repr().align.is_none()
195-
{
196-
let struct_variant = adt_def.variant(VariantIdx::ZERO);
197-
// Within a nested struct, all fields are examined to correctly
198-
// report if any fields after the nested struct within the
199-
// original struct are misaligned.
200-
for struct_field in &struct_variant.fields {
201-
let field_ty = tcx.type_of(struct_field.did).instantiate_identity();
202-
if check_arg_for_power_alignment(cx, field_ty) {
203-
return true;
204-
}
205-
}
206-
}
207-
return false;
208-
}
209-
210-
/// Check a struct definition for respect of the Power alignment Rule (as in PowerPC),
211-
/// which should be respected in the "aix" target OS.
212-
/// To do so, we must follow one of the two following conditions:
213-
/// - The first field of the struct must be floating-point type that
214-
/// is greater than 4-bytes.
215-
/// - The first field of the struct must be an aggregate whose
216-
/// recursively first field is a floating-point type greater than
217-
/// 4 bytes.
218-
fn check_struct_for_power_alignment<'tcx>(
219-
cx: &LateContext<'tcx>,
220-
item: &'tcx hir::Item<'tcx>,
221-
adt_def: AdtDef<'tcx>,
222-
) {
223-
let tcx = cx.tcx;
224-
225-
// Only consider structs (not enums or unions) on AIX.
226-
if tcx.sess.target.os != Os::Aix || !adt_def.is_struct() {
227-
return;
228-
}
229-
230-
// The struct must be repr(C), but ignore it if it explicitly specifies its alignment with
231-
// either `align(N)` or `packed(N)`.
232-
if adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none() {
233-
let struct_variant_data = item.expect_struct().2;
234-
for field_def in struct_variant_data.fields().iter().skip(1) {
235-
// Struct fields (after the first field) are checked for the
236-
// power alignment rule, as fields after the first are likely
237-
// to be the fields that are misaligned.
238-
let ty = tcx.type_of(field_def.def_id).instantiate_identity();
239-
if check_arg_for_power_alignment(cx, ty) {
240-
cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment);
241-
}
242-
}
243-
}
244-
}
245-
246118
#[derive(Clone, Copy)]
247119
enum CItemKind {
248120
Declaration,
@@ -844,23 +716,6 @@ impl<'tcx> ImproperCTypesLint {
844716
}
845717
}
846718

847-
/// For a local definition of a #[repr(C)] struct/enum/union, check that it is indeed FFI-safe.
848-
fn check_reprc_adt(
849-
&mut self,
850-
cx: &LateContext<'tcx>,
851-
item: &'tcx hir::Item<'tcx>,
852-
adt_def: AdtDef<'tcx>,
853-
) {
854-
debug_assert!(
855-
adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none()
856-
);
857-
858-
// FIXME(ctypes): this following call is awkward.
859-
// is there a way to perform its logic in MIR space rather than HIR space?
860-
// (so that its logic can be absorbed into visitor.visit_struct_or_union)
861-
check_struct_for_power_alignment(cx, item, adt_def);
862-
}
863-
864719
fn check_foreign_static(&mut self, cx: &LateContext<'tcx>, id: hir::OwnerId, span: Span) {
865720
let ty = cx.tcx.type_of(id).instantiate_identity();
866721
let mut visitor = ImproperCTypesVisitor::new(cx, ty, CItemKind::Declaration);
@@ -996,15 +851,7 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
996851
}
997852
// See `check_fn` for declarations, `check_foreign_items` for definitions in extern blocks
998853
hir::ItemKind::Fn { .. } => {}
999-
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {
1000-
// looking for extern FnPtr:s is delegated to `check_field_def`.
1001-
let adt_def: AdtDef<'tcx> = cx.tcx.adt_def(item.owner_id.to_def_id());
1002-
1003-
if adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none()
1004-
{
1005-
self.check_reprc_adt(cx, item, adt_def);
1006-
}
1007-
}
854+
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {}
1008855

1009856
// Doesn't define something that can contain a external type to be checked.
1010857
hir::ItemKind::Impl(..)

0 commit comments

Comments
 (0)