public inbox for linux-kbuild@vger.kernel.org
 help / color / mirror / Atom feed
From: Gary Guo <gary@garyguo.net>
To: Benno Lossin <lossin@kernel.org>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nicolas Schier" <nsc@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kbuild@vger.kernel.org
Subject: Re: [PATCH 1/4] rust: add projection infrastructure
Date: Sat, 14 Feb 2026 10:36:42 +0000	[thread overview]
Message-ID: <60bb58cecdef3ec4cda77cfad5c620ed@garyguo.net> (raw)
In-Reply-To: <DGELDM5523KS.3EY7C7X5PC1V4@kernel.org>

On 2026-02-14 09:53, Benno Lossin wrote:
> On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
>> Add a generic infrastructure for performing field and index projections on
>> raw pointers. This will form the basis of performing I/O projections.
>>
>> Pointers manipulations are intentionally using the safe wrapping variants
>> instead of the unsafe variants, as the latter requires pointers to be
>> inside an allocation which is not necessarily true for I/O pointers.
>>
>> This projection macro protects against rogue `Deref` implementation, which
>> can causes the projected pointer to be outside the bounds of starting
>> pointer. This is extremely unlikely and Rust has a lint to catch this, but
>> is unsoundness regardless. The protection works by inducing type inference
>> ambiguity when `Deref` is implemented.
>>
>> The projection macro supports both fallible and infallible index
>> projections. These are described in detail inside the documentation.
>>
>> Signed-off-by: Gary Guo <gary@garyguo.net>
> 
> Cool work!
> 
> I was wondering how you'd make this safe and general, but just having a
> primitive pointer projection macro makes a lot of sense. We'll have lots
> of projection macros that use this under the hood instead of a single
> one. I like this as a stop-gap solution until we have projections in the
> language.
> 
> I have a few comments, with those addressed:
> 
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> 
>> ---
>>  rust/kernel/lib.rs        |   5 +
>>  rust/kernel/projection.rs | 269 ++++++++++++++++++++++++++++++++++++++
>>  scripts/Makefile.build    |   4 +-
>>  3 files changed, 277 insertions(+), 1 deletion(-)
>>  create mode 100644 rust/kernel/projection.rs
> 
>> +// SAFETY: `proj` invokes `f` with valid allocation.
>> +unsafe impl<T> ProjectField<false> for T {
>> +    #[inline(always)]
>> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
>> +        // Create a valid allocation to start projection, as `base` is not necessarily so.
>> +        let mut place = MaybeUninit::uninit();
>> +        let place_base = place.as_mut_ptr();
>> +        let field = f(place_base);
>> +        // SAFETY: `field` is in bounds from `base` per safety requirement.
>> +        let offset = unsafe { field.byte_offset_from(place_base) };
>> +        base.wrapping_byte_offset(offset).cast()
>> +    }
> 
> There are several limitations with this impl. I don't think we can do
> anything about them, but it's probably good to list them somewhere:
> 1. We do not support projecting fields of unsized types, so `MyStruct<dyn Trait>`.
>    (note that slices are supported with `ProjectIndex`)
> 2. Since this creates a `MaybeUninit<T>` on the stack, only small `T`
>    are supported. I'm not sure how much of this will be optimized away,
>    but it might be the case that it is not. Projecting in the same
>    function call stack multiple times might result in overrunning the
>    stack pretty quickly.

I've verified codegen and haven't managed to get this to actually generate `T` on the stack.
LLVM always figures out that the offset is the only thing that matters and optimize away
everything. `memoffset` crate also creates a temporary `MaybeUninit`, and given that it was
very widely used before `offset_of!` is stable, I think we should be able to rely on this being
okay even for large types.

Note that I've taken care to mark everything `#[inline(always)]` when possible, even
closures passed to `proj`.

> 3. The `wrapping_byte_offset` function generates potentially worse
>    codegen when `base` points into a real allocation.

I'm highly skeptical that we'll lose any optimization, but this is indeed
a possibility in theory.

> 
>> +}
>> +
>> +// SAFETY: vacuously satisfied.
>> +unsafe impl<T: Deref> ProjectField<true> for T {
>> +    #[inline(always)]
>> +    unsafe fn proj<F>(_: *mut Self, _: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
>> +        build_error!("this function is a guard against `Deref` impl and is never invoked");
>> +    }
>> +}
>> +
>> +/// Create a projection from a raw pointer.
>> +///
> 
> I'd add a paragraph that explains that the pointer does not need to be
> valid in any way. It should also explain that the returned pointer is
> only valid when the original pointer was valid.
> 
>> +/// Supported projections include field projections and index projections.
>> +/// It is not allowed to project into types that implement custom `Deref` or `Index`.
>> +///
>> +/// The macro has basic syntax of `kernel::project_pointer!(ptr, projection)`, where `ptr` is an
>> +/// expression that evaluates to a raw pointer which serves as the base of projection. `projection`
>> +/// can be a projection expression of form `.field` (normally identifer, or numeral in case of
>> +/// tuple structs) or of form `[index]`.
>> +///
>> +/// If mutable pointer is needed, the macro input can be prefixed with `mut` keyword, i.e.
>> +/// `kernel::project_pointer!(mut ptr, projection)`. By default, a const pointer is created.
>> +///
>> +/// `project_pointer!` macro can perform both fallible indexing and build-time checked indexing.
>> +/// `[index]` form performs build-time bounds checking; if compiler fails to prove `[index]` is in
>> +/// bounds, compilation will fail. `[index]?` can be used to perform runtime bounds checking;
>> +/// `OutOfBound` error is raised via `?` if the index is out of bounds.
>> +///
>> +/// # Examples
>> +///
>> +/// Field projections are performed with `.field_name`:
>> +/// ```
>> +/// struct MyStruct { field: u32, }
>> +/// let ptr: *const MyStruct = core::ptr::dangling();
> 
> I would only include one example that uses `dangling` and for the rest
> just define a function that projects a raw pointer.
> 
>> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
>> +///
>> +/// struct MyTupleStruct(u32, u32);
>> +/// let ptr: *const MyTupleStruct = core::ptr::dangling();
>> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
>> +/// ```
>> +///
>> +/// Index projections are performed with `[index]`:
>> +/// ```
>> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
>> +/// let field_ptr: *const u8 = kernel::project_pointer!(ptr, [1]);
>> +/// // This will fail the build.
>> +/// // kernel::project_pointer!(ptr, [128]);
>> +/// // This will raise an `OutOfBound` error (which is convertable to `ERANGE`).
>> +/// // kernel::project_pointer!(ptr, [128]?);
>> +/// ```
>> +///
>> +/// If you need to match on the error instead of propagate, put the invocation inside a closure:
>> +/// ```
>> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
>> +/// let field_ptr: Result<*const u8> = (|| -> Result<_> {
>> +///     Ok(kernel::project_pointer!(ptr, [128]?))
>> +/// })();
>> +/// assert!(field_ptr.is_err());
>> +/// ```
>> +///
>> +/// For mutable pointers, put `mut` as the first token in macro invocation.
>> +/// ```
>> +/// let ptr: *mut [(u8, u16); 32] = core::ptr::dangling_mut();
>> +/// let field_ptr: *mut u16 = kernel::project_pointer!(mut ptr, [1].1);
>> +/// ```
>> +#[macro_export]
>> +macro_rules! project_pointer {
>> +    (@gen $ptr:ident, ) => {};
>> +    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
>> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
>> +        // SAFETY: the provided closure always return in bounds pointer.
>> +        let $ptr = unsafe {
>> +            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {
>> +                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
>> +                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
>> +                &raw mut (*ptr).$field
>> +            })
>> +        };
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    // Fallible index projection.
>> +    (@gen $ptr:ident, [$index:expr]? $($rest:tt)*) => {
>> +        let $ptr = $crate::projection::ProjectIndex::get($index, $ptr)
>> +            .ok_or($crate::projection::OutOfBound)?;
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    // Build-time checked index projection.
>> +    (@gen $ptr:ident, [$index:expr] $($rest:tt)*) => {
>> +        let $ptr = $crate::projection::ProjectIndex::index($index, $ptr);
>> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
>> +    };
>> +    (mut $ptr:expr, $($proj:tt)*) => {{
>> +        let ptr = $ptr;
> 
> I'd add a type ascription `let ptr: *mut _ = $ptr;`
> 
>> +        $crate::project_pointer!(@gen ptr, $($proj)*);
>> +        ptr
>> +    }};
>> +    ($ptr:expr, $($proj:tt)*) => {{
>> +        let ptr = $ptr.cast_mut();
> 
> This allows `$ptr` to be a random type with a `cast_mut` function. How
> about:
> 
>     let ptr: *const _ = $ptr;
>     let ptr: *mut _ = ::core::ptr::cast_mut(ptr);

I think `<*const _>::cast_mut($ptr)` probably would also do.

Thanks a lot for the review.

Best,
Gary

> 
> Cheers,
> Benno
> 
>> +        // We currently always project using mutable pointer, as it is not decided whether `&raw
>> +        // const` allows the resulting pointer to be mutated (see documentation of `addr_of!`).
>> +        $crate::project_pointer!(@gen ptr, $($proj)*);
>> +        ptr.cast_const()
>> +    }};
>> +}

  reply	other threads:[~2026-02-14 10:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214053344.1994776-1-gary@garyguo.net>
2026-02-14  5:33 ` [PATCH 1/4] rust: add projection infrastructure Gary Guo
2026-02-14  9:53   ` Benno Lossin
2026-02-14 10:36     ` Gary Guo [this message]
2026-02-14 14:48       ` Benno Lossin
2026-02-14 10:27   ` Danilo Krummrich
2026-02-22  0:57   ` Benno Lossin
2026-02-22 10:52     ` Gary Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=60bb58cecdef3ec4cda77cfad5c620ed@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox