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

Benno Lossin
2024-08-07T08:01:53Z

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`...

---
Cheers,
Benno