Update patch to elaborate on comments, add more tests, and restrict to floats greater than 4-bytes in size.

amy-kwan · amy-kwan · commit fa6bf1f3bd29 · 2025-01-20T16:12:12.000-05:00
diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs
@@ -1,9 +1,7 @@
 use std::iter;
 use std::ops::ControlFlow;
 
-use rustc_abi::{
-    BackendRepr, ExternAbi, FieldIdx, TagEncoding, VariantIdx, 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};
@@ -26,8 +24,8 @@ use crate::lints::{
     AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
     AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons,
     InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons,
-    UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons,
-    UsesPowerAlignment, VariantSizeDifferencesDiag,
+    UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, UsesPowerAlignment,
+    VariantSizeDifferencesDiag,
 };
 use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
 
@@ -731,13 +729,23 @@ declare_lint! {
 }
 
 declare_lint! {
-    /// The AIX target follows the power alignment rule. This rule specifies
-    /// that structs with either a:
-    ///   - floating-point data type as its first member, or
-    ///   - first member being an aggregate whose recursively first member is a
-    ///     floating-point data type
-    /// must have its natural alignment. This is currently unimplemented within
-    /// the compiler, so a warning is produced in these situations.
+    /// In it's platform C ABI, the AIX target follows the power alignment rule
+    /// for structs. This rule applies the natural alignment of the first struct
+    /// field if it is a floating-point type that is greater than 4-bytes, or
+    /// is an aggregate whose recursively "first" field is a floating-point
+    /// type greater than 4-bytes.
+    /// The alignment for subsequent fields of the associated structs use an
+    /// alignment value where the floating-point type is aligned on a 4-byte
+    /// boundary.
+    /// In this case, "natural alignment" (detailed in
+    /// https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment)
+    /// refers to the floating-point type being aligned to a multiple of its
+    /// size (for example, the size of a double is 8-bytes, and so is 8-byte
+    /// aligned).
+    ///
+    /// The power alignment rule for structs need for C compatibility is
+    /// currently unimplemented within repr(C) in the compiler, so a warning is
+    /// produced in these situations.
     ///
     /// ### Example
     ///
@@ -749,11 +757,16 @@ declare_lint! {
     /// }
     /// ```
     ///
-    /// A warning should be produced for the above struct as the first member
-    /// of the struct is an f64.
+    /// 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,
-    "floating point types within structs do not follow the power alignment rule under repr(C)"
+    "Structs do not follow the power alignment rule under repr(C)"
 }
 
 declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS, USES_POWER_ALIGNMENT]);
@@ -1575,18 +1588,25 @@ impl ImproperCTypesDefinitions {
         ty: Ty<'tcx>,
     ) -> bool {
         // Structs (under repr(C)) follow the power alignment rule if:
-        //   - the first argument of the struct is a floating-point type, or
-        //   - the first argument of the struct is an aggregate whose
-        //     recursively first member is a floating-point type
-        if ty.is_floating_point() {
+        //   - 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 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 first_variant = adt_def.variant(VariantIdx::ZERO);
-            if let Some(first) = first_variant.fields.get(FieldIdx::ZERO) {
-                let field_ty = cx.tcx.type_of(first.did).instantiate_identity();
-                return self.check_arg_for_power_alignment(cx, field_ty);
+            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;
@@ -1599,17 +1619,26 @@ impl ImproperCTypesDefinitions {
     ) {
         let adt_def = cx.tcx.adt_def(item.owner_id.to_def_id());
         if adt_def.repr().c()
-            && cx.tcx.sess.target.os == "aix"
+            && !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;
-            // Analyze only the first member of the struct to see if it
-            // follows the power alignment rule.
-            let first_field_def = struct_variant_data.fields()[0];
-            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);
+            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,
+                        );
+                    }
+                }
             }
         }
     }
diff --git a/tests/ui/layout/reprc-power-alignment.aix.stderr b/tests/ui/layout/reprc-power-alignment.aix.stderr
@@ -1,7 +1,7 @@
 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:10:5
+  --> $DIR/reprc-power-alignment.rs:12:5
    |
-LL |     a: f64,
+LL |     c: f64,
    |     ^^^^^^
    |
 note: the lint level is defined here
@@ -11,22 +11,100 @@ 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:23:5
+  --> $DIR/reprc-power-alignment.rs:39:5
    |
-LL |     a: f32,
+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:37:5
+  --> $DIR/reprc-power-alignment.rs:46:5
    |
-LL |     x: Floats,
+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:52: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:49:5
+  --> $DIR/reprc-power-alignment.rs:58: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:59: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:65:5
    |
-LL |     x: FloatAgg1,
+LL |     y: FloatAgg2,
    |     ^^^^^^^^^^^^
 
-warning: 4 warnings emitted
+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:71: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:72: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:78: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: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:98: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:103: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:112: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:117: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:123: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:125:5
+   |
+LL |     e: f64,
+   |     ^^^^^^
+
+warning: 17 warnings emitted
 
diff --git a/tests/ui/layout/reprc-power-alignment.rs b/tests/ui/layout/reprc-power-alignment.rs
