On 6 Jan 2022, at 22:27, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Taylor Blau <me@xxxxxxxxxxxx> writes: > >> (+cc René as another possible reviewer) >> >> On Wed, Jan 05, 2022 at 01:23:24PM +0000, Jessica Clarke wrote: >>> Currently git_qsort_s allocates a buffer on the stack that has no >>> alignment, and mem_pool_alloc assumes uintmax_t's size is adequate >>> alignment for any type. >>> >>> On CHERI, and thus Arm's Morello prototype, pointers are implemented as >>> hardware capabilities which, as well as having a normal integer address, >>> have additional bounds, permissions and other metadata in a second word, >>> so on a 64-bit architecture they are 128-bit quantities, including their >>> alignment requirements. Despite being 128-bit, their integer component >>> is still only a 64-bit field, so uintmax_t remains 64-bit, and therefore >>> uintmax_t does not sufficiently align an allocation. > > Alignment aside, if uintmax_t is 64-bit but your pointer needs > 128-bit to store, saving a pointer value in uintmax_t variable would > not work correctly, I presume, as casting the 64-bit integral type > back into pointer would not be sufficient to recover the lost > information that used to be in the second word. > > So, does the architecture have 128-bit uintptr_t and that is a safe > type both from the point of view of alignment and from the point of > view of not losing information? Yes. It is basically just a char * that lets you perform arbitrary arithmetic; not an unsigned long any more for our architectures. > If that type is larger than uintmax_t, something smells wrong, > though. max is not max anymore at that point. > > IIRC, uintptr_t is optional in C99, so a simpler solution to use the > larger type between uintptr_t and uintmax_t as a replacement for how > we use uintmax_t would not quite work out of the box X-<. This is also true of uint128_t, it doesn’t fit in a uintmax_t either. uintmax_t was a mistake as it becomes part of the ABI and can never be revised even when new integer types come along. uintmax_t can hold any valid address, but will strip the metadata. It turns out almost no software tries to put a pointer in a uintmax_t and cast it back to a pointer. Note that, even if we wanted to, we coldn't just map uintmax_t to a uintptr_t, as on 32-bit architectures uintmax_t is a 64-bit integer but uintptr_t only has a 32-bit range (plus 32 bits of metadata), and we do have a 32-bit CHERI variant for embedded use cases. This is one of a few warts that people will just have to deal with for CHERI; there’s no way round it if you want anything that implements the ideas of CHERI. Jess
