cortex-m icon indicating copy to clipboard operation
cortex-m copied to clipboard

Vector Implementation

Open AdinAck opened this issue 1 year ago • 7 comments

In the cortex-m-rt docs it recommends PACs create a Vector type like so:

pub union Vector {
    handler: unsafe extern "C" fn(),
    reserved: usize,
}

and this is exactly what PACs do. I'm curious why it is like this? The reserved variant being typed as usize when it should always be 0 and constructed like:

Vector { reserved: 0 }

is very odd to me.

Why not define Vector like this:

pub struct Vector(#[allow(unused)] *const ());

impl Vector {
    /// Create a vector with the provided function pointer.
    pub const fn handler(f: unsafe extern "C" fn()) -> Self {
        Self(f as _)
    }

    /// Create a vector that is reserved.
    pub const fn reserved() -> Self {
        Self(core::ptr::null())
    }
}

and use it like this:

pub static __INTERRUPTS: [Vector; 2] = [
    Vector::handler(Foo),
    Vector::reserved(),
];

and why is this type not provided by cortex-m-rt so PACs don't have to redefine it every time?

I imagine there is some low-level issue with representing the function pointer as a unit pointer that I don't know of.

I am currently using my proposed Vector implementation and it does work, I'm just not sure if it is correct.

Thanks!

AdinAck avatar Dec 30 '24 19:12 AdinAck

I imagine there is some low-level issue with representing the function pointer as a unit pointer that I don't know of.

It should be fine in this case. The AAPCS (default extern "C" for arm, see https://doc.rust-lang.org/reference/items/external-blocks.html#abi) defines a Code Pointer as 4 byte value (see https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#51fundamental-data-types), which corresponds to a C function pointer (see https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#812pointer-types).

As *const () in Rust is not dynamically sized, it has the same memory layout as a C pointer, which corresponds to a Data Pointer in AAPCS and is also defined as a 4-byte value (see above). So, in this case, representing a function pointer as a unit pointer should be fine. (Although as far as I know, it is not explicitly allowed, at least in the C standard)

thomasw04 avatar Jan 02 '25 19:01 thomasw04

Ah, nice digging. Makes sense, thanks!

Is there a better type to use than *const () in your opinion?

AdinAck avatar Jan 06 '25 02:01 AdinAck

Is there a better type to use than *const () in your opinion?

I mean, what about something like this:

#[repr(C)]
pub struct Vector(usize);

impl Vector {
    /// Create a vector with the provided function pointer.
    pub fn handler(f: unsafe extern "C" fn()) -> Self {
        Self(f as usize)
    }

    /// Create a vector that is reserved.
    pub const fn reserved() -> Self {
        Self(0 as usize)
    }
}

thomasw04 avatar Jan 09 '25 13:01 thomasw04

Unfortunately handler must be const since it's used in the __INTERRUPTS static.

For some reason casting pointers as integers is not permitted in const contexts. (even though it's an extern function)

AdinAck avatar Jan 10 '25 17:01 AdinAck

There was recently discussion about cortex-m-rt's use of a union here, see: https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Is.20NPO.20guaranteed.20for.20.60Option.3C*mut.20T.3E.60

What about just using Option<unsafe extern "C" fn()> nowadays? The docs call out that transmute::<_, Option<T>>([0u8; size_of::<T>()]) where T = fn() is sound and produces Option::None (i.e. that an all bit-pattern 0 represents the None), and naturally, this must also hold in reverse (that a None produces an all bit-pattern 0).

madsmtm avatar Feb 05 '25 09:02 madsmtm

Ah, nice. From the cortex-m-rt docs:

We recommend using a union to set the reserved spots to 0; None (Option<fn()>) may also work but it’s not guaranteed that the None variant will always be represented by the value 0.

I think their concern is that the Rust guarantee that you sent is unstable, or may change between Rust releases.

So they use a union because that has to conform to the C ABI which is not going to change. A reasonable concern I suppose, but surely with a little documentation explaining how and why Option<fn()> is ok, there would be no issues until that day comes.

AdinAck avatar Feb 08 '25 17:02 AdinAck

I think their concern is that the Rust guarantee that you sent is unstable, or may change between Rust releases.

It's a stable guarantee.

I suspect it was just because that section has only been added fairly recently (Rust 1.74 does not go into this much detail, and the union Vector recommendation seems to be several years old).

madsmtm avatar Feb 08 '25 17:02 madsmtm