user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Implement volatile_composites lint (#15686)

Browse source 
Volatile reads and writes to non-primitive types are not well-defined,
and can cause problems.

Fixes rust-lang/rust-clippy#15529

changelog: [`volatile_composites`]: Lint when read/write_volatile is
used on composite types
(structs, arrays, etc) as their semantics are not well defined.
This commit is contained in:
Samuel Tardieu 2025-10-06 21:06:29 +00:00 • committed by GitHub
commit 2c71638be5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 496 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -6798,6 +6798,7 @@ Released 2018-09-13
[`vec_resize_to_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_resize_to_zero
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
[`volatile_composites`]: https://rust-lang.github.io/rust-clippy/master/index.html#volatile_composites
[`vtable_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons
[`waker_clone_wake`]: https://rust-lang.github.io/rust-clippy/master/index.html#waker_clone_wake
[`while_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_float

1
clippy_lints/src/declared_lints.rs
View file

@ -780,6 +780,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::visibility::NEEDLESS_PUB_SELF_INFO,
crate::visibility::PUB_WITHOUT_SHORTHAND_INFO,
crate::visibility::PUB_WITH_SHORTHAND_INFO,
crate::volatile_composites::VOLATILE_COMPOSITES_INFO,
crate::wildcard_imports::ENUM_GLOB_USE_INFO,
crate::wildcard_imports::WILDCARD_IMPORTS_INFO,
crate::write::PRINTLN_EMPTY_STRING_INFO,

2
clippy_lints/src/lib.rs
View file

@ -399,6 +399,7 @@ mod useless_conversion;
mod vec;
mod vec_init_then_push;
mod visibility;
mod volatile_composites;
mod wildcard_imports;
mod write;
mod zero_div_zero;
@ -830,5 +831,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
// add lints here, do not remove this comment, it's used in `new_lint`
}

180
clippy_lints/src/volatile_composites.rs Normal file
View file

@ -0,0 +1,180 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::sym;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
use rustc_session::declare_lint_pass;
declare_clippy_lint! {
/// ### What it does
///
/// This lint warns when volatile load/store operations
/// (`write_volatile`/`read_volatile`) are applied to composite types.
///
/// ### Why is this bad?
///
/// Volatile operations are typically used with memory mapped IO devices,
/// where the precise number and ordering of load and store instructions is
/// important because they can have side effects. This is well defined for
/// primitive types like `u32`, but less well defined for structures and
/// other composite types. In practice it's implementation defined, and the
/// behavior can be rustc-version dependent.
///
/// As a result, code should only apply `write_volatile`/`read_volatile` to
/// primitive types to be fully well-defined.
///
/// ### Example
/// ```no_run
/// struct MyDevice {
/// addr: usize,
/// count: usize
/// }
///
/// fn start_device(device: *mut MyDevice, addr: usize, count: usize) {
/// unsafe {
/// device.write_volatile(MyDevice { addr, count });
/// }
/// }
/// ```
/// Instead, operate on each primtive field individually:
/// ```no_run
/// struct MyDevice {
/// addr: usize,
/// count: usize
/// }
///
/// fn start_device(device: *mut MyDevice, addr: usize, count: usize) {
/// unsafe {
/// (&raw mut (*device).addr).write_volatile(addr);
/// (&raw mut (*device).count).write_volatile(count);
/// }
/// }
/// ```
#[clippy::version = "1.92.0"]
pub VOLATILE_COMPOSITES,
nursery,
"warn about volatile read/write applied to composite types"
}
declare_lint_pass!(VolatileComposites => [VOLATILE_COMPOSITES]);
/// Zero-sized types are intrinsically safe to use volatile on since they won't
/// actually generate *any* loads or stores. But this is also used to skip zero-sized
/// fields of `#[repr(transparent)]` structures.
fn is_zero_sized_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
cx.layout_of(ty).is_ok_and(|layout| layout.is_zst())
}
/// A thin raw pointer or reference.
fn is_narrow_ptr<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
match ty.kind() {
ty::RawPtr(inner, _) | ty::Ref(_, inner, _) => inner.has_trivial_sizedness(cx.tcx, ty::SizedTraitKind::Sized),
_ => false,
}
}
/// Enum with some fixed representation and no data-carrying variants.
fn is_enum_repr_c<'tcx>(_cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty.ty_adt_def().is_some_and(|adt_def| {
adt_def.is_enum() && adt_def.repr().inhibit_struct_field_reordering() && adt_def.is_payloadfree()
})
}
/// `#[repr(transparent)]` structures are also OK if the only non-zero
/// sized field contains a volatile-safe type.
fn is_struct_repr_transparent<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
if let ty::Adt(adt_def, args) = ty.kind()
&& adt_def.is_struct()
&& adt_def.repr().transparent()
&& let [fieldty] = adt_def
.all_fields()
.filter_map(|field| {
let fty = field.ty(cx.tcx, args);
if is_zero_sized_ty(cx, fty) { None } else { Some(fty) }
})
.collect::<Vec<_>>()
.as_slice()
{
is_volatile_safe_ty(cx, *fieldty)
} else {
false
}
}
/// SIMD can be useful to get larger single loads/stores, though this is still
/// pretty machine-dependent.
fn is_simd_repr<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
if let ty::Adt(adt_def, _args) = ty.kind()
&& adt_def.is_struct()
&& adt_def.repr().simd()
{
let (_size, simdty) = ty.simd_size_and_type(cx.tcx);
is_volatile_safe_ty(cx, simdty)
} else {
false
}
}
/// Top-level predicate for whether a type is volatile-safe or not.
fn is_volatile_safe_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty.is_primitive()
|| is_narrow_ptr(cx, ty)
|| is_zero_sized_ty(cx, ty)
|| is_enum_repr_c(cx, ty)
|| is_simd_repr(cx, ty)
|| is_struct_repr_transparent(cx, ty)
// We can't know about a generic type, so just let it pass to avoid noise
|| ty.has_non_region_param()
}
/// Print diagnostic for volatile read/write on non-volatile-safe types.
fn report_volatile_safe<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) {
if !is_volatile_safe_ty(cx, ty) {
span_lint(
cx,
VOLATILE_COMPOSITES,
expr.span,
format!("type `{ty}` is not volatile-compatible"),
);
}
}
impl<'tcx> LateLintPass<'tcx> for VolatileComposites {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
// Check our expr is calling a method with pattern matching
match expr.kind {
// Look for method calls to `write_volatile`/`read_volatile`, which
// apply to both raw pointers and std::ptr::NonNull.
ExprKind::MethodCall(name, self_arg, _, _)
if matches!(name.ident.name, sym::read_volatile | sym::write_volatile) =>
{
let self_ty = cx.typeck_results().expr_ty(self_arg);
match self_ty.kind() {
// Raw pointers
ty::RawPtr(innerty, _) => report_volatile_safe(cx, expr, *innerty),
// std::ptr::NonNull
ty::Adt(_, args) if is_type_diagnostic_item(cx, self_ty, sym::NonNull) => {
report_volatile_safe(cx, expr, args.type_at(0));
},
_ => (),
}
},
// Also plain function calls to std::ptr::{read,write}_volatile
ExprKind::Call(func, [arg_ptr, ..]) => {
if let ExprKind::Path(ref qpath) = func.kind
&& let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
&& matches!(
cx.tcx.get_diagnostic_name(def_id),
Some(sym::ptr_read_volatile | sym::ptr_write_volatile)
)
&& let ty::RawPtr(ptrty, _) = cx.typeck_results().expr_ty_adjusted(arg_ptr).kind()
{
report_volatile_safe(cx, expr, *ptrty);
}
},
_ => {},
}
}
}

2
clippy_utils/src/sym.rs
View file

@ -265,6 +265,7 @@ generate! {
read_to_end,
read_to_string,
read_unaligned,
read_volatile,
redundant_imports,
redundant_pub_crate,
regex,
@ -374,6 +375,7 @@ generate! {
wrapping_offset,
write,
write_unaligned,
write_volatile,
writeln,
zip,
}

221
tests/ui/volatile_composites.rs Normal file
View file

@ -0,0 +1,221 @@
#![feature(ptr_metadata)]
#![feature(portable_simd)]
#![warn(clippy::volatile_composites)]
use std::ptr::null_mut;
#[repr(C)]
#[derive(Copy, Clone, Default)]
struct MyDevRegisters {
baseaddr: usize,
count: usize,
}
#[repr(transparent)]
struct Wrapper<T>((), T, ());
// Not to be confused with std::ptr::NonNull
struct NonNull<T>(T);
impl<T> NonNull<T> {
fn write_volatile(&self, _arg: &T) {
unimplemented!("Something entirely unrelated to std::ptr::NonNull");
}
}
fn main() {
let regs = MyDevRegisters {
baseaddr: 0xabc123,
count: 42,
};
const DEVICE_ADDR: *mut MyDevRegisters = 0xdead as *mut _;
// Raw pointer methods
unsafe {
(&raw mut (*DEVICE_ADDR).baseaddr).write_volatile(regs.baseaddr); // OK
(&raw mut (*DEVICE_ADDR).count).write_volatile(regs.count); // OK
DEVICE_ADDR.write_volatile(regs);
//~^ volatile_composites
let _regs = MyDevRegisters {
baseaddr: (&raw const (*DEVICE_ADDR).baseaddr).read_volatile(), // OK
count: (&raw const (*DEVICE_ADDR).count).read_volatile(), // OK
};
let _regs = DEVICE_ADDR.read_volatile();
//~^ volatile_composites
}
// std::ptr functions
unsafe {
std::ptr::write_volatile(&raw mut (*DEVICE_ADDR).baseaddr, regs.baseaddr); // OK
std::ptr::write_volatile(&raw mut (*DEVICE_ADDR).count, regs.count); // OK
std::ptr::write_volatile(DEVICE_ADDR, regs);
//~^ volatile_composites
let _regs = MyDevRegisters {
baseaddr: std::ptr::read_volatile(&raw const (*DEVICE_ADDR).baseaddr), // OK
count: std::ptr::read_volatile(&raw const (*DEVICE_ADDR).count), // OK
};
let _regs = std::ptr::read_volatile(DEVICE_ADDR);
//~^ volatile_composites
}
// core::ptr functions
unsafe {
core::ptr::write_volatile(&raw mut (*DEVICE_ADDR).baseaddr, regs.baseaddr); // OK
core::ptr::write_volatile(&raw mut (*DEVICE_ADDR).count, regs.count); // OK
core::ptr::write_volatile(DEVICE_ADDR, regs);
//~^ volatile_composites
let _regs = MyDevRegisters {
baseaddr: core::ptr::read_volatile(&raw const (*DEVICE_ADDR).baseaddr), // OK
count: core::ptr::read_volatile(&raw const (*DEVICE_ADDR).count), // OK
};
let _regs = core::ptr::read_volatile(DEVICE_ADDR);
//~^ volatile_composites
}
// std::ptr::NonNull
unsafe {
let ptr = std::ptr::NonNull::new(DEVICE_ADDR).unwrap();
ptr.write_volatile(regs);
//~^ volatile_composites
let _regs = ptr.read_volatile();
//~^ volatile_composites
}
// Red herring
{
let thing = NonNull("hello".to_string());
thing.write_volatile(&"goodbye".into()); // OK
}
// Zero size types OK
unsafe {
struct Empty;
(0xdead as *mut Empty).write_volatile(Empty); // OK
// Note that this is OK because Wrapper<Empty> is itself ZST, not because of the repr transparent
// handling tested below.
(0xdead as *mut Wrapper<Empty>).write_volatile(Wrapper((), Empty, ())); // OK
}
// Via repr transparent newtype
unsafe {
(0xdead as *mut Wrapper<usize>).write_volatile(Wrapper((), 123, ())); // OK
(0xdead as *mut Wrapper<Wrapper<usize>>).write_volatile(Wrapper((), Wrapper((), 123, ()), ())); // OK
(0xdead as *mut Wrapper<MyDevRegisters>).write_volatile(Wrapper((), MyDevRegisters::default(), ()));
//~^ volatile_composites
}
// Plain type alias OK
unsafe {
type MyU64 = u64;
(0xdead as *mut MyU64).write_volatile(123); // OK
}
// Wide pointers are not OK as data
unsafe {
let things: &[u32] = &[1, 2, 3];
(0xdead as *mut *const u32).write_volatile(things.as_ptr()); // OK
let wideptr: *const [u32] = std::ptr::from_raw_parts(things.as_ptr(), things.len());
(0xdead as *mut *const [u32]).write_volatile(wideptr);
//~^ volatile_composites
}
// Plain pointers and pointers with lifetimes are OK
unsafe {
let v: u32 = 123;
let rv: &u32 = &v;
(0xdead as *mut &u32).write_volatile(rv); // OK
}
// C-style enums are OK
unsafe {
// Bad: need some specific repr
enum PlainEnum {
A = 1,
B = 2,
C = 3,
}
(0xdead as *mut PlainEnum).write_volatile(PlainEnum::A);
//~^ volatile_composites
// OK
#[repr(u32)]
enum U32Enum {
A = 1,
B = 2,
C = 3,
}
(0xdead as *mut U32Enum).write_volatile(U32Enum::A); // OK
// OK
#[repr(C)]
enum CEnum {
A = 1,
B = 2,
C = 3,
}
(0xdead as *mut CEnum).write_volatile(CEnum::A); // OK
// Nope
enum SumType {
A(String),
B(u32),
C,
}
(0xdead as *mut SumType).write_volatile(SumType::C);
//~^ volatile_composites
// A repr on a complex sum type is not good enough
#[repr(C)]
enum ReprSumType {
A(String),
B(u32),
C,
}
(0xdead as *mut ReprSumType).write_volatile(ReprSumType::C);
//~^ volatile_composites
}
// SIMD is OK
unsafe {
(0xdead as *mut std::simd::u32x4).write_volatile(std::simd::u32x4::splat(1)); // OK
}
// Can't see through generic wrapper
unsafe {
do_device_write::<MyDevRegisters>(0xdead as *mut _, Default::default()); // OK
}
let mut s = String::from("foo");
unsafe {
std::ptr::write_volatile(&mut s, String::from("bar"));
//~^ volatile_composites
}
}
// Generic OK
unsafe fn do_device_write<T>(ptr: *mut T, v: T) {
unsafe {
ptr.write_volatile(v); // OK
}
}

89
tests/ui/volatile_composites.stderr Normal file
View file

@ -0,0 +1,89 @@
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:39:9
|
LL | DEVICE_ADDR.write_volatile(regs);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::volatile-composites` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::volatile_composites)]`
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:47:21
|
LL | let _regs = DEVICE_ADDR.read_volatile();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:56:9
|
LL | std::ptr::write_volatile(DEVICE_ADDR, regs);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:64:21
|
LL | let _regs = std::ptr::read_volatile(DEVICE_ADDR);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:73:9
|
LL | core::ptr::write_volatile(DEVICE_ADDR, regs);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:81:21
|
LL | let _regs = core::ptr::read_volatile(DEVICE_ADDR);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:89:9
|
LL | ptr.write_volatile(regs);
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: type `MyDevRegisters` is not volatile-compatible
--> tests/ui/volatile_composites.rs:92:21
|
LL | let _regs = ptr.read_volatile();
| ^^^^^^^^^^^^^^^^^^^
error: type `Wrapper<MyDevRegisters>` is not volatile-compatible
--> tests/ui/volatile_composites.rs:118:9
|
LL | (0xdead as *mut Wrapper<MyDevRegisters>).write_volatile(Wrapper((), MyDevRegisters::default(), ()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `*const [u32]` is not volatile-compatible
--> tests/ui/volatile_composites.rs:136:9
|
LL | (0xdead as *mut *const [u32]).write_volatile(wideptr);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `main::PlainEnum` is not volatile-compatible
--> tests/ui/volatile_composites.rs:157:9
|
LL | (0xdead as *mut PlainEnum).write_volatile(PlainEnum::A);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `main::SumType` is not volatile-compatible
--> tests/ui/volatile_composites.rs:185:9
|
LL | (0xdead as *mut SumType).write_volatile(SumType::C);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `main::ReprSumType` is not volatile-compatible
--> tests/ui/volatile_composites.rs:195:9
|
LL | (0xdead as *mut ReprSumType).write_volatile(ReprSumType::C);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type `std::string::String` is not volatile-compatible
--> tests/ui/volatile_composites.rs:211:9
|
LL | std::ptr::write_volatile(&mut s, String::from("bar"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 14 previous errors