Allow borrowing array elements from packed structs with ABI align <= packed align

[hns1971]

Fixes #145376

Summary

This PR fixes a false positive E0793 ("reference to packed field is unaligned")
when borrowing from a #[repr(C, packed)] struct field whose type is a const-generic array
with an element type that has ABI alignment <= the struct's packed alignment.

Example before this change:

#[repr(C, packed)]
struct PascalString<const CAP: usize> {
    len: u8,
    buf: [u8; CAP],
}

fn bar<const CAP: usize>(s: &PascalString<CAP>) -> &str {
    std::str::from_utf8(&s.buf[0..s.len as usize]).unwrap()
}

[rustbot]

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

[gralpli]

Hey, I’m the bug reporter. Just what I noticed on a first glance.

[lcnr]

why? it feels a lot safer to conservatively return true here, same as when layout_of fails

[lcnr]

still relevant

[lcnr]

we're not making a reasonable assumption. Their layout literally only depend son the layout of their element type, doesn't it?

Getting this right is soundness critical, so we need to be confident about that we don#t introduce false negatives.

Also, please rename this function to is_potentially_disaligned, to clarify that it's allowed to return false positives, but not false nnegatives

[RalfJung]

We use "misaligned" elsewhere in the compiler, maybe let's go with that instead of "disaligned" (which I never saw/heard before).

[lcnr]

also still relevant

[hns1971]

Thanks for the detailed feedback!

I agree with the soundness concern and with using the term “misaligned”.
I will:

• rename the function to is_potentially_misaligned and update call sites/docs to clarify that it may return false positives but must not return false negatives (soundness-critical),
• avoid relaxing the check when layout_of(ty) fails in general.

To still address the motivating case without introducing false negatives, I’ll only add a very small, layout-free special-case:
when ty.kind() is Array(elem, _) and elem is u8 or i8 (which have ABI alignment 1 by definition, independent of the array length), we treat the borrow as not potentially misaligned. All other cases remain conservative and return true when layout_of(ty) fails.

This fixes the [u8; CAP] packed-struct case while keeping the function strictly conservative for any type whose ABI alignment could exceed the packed alignment.

If you’d prefer the even stricter variant (no special-casing at all), I can drop the exception and keep returning true on layout_of failure across the board.

[lcnr]

I think ignoring the length and only checking the argument is fine. My issue was with the comment, not the behavior. We already guarantee that the alignment constraints are equal:

you can get a &T out of an &[T], so [T] must have stronger requirements than &T

you can get a &[T] from a &T via std::array::from_ref , so T must have stronger requirements than &[T]

[hns1971]

Thanks! I’ve updated the comment to formally justify that align([T]) == align(T),
using the two directions you mentioned (&[T] -> &T and std::array::from_ref(&T)).
Behavior remains unchanged: we only consider the element type’s ABI alignment and
ignore the array length.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[hns1971]

Fixed:

1. Rename is_disaligned to is_potentially_disaligned for better semantic clarity
2. Unified to ty::Array(element_ty, _) | ty::Slice(element_ty) => …
3. Modify Chinese comments to English comments.

Thanks

[hns1971]

I've updated the PR as requested (renamed function, updated comment, removed special-casing). Just let me know if further adjustments are needed.

[lcnr]

properly compute the of the element_ty here, limiting it to only u8 and i8 feels unnecessary 🤔

[lcnr]

please merge this into a single test and use generic-length instead of const-generic in the file name

[lcnr]

cc @rust-lang/lang this causes us to no longer lint "due to a borrow of a potentially misaligned field" when borrowing from a [sufficiently_small_alignment; N] field. We previous failed to compute the layout of that array but now manually check the alignment of the array element type.

I feel like this may be fine without an FCP as it seems like a small enough change 🤷

[traviscross]

Thanks for the ping. Specifically, we were giving a hard error here, so allowing this is a stabilization.

Since we're relatively caught up, it'll be as fast for us to FCP this as to decide not to do so, and lately we've generally been leaning toward FCPing everything that we technically should. We could waive our 10-day period, but it doesn't seem likely that'll affect what release this lands in, so let's just, in the normal way...

@rfcbot fcp merge
@rustbot labels +I-lang-easy-decision

[rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• exact-rules-list (Allow borrowing array elements from packed structs with ABI align <= packed align #145419 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[lcnr]

Oh, yeah, thought this was a lint 👍 an FCP seems appropriate

[scottmcm]

Yeah, if reverting it would be a breaking change, we've got to FCP the initial merge.

👍 to the general change of being smarter about alignment where possible.

@rfcbot reviewed

That said, I think we need the rules written up more. Since this is a hard error, it's something that would go into a spec so that other things could make the same checks.

@rfcbot concern exact-rules-list

Looking at the PR, it looks like it's actually only [u8] and [i8], not, say, [NonZero<u8>]. And that feels weirdly inconsistent.

---

It reminds me of the experiments that have been done towards having niches for references, which wanted a query for alignment and size (or a sound approximation thereof) that would work even in cases where the full layout couldn't be calculated. This seems like it would like to have the same kind of thing, rather than adding a bunch of ad-hoc layout knowledge into this specific check.

[traviscross]

@rustbot labels +S-waiting-on-documentation

@hns1971, we'll probably want to see a PR to the Reference here documenting the new behavior. Let us know if you have any questions about that.

cc @rust-lang/lang-docs

[hns1971]

Thanks for the detailed feedback!

I’ve updated the PR with the following:

• Generalized the check to any [T; N]: when layout_of([T; N]) is unavailable,
  we compute layout_of(T) and compare abi_align(T) to the packed alignment.
  This relies on align([T]) == align(T) (length is irrelevant).
  I also updated the comment with the proof sketch:
  (1) &[T] -> &T implies align([T]) >= align(T)
  (2) std::array::from_ref(&T) -> &[T; 1] implies align(T) >= align([T])
  Hence align([T]) == align(T).
• Renamed the helper to is_potentially_misaligned and switched terminology to “misaligned”.
  The doc comment now explicitly states that this may return false positives, but must not
  return false negatives (soundness-critical).
• Merged tests into a single file and renamed it to packed-array-generic-length.rs.
  The test covers:
  • allowed: [u8; N], [i8; N], [NonZeroU8; N], [MaybeUninit<u8>; N],
    and #[repr(transparent)](u8) wrappers
  • still error: [u16; N], [NonZeroU16; N], and #[repr(transparent)](u16) wrappers

I’ve also drafted the exact rules in the PR description and opened a Reference PR
documenting this behavior (see rust-lang/reference#1984).
Please let me know if you want the rules tweaked further or if there’s a preferred place
in the Reference to put this.