Skip to content

feat(cast_ptr_alignment): also lint to-types with unknown alignment #15879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+99 −35
Draft

feat(cast_ptr_alignment): also lint to-types with unknown alignment #15879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
505a44a
clean-up
ada4a Oct 13, 2025
3d18841
feat(cast_ptr_alignment): also lint to-types with unknown alignment
ada4a Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 41 additions & 14 deletions clippy_lints/src/casts/cast_ptr_alignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::ty::is_c_void;
use clippy_utils::{get_parent_expr, is_hir_ty_cfg_dependant, sym};
use rustc_abi::Align;
use rustc_hir::{Expr, ExprKind, GenericArg};
use rustc_lint::LateContext;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::layout::{LayoutError, LayoutOf};
use rustc_middle::ty::{self, Ty};

use super::CAST_PTR_ALIGNMENT;
Expand All @@ -29,24 +30,50 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f
if let ty::RawPtr(from_ptr_ty, _) = *cast_from.kind()
&& let ty::RawPtr(to_ptr_ty, _) = *cast_to.kind()
&& let Ok(from_layout) = cx.layout_of(from_ptr_ty)
&& let Ok(to_layout) = cx.layout_of(to_ptr_ty)
&& from_layout.align.abi < to_layout.align.abi
// with c_void, we inherently need to trust the user
&& !is_c_void(cx, from_ptr_ty)
// when casting from a ZST, we don't know enough to properly lint
&& !from_layout.is_zst()
&& !is_used_as_unaligned(cx, expr)
{
span_lint(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It doesn't let me select lines above this): Would you find a let-else more readable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally liked them but it's up to you. I find this current version just fine too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean something like this?

let Ok(to_layout) = cx.layout_of(to_ptry_ty) else {
    // the logic for unknown to-alignment
};

// the logic for known-more-strict to-alignment

I don't think that would be possible unfortunately, given that there are multiple kinds of LayoutError, and only LayoutError::TooGeneric gives us access to the exact part of the to-type that made it impossible to calculate the layout and therefore the alignment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope! I mean replacing the whole if let. This does require a ton of returns but I personally like it. Again, up to you—implementation wise, your code is good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought that would only require putting returns on the let expressions, but really they would be required on the rest of the expression as well, meaning 6 returns in total, which.. I'm not sure is really desirable? It would lead to a lot of boilerplate inbetween the actual logic...

cx,
CAST_PTR_ALIGNMENT,
expr.span,
format!(
"casting from `{cast_from}` to a more-strictly-aligned pointer (`{cast_to}`) ({} < {} bytes)",
from_layout.align.bytes(),
to_layout.align.bytes(),
),
);
// When the to-type has a lower alignment, it's always properly aligned when cast. Only when the
// to-type has a higher alignment can it not be properly aligned (16 -> 32, 48 is not a multiple of
// 32!)
match cx.layout_of(to_ptr_ty) {
Ok(to_layout) => {
if from_layout.align.abi < to_layout.align.abi {
span_lint(
cx,
CAST_PTR_ALIGNMENT,
expr.span,
format!(
"casting from `{cast_from}` to a more-strictly-aligned pointer (`{cast_to}`) ({} < {} bytes)",
from_layout.align.bytes(),
to_layout.align.bytes(),
),
);
}
},
Err(LayoutError::TooGeneric(too_generic_ty)) => {
// With the maximum possible alignment, there is no "higher alignment" case.
if from_layout.align.abi != Align::MAX {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on this check? I might just be missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that part was a bit awkward to explain -- how about something like this?

// If the from-type already has the maximum possible alignment, then, whatever the unknown alignment
// of the to-type is, it can't possibly be stricter than that of the from-type

Copy link
Member

@Centri3 Centri3 Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh!! Okay I got it now ignore me 😅.

How about something like this?:

// When the to-type has a lower alignment, it's always properly aligned when cast. Only when the to-type has a higher alignment can it not be properly aligned (16 -> 32, 48 is not a multiple of 32!). With the maximum possible alignment, there is no "higher alignment" case.

Copy link
Member

@Centri3 Centri3 Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just found this a bit confusing and I think elaborating on why we're linting only the higher alignment case somewhere could be helpful to others

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, the first two sentences honestly apply to the whole lint, so maybe I could put them onto the match?

diff --git i/clippy_lints/src/casts/cast_ptr_alignment.rs w/clippy_lints/src/casts/cast_ptr_alignment.rs
index 12057fd40..76f4ae00e 100644
--- i/clippy_lints/src/casts/cast_ptr_alignment.rs
+++ w/clippy_lints/src/casts/cast_ptr_alignment.rs
@@ -36,6 +36,9 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f
         && !from_layout.is_zst()
         && !is_used_as_unaligned(cx, expr)
     {
+        // When the to-type has a lower alignment, it's always properly aligned when cast. Only when the
+        // to-type has a higher alignment can it not be properly aligned (16 -> 32, 48 is not a multiple of
+        // 32!)
         match cx.layout_of(to_ptr_ty) {
             Ok(to_layout) => {
                 if from_layout.align.abi < to_layout.align.abi {
@@ -52,6 +55,7 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f
                 }
             },
             Err(LayoutError::TooGeneric(too_generic_ty)) => {
-                // a maximally aligned type can be aligned down to anything
+                // With the maximum possible alignment, there is no "higher alignment" case.
                 if from_layout.align.abi != Align::MAX {
                     span_lint_and_then(
                         cx,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

span_lint_and_then(
cx,
CAST_PTR_ALIGNMENT,
expr.span,
format!("casting from `{cast_from}` to a possibly more-strictly-aligned pointer (`{cast_to}`)"),
|diag| {
if too_generic_ty == to_ptr_ty {
diag.note(format!("the alignment of `{too_generic_ty}` can vary"));
} else {
diag.note(format!("the alignment of the target pointer isn't known because the alignment of `{too_generic_ty}` can vary"));
}
},
);
}
},
_ => {},
}
}
}

Expand Down
12 changes: 7 additions & 5 deletions clippy_lints/src/casts/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,16 +222,18 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Checks for casts, using `as` or `pointer::cast`, from a
/// less strictly aligned pointer to a more strictly aligned pointer.
/// less strictly aligned pointer to a (possibly) more strictly aligned pointer.
///
/// ### Why is this bad?
/// Dereferencing the resulting pointer may be undefined behavior.
///
/// ### Known problems
/// Using [`std::ptr::read_unaligned`](https://doc.rust-lang.org/std/ptr/fn.read_unaligned.html) and [`std::ptr::write_unaligned`](https://doc.rust-lang.org/std/ptr/fn.write_unaligned.html) or
/// similar on the resulting pointer is fine. Is over-zealous: casts with
/// manual alignment checks or casts like `u64` -> `u8` -> `u16` can be
/// fine. Miri is able to do a more in-depth analysis.
/// Using [`std::ptr::read_unaligned`](https://doc.rust-lang.org/std/ptr/fn.read_unaligned.html)
/// and [`std::ptr::write_unaligned`](https://doc.rust-lang.org/std/ptr/fn.write_unaligned.html)
/// or similar on the resulting pointer is fine.
///
/// Is over-zealous: casts with manual alignment checks or casts like `u64` -> `u8` -> `u16` can be fine.
/// Miri is able to do a more in-depth analysis.
///
/// ### Example
/// ```no_run
Expand Down
41 changes: 30 additions & 11 deletions tests/ui/cast_alignment.rs → tests/ui/cast_ptr_alignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
//! Test casts for alignment issues

//@require-annotations-for-level: ERROR
#![feature(core_intrinsics)]
#![warn(clippy::cast_ptr_alignment)]
#![allow(
clippy::no_effect,
clippy::unnecessary_operation,
clippy::cast_lossless,
clippy::borrow_as_ptr
)]
#![expect(clippy::no_effect, clippy::cast_lossless, clippy::borrow_as_ptr)]

fn main() {
/* These should be warned against */
Expand All @@ -33,13 +27,20 @@ fn main() {
// cast to less-strictly-aligned type
(&1u16 as *const u16) as *const u8;
(&mut 1u16 as *mut u16) as *mut u8;
// For c_void, we should trust the user. See #2677
}

// For c_void, we should trust the user
fn issue_2677() {
(&1u32 as *const u32 as *const std::os::raw::c_void) as *const u32;
(&1u32 as *const u32 as *const libc::c_void) as *const u32;
// For ZST, we should trust the user. See #4256
}

// For ZST, we should trust the user
fn issue_4256() {
(&1u32 as *const u32 as *const ()) as *const u32;
}

// Issue #2881
fn issue_2881() {
let mut data = [0u8, 0u8];
unsafe {
let ptr = &data as *const [u8; 2] as *const u8;
Expand All @@ -52,3 +53,21 @@ fn main() {
core::intrinsics::unaligned_volatile_store(ptr as *mut u16, 0);
}
}

fn issue_3440() {
#[rustfmt::skip] // the error message comment gets split in 2 lines...
trait Trait {
unsafe fn frob(bytes: *const u8) -> *const Self
where
Self: Sized,
{
let _ = bytes as *const [Self; 2];
//~^ ERROR: casting from `*const u8` to a possibly more-strictly-aligned pointer (`*const [Self; 2]`)
//~| NOTE: the alignment of the target pointer isn't known because the alignment of `Self` can vary

bytes as *const Self
//~^ ERROR: casting from `*const u8` to a possibly more-strictly-aligned pointer (`*const Self`)
//~| NOTE: the alignment of `Self` can vary
}
}
}
26 changes: 21 additions & 5 deletions tests/ui/cast_alignment.stderr → tests/ui/cast_ptr_alignment.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) (1 < 2 bytes)
--> tests/ui/cast_alignment.rs:16:5
--> tests/ui/cast_ptr_alignment.rs:10:5
|
LL | (&1u8 as *const u8) as *const u16;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,22 +8,38 @@ LL | (&1u8 as *const u8) as *const u16;
= help: to override `-D warnings` add `#[allow(clippy::cast_ptr_alignment)]`

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) (1 < 2 bytes)
--> tests/ui/cast_alignment.rs:19:5
--> tests/ui/cast_ptr_alignment.rs:13:5
|
LL | (&mut 1u8 as *mut u8) as *mut u16;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) (1 < 2 bytes)
--> tests/ui/cast_alignment.rs:23:5
--> tests/ui/cast_ptr_alignment.rs:17:5
|
LL | (&1u8 as *const u8).cast::<u16>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) (1 < 2 bytes)
--> tests/ui/cast_alignment.rs:26:5
--> tests/ui/cast_ptr_alignment.rs:20:5
|
LL | (&mut 1u8 as *mut u8).cast::<u16>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors
error: casting from `*const u8` to a possibly more-strictly-aligned pointer (`*const [Self; 2]`)
--> tests/ui/cast_ptr_alignment.rs:64:21
|
LL | let _ = bytes as *const [Self; 2];
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the alignment of the target pointer isn't known because the alignment of `Self` can vary

error: casting from `*const u8` to a possibly more-strictly-aligned pointer (`*const Self`)
--> tests/ui/cast_ptr_alignment.rs:68:13
|
LL | bytes as *const Self
| ^^^^^^^^^^^^^^^^^^^^
|
= note: the alignment of `Self` can vary

error: aborting due to 6 previous errors