On 11.09.24 16:50, Danilo Krummrich wrote: > On Wed, Sep 11, 2024 at 03:27:57PM +0200, Alice Ryhl wrote: >> On Wed, Sep 11, 2024 at 3:26 PM Benno Lossin <benno.lossin@xxxxxxxxx> wrote: >>> On 11.09.24 13:02, Danilo Krummrich wrote: >>>> On Wed, Sep 11, 2024 at 08:36:38AM +0000, Benno Lossin wrote: >>>>> On 11.09.24 01:25, Danilo Krummrich wrote: >>>>>> On Tue, Sep 10, 2024 at 07:49:42PM +0000, Benno Lossin wrote: >>>>>>> On 10.09.24 19:40, Danilo Krummrich wrote: >>>>>>>> On Sat, Aug 31, 2024 at 05:39:07AM +0000, Benno Lossin wrote: >>>>>>>>> On 16.08.24 02:10, Danilo Krummrich wrote: >>>>>>>>>> +/// >>>>>>>>>> +/// ``` >>>>>>>>>> +/// # use kernel::bindings; >>>>>>>>>> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1; >>>>>>>>>> +/// struct Huge([u8; SIZE]); >>>>>>>>>> +/// >>>>>>>>>> +/// assert!(KVBox::<Huge>::new_uninit(GFP_KERNEL).is_ok()); >>>>>>>>>> +/// ``` >>>>>>>>> >>>>>>>>> Similarly, you could then say above this one "Instead use either `VBox` >>>>>>>>> or `KVBox`:" >>>>>>>>> >>>>>>>>>> +/// >>>>>>>>>> +/// # Invariants >>>>>>>>>> +/// >>>>>>>>>> +/// The [`Box`]' pointer is always properly aligned and either points to memory allocated with `A` >>>>>>>>> >>>>>>>>> Please use `self.0` instead of "[`Box`]'". >>>>>>>>> >>>>>>>>>> +/// or, for zero-sized types, is a dangling pointer. >>>>>>>>> >>>>>>>>> Probably "dangling, well aligned pointer.". >>>>>>>> >>>>>>>> Does this add any value? For ZSTs everything is "well aligned", isn't it? >>>>>>> >>>>>>> ZSTs can have alignment and then unaligned pointers do exist for them >>>>>>> (and dereferencing them is UB!): >>>>>> >>>>>> Where is this documented? The documentation says: >>>>>> >>>>>> "For operations of size zero, *every* pointer is valid, including the null >>>>>> pointer. The following points are only concerned with non-zero-sized accesses." >>>>>> [1] >>>>> >>>>> That's a good point, the documentation looks a bit outdated. I found >>>>> this page in the nomicon: https://doc.rust-lang.org/nomicon/vec/vec-zsts.html >>>>> The first iterator implementation has an alignment issue. (Nevertheless, >>>>> that chapter of the nomicon is probably useful to you, since it goes >>>>> over implementing `Vec`, but maybe you already saw it) >>>>> >>>>>> [1] https://doc.rust-lang.org/std/ptr/index.html >>>>> >>>>> Might be a good idea to improve/complain about this at the rust project. >>>> >>>> Well, my point is how do we know? There's no language specification and the >>>> documentation is (at least) ambiguous. >>> >>> So I went through the unsafe-coding-guidelines issues list and only >>> found this one: https://github.com/rust-lang/unsafe-code-guidelines/issues/93 >>> Maybe I missed something. You could also try to ask at the rust zulip in >>> the t-opsem channel for further clarification. >>> >>> I think we should just be on the safe side and assume that ZSTs require >>> alignment. But if you get a convincing answer and if they say that they >>> will document it, then I don't mind removing the alignment requirement. > > I agree -- I also wrote this in a previous mail. > > I was just wondering why you are so sure about it, since the documentation > doesn't seem to be clear about it. As Alice found below, the documentation is actually clear about this. (I think I read it at some point, but forgot exactly where it was) Maybe it could be better documented that dereferencing has the same requirements as `read` (or whatever they are). >> Please see the section on alignment in the same page. Just because a >> pointer is valid does not mean that it is properly aligned. >> >> From the page: >> >> Valid raw pointers as defined above are not necessarily properly >> aligned (where “proper” alignment is defined by the pointee type, >> i.e., *const T must be aligned to mem::align_of::<T>()). However, most >> functions require their arguments to be properly aligned, and will >> explicitly state this requirement in their documentation. Notable >> exceptions to this are read_unaligned and write_unaligned. >> >> When a function requires proper alignment, it does so even if the >> access has size 0, i.e., even if memory is not actually touched. >> Consider using NonNull::dangling in such cases. > > Good point. > > It still sounds like it's only required for functions that explicitly state so. > > And as cited from nomicon "This is possibly needless pedantry because ptr::read > is a noop for a ZST, [...]". But, no question, of course we have to honor it > anyways. This sounds to me like an implementation detail note, not something that a caller should consider. But that's my interpretation. --- Cheers, Benno
