Skip to content

Commit 823caa8

Browse files
committed
Increase portability by masking rather than dividing and multiplying pointers
If (u)intptr_t exist, the only guarantee provided by the C standard is that you can cast a pointer to one and back. No claims are made about what that injective pointer-to-integer mapping is, nor the surjective reverse integer-to-pointer mapping; for example, nowhere is it specified that, given a char *p, p + 1 == (char *)((uintptr_t)p + 1), although no sensible implementation would make that not the case (provided p is a valid strictly in bounds pointer). With real pointers, taking them out of bounds (other than the one exception that is a one-past-the-end pointer) of their corresponding allocation is UB. Whilst the semantics of arithmetic on uintptr_t is not specified when cast back to a pointer, compilers already assume that uintptr_t arithmetic does not go out of bounds (or, put another way, that the result always ends up pointing to the same allocation) and their alias analysis-based optimisations already assume this. CHERI, and Arm's Morello, implement C language pointers with hardware capabilities, which are unforgeable pointers with bounds and permissions. In order to only double rather than quadruple the size of pointers, CHERI exploits C's requirement that pointers not be taken out of bounds, and compresses these bounds using a floating-point-like representation. The important implication of this for most software is that if a capability, i.e. C pointer in CHERI C, is taken too far out of bounds (where "too far" is proportional to the size of the allocation) it can no longer be represented and thus is invalidated, meaning it will trap on a later dereference. This also extends to (u)intptr_t, which are also implemented as capabilities, since if it were just a plain integer then casting a pointer to (u)intptr_t would lose the metadata and break the ability to cast back to a pointer. Whilst the composition of dividing a pointer and then multiplying it again has a somewhat sensible interpretation of rounding it down (even though technically no such guarantees are made by the spec), the individual operations do not, and the division is highly likely to take the pointer far out of bounds of its allocation and, on CHERI, result in it being unrepresentable and thus invalidated. Instead, we can perform a more standard bitwise AND with a mask to clear the low bits, exploiting the fact that alignments are powers of two; note that technically the rounding up variant of this does take the pointer out of bounds slightly and, whilst there are other ways to write such code that avoid doing so, they are more cumbersome, and this code does not need to worry about that.
1 parent 9c5654e commit 823caa8

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

include/msgpack/zone.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static inline void* msgpack_zone_malloc(msgpack_zone* zone, size_t size)
109109
(char*)(
110110
(uintptr_t)(
111111
zone->chunk_list.ptr + (MSGPACK_ZONE_ALIGN - 1)
112-
) / MSGPACK_ZONE_ALIGN * MSGPACK_ZONE_ALIGN
112+
) & ~(uintptr_t)(MSGPACK_ZONE_ALIGN - 1)
113113
);
114114
size_t adjusted_size = size + (size_t)(aligned - zone->chunk_list.ptr);
115115
if(zone->chunk_list.free >= adjusted_size) {
@@ -120,7 +120,7 @@ static inline void* msgpack_zone_malloc(msgpack_zone* zone, size_t size)
120120
{
121121
void* ptr = msgpack_zone_malloc_expand(zone, size + (MSGPACK_ZONE_ALIGN - 1));
122122
if (ptr) {
123-
return (char*)((uintptr_t)(ptr) / MSGPACK_ZONE_ALIGN * MSGPACK_ZONE_ALIGN);
123+
return (char*)((uintptr_t)(ptr) & ~(uintptr_t)(MSGPACK_ZONE_ALIGN - 1));
124124
}
125125
}
126126
return NULL;

0 commit comments

Comments
 (0)