Re: [PATCH v4 09/28] rust: alloc: implement kernel `Box`

[Danilo Krummrich <dakr@kernel.org>]

On Wed, Aug 07, 2024 at 08:01:00AM +0000, Benno Lossin wrote:
> On 07.08.24 09:51, Alice Ryhl wrote:
> > On Wed, Aug 7, 2024 at 9:49 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >> On 07.08.24 01:01, Danilo Krummrich wrote:
> >>> On Tue, Aug 06, 2024 at 07:47:17PM +0000, Benno Lossin wrote:
> >>>> On 05.08.24 17:19, Danilo Krummrich wrote:
> >>>>> +impl<T, A> Box<T, A>
> >>>>> +where
> >>>>> +    T: ?Sized,
> >>>>> +    A: Allocator,
> >>>>> +{
> >>>>> +    /// Constructs a `Box<T, A>` from a raw pointer.
> >>>>> +    ///
> >>>>> +    /// # Safety
> >>>>> +    ///
> >>>>> +    /// `raw` must point to valid memory, previously allocated with `A`, and at least the size of
> >>>>> +    /// type `T`.
> >>>>
> >>>> With this requirement and the invariant on `Box`, I am lead to believe
> >>>> that you can't use this for ZSTs, since they are not allocated with `A`.
> >>>> One solution would be to adjust this requirement. But I would rather use
> >>>> a different solution: we move the dangling pointer stuff into the
> >>>> allocator and also call it when `T` is a ZST (ie don't special case them
> >>>> in `Box` but in the impls of `Allocator`). That way this can stay as-is
> >>>> and the part about ZSTs in the invariant can be removed.
> >>>
> >>> Actually, we already got that. Every zero sized allocation will return a
> >>> dangling pointer. However, we can't call `Allocator::free` with (any) dangling
> >>> pointer though.
> >>
> >> The last part is rather problematic in my opinion, since the safety
> >> requirements of the functions in `Allocator` don't ensure that you're
> >> not allowed to do it. We should make it possible to free dangling
> >> pointers that were previously "allocated" by the allocator (ie returned
> >> by `realloc`).
> >> Maybe we do need an `old_layout` parameter for that (that way we can
> >> also `debug_assert_eq!(old_layout.align(), new_layout.align())`).
> > 
> > The std allocators generally prohibit zero sized allocations, so it
> > seems sensible for us to do the same?
> 
> I never understood why they do that, the stdlib `Allocator` trait has
> all the information it needs to detect zero-sized allocations, so it
> could just return dangling pointers. I don't see the point of
> duplicating the zero-sized logic in `Box` and `Vec`...

I think it's simpler and less error prone.

Besides that, I think the stdlib `Box` allows to call `from_raw` with any random
pointer for ZST, which the allocator API can't catch.

> 
> ---
> Cheers,
> Benno
>