Re: [PATCH v3 1/2] rust: add projection infrastructure

["Gary Guo" <gary@garyguo.net>]

On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 3da92f18f4ee..50866b481bdb 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -20,6 +20,7 @@
>>  #![feature(generic_nonzero)]
>>  #![feature(inline_const)]
>>  #![feature(pointer_is_aligned)]
>> +#![feature(slice_ptr_len)]
>
> This is missing a stability comment (stable since 1.79).
>
>>  //
>>  // Stable since Rust 1.80.0.
>>  #![feature(slice_flatten)]
> ...
>> +/// A helper trait to perform index projection.
>> +///
>> +/// This is similar to `core::slice::SliceIndex`, but operate on raw pointers safely and fallibly.
>> +///
>> +/// # Safety
>> +///
>> +/// `get` must return a pointer in bounds of the provided pointer.
>
> This only makes sense when the provided pointer already points at an
> allocation. But since the functions of this trait aren't `unsafe`, it
> must be sound to pass `ptr::null` to them.

The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
with size `x`, the address of the returned pointer lies between `ptr .. ptr +
x`.

>
> I first thought that we might be able to just use `mem::size_of_val_raw`
> [1] to give an upper and lower bound on the address of the returned
> pointer, but that is unsafe and cannot be called with an arbitrary
> pointer. Interestingly, `ptr::metadata` [2] can be called safely & with
> any pointer; I would expect them to be very similar (except of course
> for extern types).
>
> [1]: https://doc.rust-lang.org/std/mem/fn.size_of_val_raw.html
> [2]: https://doc.rust-lang.org/std/ptr/fn.metadata.html

I have a `KnownSize` trait for this in my I/O projection series that is
implemented for `T: Sized` and `[T]`, and it returns the size when given a raw
pointer.

>
> A pretty expensive solution would be to add a sealed trait `Indexable`
> that we implement for all things that `T` is allowed to be; and then we
> provide a safe function in that trait to query the maximum offset the
> `get` function is allowed to make.
>
> Alternatively, we could use something like this:
>
>     The implementation of `get` must:
>     - return a pointer obtained by offsetting the input pointer.
>     - ensure that when the input pointer points at a valid value of type
>       `T`, the offset must not be greater than [`mem::size_of_val_raw`]
>       of the input pointer.

Given that I'm not introducing `KnownSize` trait in this patch, this is why I
haven't used this kind of wording. Perhaps I can just bring `KnownSize` in early
and use it first for documentation purpose only?

>
> Or something simpler that says "if the input pointer is valid, then
> `get` must return a valid output pointer"?

Hmm, wouldn't this give impression that "you can do whatever you want if the
input pointer is not valid"?

>
>> +#[diagnostic::on_unimplemented(message = "`{Self}` cannot be used to index `{T}`")]
>> +#[doc(hidden)]
>> +pub unsafe trait ProjectIndex<T: ?Sized>: Sized {
>> +    type Output: ?Sized;
>> +
>> +    /// Returns an index-projected pointer, if in bounds.
>> +    fn get(self, slice: *mut T) -> Option<*mut Self::Output>;
>
> How about we name this `try_index` instead of the general `get`?

I'm following the name on `SliceIndex`:
https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html.

Best,
Gary

>
>> +
>> +    /// Returns an index-projected pointer; fail the build if it cannot be proved to be in bounds.
>> +    #[inline(always)]
>> +    fn index(self, slice: *mut T) -> *mut Self::Output {
>> +        Self::get(self, slice).unwrap_or_else(|| build_error!())
>> +    }
>> +}
> ...
>> +/// A helper trait to perform field projection.
>> +///
>> +/// This trait has a `DEREF` generic parameter so it can be implemented twice for types that
>> +/// implement `Deref`. This will cause an ambiguity error and thus block `Deref` types being used
>> +/// as base of projection, as they can inject unsoundness.
>
> I think it's important to also say that the ambiguity error only happens
> when calling the function without specifying the `DEREF` constant.
> Essentially it is a load-bearing part of the macro that it does this.
>
>> +///
>> +/// # Safety
>> +///
>> +/// `proj` should invoke `f` with valid allocation, as documentation described.
>
> s/should invoke `f` with/may invoke `f` only with a/
>
> "should" sounds like only a suggestion. If it is a requirement, then the
> `build_error!` impl of the `DEREF = true` case would be violating it.
>
>> +#[doc(hidden)]
>> +pub unsafe trait ProjectField<const DEREF: bool> {
>> +    /// Project a pointer to a type to a pointer of a field.
>> +    ///
>> +    /// `f` is always invoked with valid allocation so it can safely obtain raw pointers to fields
>> +    /// using `&raw mut`.
>> +    ///
>> +    /// This is needed because `base` might not point to a valid allocation, while `&raw mut`
>> +    /// requires pointers to be in bounds of a valid allocation.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// `f` must returns a pointer in bounds of the provided pointer.
>
> Typo: "must returns" -> "must return"
>
> Cheers,
> Benno
>
>> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F;
>> +}