Skip to content

ptr::write_volatile and read_volatile are not well-defined on compound types #15529

@jsgf

Description

@jsgf

What it does

The write_volatile and read_volatile functions are documented as

Volatile operations are intended to act on I/O memory, and are guaranteed to not be elided or reordered by the compiler across other volatile operations.

But it is unclear what this means when used to write a struct with multiple fields: is it one write per field, or could they be coalesced into fewer writes? (See https://internals.rust-lang.org/t/what-are-the-semantics-of-write-volatile-with-compound-types/20438 for more of a discussion.)

That said, there's broad consensus for what these mean on types which are close to CPU primitive types, such as u8/u16/u32/u64/usize and so on. These are expected to generate a single load or store instruction (assuming the CPU can handle the corresponding size). Therefore all write_volatile or read_volatile calls should be operating on objects with this kind of type.

This has material impact. rustc's behaviour changed between 1.88 and 1.89 which broke some (erronious) code in our codebase. This lint would have helped avoid that, and will also flag any other (as yet undetected) instances in our codebase.

Advantage

I could not find any existing lints covering this class of errors.

Drawbacks

It's fairly a niche use-case, but it could flag cases which are considered to be correct.

Example

/// This represents a memory-mapped hardware device. It must be configured by setting baseaddr and count.
/// Writing to `count` also has the effect of triggering the device into action.
#[repr(C)]
struct MyRegisters {
    baseaddr: usize,
    count: usize,
}

const HARDWARE_ADDR: *mut MyRegisters = ...;

fn trigger(baseaddr: usize, count: usize) {
    // Relies on MyRegisters being written as two word writes, in order
    HARDWARE_ADDR.write_volatile(MyRegisters { baseaddr, count })
}

Could be written as:

fn trigger(baseaddr: usize, count: usize) {
     // More verbose but guaranteed to be correct
    (&raw mut (*HARDWARE_ADDR).baseaddr).write_volatile(baseaddr);
    (&raw mut (*HARDWARE_ADDR).count).write_volatile(count);
}

Comparison with existing lints

I couldn't find any similar lints.

Additional Context

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintArea: New lints

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions