Cheri-research/snmalloc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the remaining clang-tidy warnings.

Browse Source 
This introduces a new `address_t` type and two new casts: `pointer_cast`
and `address_cast` for casting between an `address_t` and a pointer.
These should make it easier to audit the codebase for casts between
pointers and integers.  In particular, the remaining `reinterpret_cast`s
and `pointer_cast`s should be the only places where we could perform
invalid pointer arithmetic.

Also adds a `pointer_offset` helper that adds an offset (in bytes) to a
pointer, preserving its original type.  This is a sufficiently common
pattern that it seemed worthwhile to centralise it.
This commit is contained in:
David Chisnall
2019-04-29 13:37:05 +01:00
parent 4bafca9be7
commit 28fac4d700
12 changed files with 128 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
src/ds/address.h Normal file
View File

@@ -0,0 +1,42 @@
#pragma once
#include <cstdint>
namespace snmalloc
{
/**
* The type used for an address. Currently, all addresses are assumed to be
* provenance-carrying values and so it is possible to cast back from the
* result of arithmetic on an address_t. Eventually, this will want to be
* separated into two types, one for raw addresses and one for addresses that
* can be cast back to pointers.
*/
typedef uintptr_t address_t;
/**
* Perform pointer arithmetic and return the adjusted pointer.
*/
template<typename T>
inline T* pointer_offset(T* base, size_t diff)
{
return reinterpret_cast<T*>(reinterpret_cast<char*>(base) + diff);
}
/**
* Cast from a pointer type to an address.
*/
template<typename T>
inline address_t address_cast(T* ptr)
{
return reinterpret_cast<address_t>(ptr);
}
/**
* Cast from an address back to a pointer of the specified type. All uses of
* this will eventually need auditing for CHERI compatibility.
*/
template<typename T>
inline T* pointer_cast(address_t address)
{
return reinterpret_cast<T*>(address);
}
}

11
src/ds/bits.h
View File

@@ -44,6 +44,8 @@
// #define USE_LZCNT
#include "address.h"
#include <atomic>
#include <cassert>
#include <cstdint>
@@ -310,7 +312,7 @@ namespace snmalloc
inline static size_t hash(void* p)
{
size_t x = (size_t)p;
size_t x = static_cast<size_t>(address_cast(p));
if (is64())
{
@@ -359,7 +361,8 @@ namespace snmalloc
{
assert(next_pow2(alignment) == alignment);
return (((size_t)p | size) & (alignment - 1)) == 0;
return ((static_cast<size_t>(address_cast(p)) | size) &
(alignment - 1)) == 0;
}
template<class T>
@@ -371,8 +374,8 @@ namespace snmalloc
using S = std::make_signed_t<T>;
constexpr S shift = (sizeof(S) * 8) - 1;
S a = (S)(v + 1);
S b = (S)(mod - a - 1);
S a = static_cast<S>(v + 1);
S b = static_cast<S>(mod - a - 1);
return a & ~(b >> shift);
}

2
src/ds/helpers.h
View File

@@ -50,7 +50,7 @@ namespace snmalloc
public:
operator T()
{
return (T)(value & (length - 1));
return static_cast<T>(value & (length - 1));
}
T& operator=(const T v)

54
src/mem/alloc.h
View File

@@ -133,7 +133,7 @@ namespace snmalloc
/**
* Get the pagemap entry corresponding to a specific address.
*/
uint8_t get(uintptr_t p)
uint8_t get(address_t p)
{
return PagemapProvider::pagemap().get(p);
}
@@ -143,7 +143,7 @@ namespace snmalloc
*/
uint8_t get(void* p)
{
return get((uintptr_t)p);
return get(address_cast(p));
}
/**
@@ -186,7 +186,7 @@ namespace snmalloc
size_t size_bits = bits::next_pow2_bits(size);
set(p, static_cast<uint8_t>(size_bits));
// Set redirect slide
uintptr_t ss = (uintptr_t)p + SUPERSLAB_SIZE;
auto ss = address_cast(p) + SUPERSLAB_SIZE;
for (size_t i = 0; i < size_bits - SUPERSLAB_BITS; i++)
{
size_t run = 1ULL << i;
@@ -195,7 +195,7 @@ namespace snmalloc
ss = ss + SUPERSLAB_SIZE * run;
}
PagemapProvider::pagemap().set(
(uintptr_t)p, static_cast<uint8_t>(size_bits));
address_cast(p), static_cast<uint8_t>(size_bits));
}
/**
* Update the pagemap to remove a large allocation, of `size` bytes from
@@ -203,7 +203,7 @@ namespace snmalloc
*/
void clear_large_size(void* vp, size_t size)
{
uintptr_t p = (uintptr_t)vp;
auto p = address_cast(vp);
size_t rounded_size = bits::next_pow2(size);
assert(get(p) == bits::next_pow2_bits(size));
auto count = rounded_size >> SUPERSLAB_BITS;
@@ -218,7 +218,7 @@ namespace snmalloc
*/
void set(void* p, uint8_t x)
{
PagemapProvider::pagemap().set((uintptr_t)p, x);
PagemapProvider::pagemap().set(address_cast(p), x);
}
};
@@ -427,7 +427,7 @@ namespace snmalloc
// Free memory of an unknown size. Must be called with an external
// pointer.
uint8_t size = pagemap().get((uintptr_t)p);
uint8_t size = pagemap().get(address_cast(p));
if (size == 0)
{
@@ -470,7 +470,7 @@ namespace snmalloc
}
# ifndef SNMALLOC_SAFE_CLIENT
if (size > 64 || (uintptr_t)super != (uintptr_t)p)
if (size > 64 || address_cast(super) != address_cast(p))
{
error("Not deallocating start of an object");
}
@@ -480,13 +480,13 @@ namespace snmalloc
}
template<Boundary location = Start>
static uintptr_t external_uintptr(void* p)
static address_t external_address(void* p)
{
#ifdef USE_MALLOC
error("Unsupported");
UNUSED(p);
#else
uint8_t size = global_pagemap.get((uintptr_t)p);
uint8_t size = global_pagemap.get(address_cast(p));
Superslab* super = Superslab::get(p);
if (size == PMSuperslab)
@@ -495,7 +495,7 @@ namespace snmalloc
Metaslab& meta = super->get_meta(slab);
uint8_t sc = meta.sizeclass;
size_t slab_end = (size_t)slab + SLAB_SIZE;
size_t slab_end = static_cast<size_t>(address_cast(slab) + SLAB_SIZE);
return external_pointer<location>(p, sc, slab_end);
}
@@ -504,12 +504,13 @@ namespace snmalloc
Mediumslab* slab = Mediumslab::get(p);
uint8_t sc = slab->get_sizeclass();
size_t slab_end = (size_t)slab + SUPERSLAB_SIZE;
size_t slab_end =
static_cast<size_t>(address_cast(slab) + SUPERSLAB_SIZE);
return external_pointer<location>(p, sc, slab_end);
}
uintptr_t ss = (uintptr_t)super;
auto ss = address_cast(super);
while (size > 64)
{
@@ -541,13 +542,13 @@ namespace snmalloc
template<Boundary location = Start>
static void* external_pointer(void* p)
{
return (void*)external_uintptr<location>(p);
return pointer_cast<void>(external_address<location>(p));
}
static size_t alloc_size(void* p)
{
// This must be called on an external pointer.
size_t size = global_pagemap.get((uintptr_t)p);
size_t size = global_pagemap.get(address_cast(p));
if (size == 0)
{
@@ -799,7 +800,8 @@ namespace snmalloc
size_t end_point_correction = location == End ?
(end_point - 1) :
(location == OnePastEnd ? end_point : (end_point - rsize));
size_t offset_from_end = (end_point - 1) - (size_t)p;
size_t offset_from_end =
(end_point - 1) - static_cast<size_t>(address_cast(p));
size_t end_to_end = round_by_sizeclass(rsize, offset_from_end);
return end_point_correction - end_to_end;
}
@@ -884,8 +886,9 @@ namespace snmalloc
if (super != nullptr)
return super;
super = (Superslab*)large_allocator.template alloc<NoZero, allow_reserve>(
0, SUPERSLAB_SIZE);
super = reinterpret_cast<Superslab*>(
large_allocator.template alloc<NoZero, allow_reserve>(
0, SUPERSLAB_SIZE));
if ((allow_reserve == NoReserve) && (super == nullptr))
return super;
@@ -1071,7 +1074,7 @@ namespace snmalloc
if constexpr (decommit_strategy == DecommitSuper)
{
large_allocator.memory_provider.notify_not_using(
(void*)((size_t)super + OS_PAGE_SIZE),
pointer_offset(super, OS_PAGE_SIZE),
SUPERSLAB_SIZE - OS_PAGE_SIZE);
}
else if constexpr (decommit_strategy == DecommitSuperLazy)
@@ -1118,9 +1121,9 @@ namespace snmalloc
}
else
{
slab =
(Mediumslab*)large_allocator.template alloc<NoZero, allow_reserve>(
0, SUPERSLAB_SIZE);
slab = reinterpret_cast<Mediumslab*>(
large_allocator.template alloc<NoZero, allow_reserve>(
0, SUPERSLAB_SIZE));
if ((allow_reserve == NoReserve) && (slab == nullptr))
return nullptr;
@@ -1146,7 +1149,7 @@ namespace snmalloc
#ifndef SNMALLOC_SAFE_CLIENT
if (!is_multiple_of_sizeclass(
sizeclass_to_size(sizeclass),
(uintptr_t)slab + SUPERSLAB_SIZE - (uintptr_t)p))
address_cast(slab) + SUPERSLAB_SIZE - address_cast(p)))
{
error("Not deallocating start of an object");
}
@@ -1164,8 +1167,7 @@ namespace snmalloc
if constexpr (decommit_strategy == DecommitSuper)
{
large_allocator.memory_provider.notify_not_using(
(void*)((size_t)slab + OS_PAGE_SIZE),
SUPERSLAB_SIZE - OS_PAGE_SIZE);
pointer_offset(slab, OS_PAGE_SIZE), SUPERSLAB_SIZE - OS_PAGE_SIZE);
}
pagemap().clear_slab(slab);
@@ -1219,7 +1221,7 @@ namespace snmalloc
if ((decommit_strategy != DecommitNone) || (large_class > 0))
large_allocator.memory_provider.notify_not_using(
(void*)((size_t)p + OS_PAGE_SIZE), rsize - OS_PAGE_SIZE);
pointer_offset(p, OS_PAGE_SIZE), rsize - OS_PAGE_SIZE);
// Initialise in order to set the correct SlabKind.
Largeslab* slab = static_cast<Largeslab*>(p);

5
src/mem/allocstats.h
View File

@@ -82,7 +82,8 @@ namespace snmalloc
if (slab_count.current != 0)
{
double occupancy = (double)count.current / (double)slab_count.current;
double occupancy = static_cast<double>(count.current) /
static_cast<double>(slab_count.current);
uint64_t duration = now - time;
if (ticks == 0)
@@ -103,7 +104,7 @@ namespace snmalloc
// Keep in sync with header lower down
count.print(csv, multiplier);
slab_count.print(csv, slab_multiplier);
size_t average = (size_t)(online_average * multiplier);
size_t average = static_cast<size_t>(online_average * multiplier);
csv << average << (slab_multiplier - average) * slab_count.max
<< csv.endl;

2
src/mem/globalalloc.h
View File

@@ -19,7 +19,7 @@ namespace snmalloc
sizeof(AllocPool) == sizeof(Parent),
"You cannot add fields to this class.");
// This cast is safe due to the static assert.
return (AllocPool*)Parent::make(mp);
return static_cast<AllocPool*>(Parent::make(mp));
}
static AllocPool* make() noexcept

40
src/mem/largealloc.h
View File

@@ -56,7 +56,7 @@ namespace snmalloc
class MemoryProviderStateMixin : public PAL
{
std::atomic_flag lock = ATOMIC_FLAG_INIT;
size_t bump;
address_t bump;
size_t remaining;
void new_block()
@@ -69,7 +69,7 @@ namespace snmalloc
PAL::template notify_using<NoZero>(r, OS_PAGE_SIZE);
bump = (size_t)r;
bump = address_cast(r);
remaining = size;
}
@@ -109,7 +109,8 @@ namespace snmalloc
// the stack.
if (slab->get_kind() != Decommitted)
{
PAL::notify_not_using(((char*)slab) + OS_PAGE_SIZE, decommit_size);
PAL::notify_not_using(
static_cast<char*>(slab) + OS_PAGE_SIZE, decommit_size);
}
// Once we've removed these from the stack, there will be no
// concurrent accesses and removal should have established a
@@ -159,16 +160,16 @@ namespace snmalloc
new_block();
}
p = (void*)bump;
p = pointer_cast<void>(bump);
bump += size;
remaining -= size;
}
auto page_start = bits::align_down((size_t)p, OS_PAGE_SIZE);
auto page_end = bits::align_up((size_t)p + size, OS_PAGE_SIZE);
auto page_start = bits::align_down(address_cast(p), OS_PAGE_SIZE);
auto page_end = bits::align_up(address_cast(p) + size, OS_PAGE_SIZE);
PAL::template notify_using<NoZero>(
(void*)page_start, page_end - page_start);
pointer_cast<void>(page_start), page_end - page_start);
return new (p) T(std::forward<Args...>(args)...);
}
@@ -218,17 +219,16 @@ namespace snmalloc
void* p = PAL::template reserve<committed>(&request);
*size = request;
uintptr_t p0 = (uintptr_t)p;
uintptr_t start = bits::align_up(p0, align);
auto p0 = address_cast(p);
auto start = bits::align_up(p0, align);
if (start > p0)
{
uintptr_t end = bits::align_down(p0 + request, align);
*size = end - start;
PAL::notify_not_using(p, start - p0);
PAL::notify_not_using(
reinterpret_cast<void*>(end), (p0 + request) - end);
p = reinterpret_cast<void*>(start);
PAL::notify_not_using(pointer_cast<void>(end), (p0 + request) - end);
p = pointer_cast<void>(start);
}
return p;
}
@@ -295,16 +295,16 @@ namespace snmalloc
template<AllowReserve allow_reserve>
bool reserve_memory(size_t need, size_t add)
{
if (((size_t)reserved_start + need) > (size_t)reserved_end)
if ((address_cast(reserved_start) + need) > address_cast(reserved_end))
{
if constexpr (allow_reserve == YesReserve)
{
stats.segment_create();
reserved_start =
memory_provider.template reserve<false>(&add, SUPERSLAB_SIZE);
reserved_end = (void*)((size_t)reserved_start + add);
reserved_start =
(void*)bits::align_up((size_t)reserved_start, SUPERSLAB_SIZE);
reserved_end = pointer_offset(reserved_start, add);
reserved_start = pointer_cast<void>(
bits::align_up(address_cast(reserved_start), SUPERSLAB_SIZE));
if (add < need)
return false;
@@ -341,8 +341,8 @@ namespace snmalloc
if (!reserve_memory<allow_reserve>(rsize, add))
return nullptr;
p = (void*)reserved_start;
reserved_start = (void*)((size_t)p + rsize);
p = reserved_start;
reserved_start = pointer_offset(p, rsize);
// All memory is zeroed since it comes from reserved space.
memory_provider.template notify_using<NoZero>(p, size);
@@ -362,7 +362,7 @@ namespace snmalloc
// Passing zero_mem ensures the PAL provides zeroed pages if
// required.
memory_provider.template notify_using<zero_mem>(
(void*)((size_t)p + OS_PAGE_SIZE),
pointer_offset(p, OS_PAGE_SIZE),
bits::align_up(size, OS_PAGE_SIZE) - OS_PAGE_SIZE);
}
else
@@ -382,7 +382,7 @@ namespace snmalloc
// Notify we are using the rest of the allocation.
// Passing zero_mem ensures the PAL provides zeroed pages if required.
memory_provider.template notify_using<zero_mem>(
(void*)((size_t)p + OS_PAGE_SIZE),
pointer_offset(p, OS_PAGE_SIZE),
bits::align_up(size, OS_PAGE_SIZE) - OS_PAGE_SIZE);
}
else

7
src/mem/mediumslab.h
View File

@@ -41,7 +41,7 @@ namespace snmalloc
static Mediumslab* get(void* p)
{
return (Mediumslab*)((size_t)p & SUPERSLAB_MASK);
return pointer_cast<Mediumslab>(address_cast(p) & SUPERSLAB_MASK);
}
void init(RemoteAllocator* alloc, uint8_t sc, size_t rsize)
@@ -81,7 +81,7 @@ namespace snmalloc
assert(!full());
uint16_t index = stack[head++];
void* p = (void*)((size_t)this + (static_cast<size_t>(index) << 8));
void* p = pointer_offset(this, (static_cast<size_t>(index) << 8));
free--;
assert(bits::is_aligned_block<OS_PAGE_SIZE>(p, OS_PAGE_SIZE));
@@ -125,7 +125,8 @@ namespace snmalloc
uint16_t pointer_to_index(void* p)
{
// Get the offset from the slab for a memory location.
return static_cast<uint16_t>(((size_t)p - (size_t)this) >> 8);
return static_cast<uint16_t>(
((address_cast(p) - address_cast(this))) >> 8);
}
};
}

6
src/mem/metaslab.h
View File

@@ -15,7 +15,7 @@ namespace snmalloc
Slab* get_slab()
{
return (Slab*)((size_t)this & SLAB_MASK);
return pointer_cast<Slab>(address_cast(this) & SLAB_MASK);
}
};
@@ -95,7 +95,7 @@ namespace snmalloc
SlabLink* get_link(Slab* slab)
{
return (SlabLink*)((size_t)slab + link);
return reinterpret_cast<SlabLink*>(pointer_offset(slab, link));
}
bool valid_head(bool is_short)
@@ -146,7 +146,7 @@ namespace snmalloc
if (curr == link)
break;
// Iterate bump/free list segment
curr = *(uint16_t*)((uintptr_t)slab + curr);
curr = *reinterpret_cast<uint16_t*>(pointer_offset(slab, curr));
}
// Check we terminated traversal on a correctly aligned block

12
src/mem/pagemap.h
View File

@@ -78,7 +78,7 @@ namespace snmalloc
(INDEX_LEVELS * BITS_PER_INDEX_LEVEL) + BITS_FOR_LEAF + GRANULARITY_BITS;
// Value used to represent when a node is being added too
static constexpr uintptr_t LOCKED_ENTRY = 1;
static constexpr address_t LOCKED_ENTRY = 1;
struct Leaf
{
@@ -112,14 +112,16 @@ namespace snmalloc
// to see that correctly.
PagemapEntry* value = e->load(std::memory_order_relaxed);
if ((uintptr_t)value <= LOCKED_ENTRY)
if (address_cast(value) <= LOCKED_ENTRY)
{
if constexpr (create_addr)
{
value = nullptr;
if (e->compare_exchange_strong(
value, (PagemapEntry*)LOCKED_ENTRY, std::memory_order_relaxed))
value,
pointer_cast<PagemapEntry>(LOCKED_ENTRY),
std::memory_order_relaxed))
{
auto& v = default_memory_provider;
value = v.alloc_chunk<PagemapEntry, OS_PAGE_SIZE>();
@@ -127,7 +129,7 @@ namespace snmalloc
}
else
{
while ((uintptr_t)e->load(std::memory_order_relaxed) ==
while (address_cast(e->load(std::memory_order_relaxed)) ==
LOCKED_ENTRY)
{
bits::pause();
@@ -178,7 +180,7 @@ namespace snmalloc
break;
}
Leaf* leaf = (Leaf*)get_node<create_addr>(e, result);
Leaf* leaf = reinterpret_cast<Leaf*>(get_node<create_addr>(e, result));
if (!result)
return std::pair(nullptr, 0);

12
src/mem/slab.h
View File

@@ -10,13 +10,13 @@ namespace snmalloc
uint16_t pointer_to_index(void* p)
{
// Get the offset from the slab for a memory location.
return static_cast<uint16_t>((size_t)p - (size_t)this);
return static_cast<uint16_t>(address_cast(p) - address_cast(this));
}
public:
static Slab* get(void* p)
{
return (Slab*)((size_t)p & SLAB_MASK);
return pointer_cast<Slab>(address_cast(p) & SLAB_MASK);
}
Metaslab& get_meta()
@@ -48,7 +48,7 @@ namespace snmalloc
if ((head & 1) == 0)
{
void* node = (void*)((size_t)this + head);
void* node = pointer_offset(this, head);
// Read the next slot from the memory that's about to be allocated.
uint16_t next = *static_cast<uint16_t*>(node);
@@ -59,7 +59,7 @@ namespace snmalloc
else
{
// This slab is being bump allocated.
p = (void*)((size_t)this + head - 1);
p = pointer_offset(this, head - 1);
meta.head = (head + static_cast<uint16_t>(rsize)) & (SLAB_SIZE - 1);
if (meta.head == 1)
{
@@ -89,7 +89,7 @@ namespace snmalloc
Metaslab& meta = super->get_meta(this);
return is_multiple_of_sizeclass(
sizeclass_to_size(meta.sizeclass),
(uintptr_t)this + SLAB_SIZE - (uintptr_t)p);
address_cast(this) + SLAB_SIZE - address_cast(p));
}
// Returns true, if it alters get_status.
@@ -160,7 +160,7 @@ namespace snmalloc
bool is_short()
{
return ((size_t)this & SUPERSLAB_MASK) == (size_t)this;
return (address_cast(this) & SUPERSLAB_MASK) == address_cast(this);
}
};
}

12
src/mem/superslab.h
View File

@@ -44,7 +44,7 @@ namespace snmalloc
// Used size_t as results in better code in MSVC
size_t slab_to_index(Slab* slab)
{
auto res = (((size_t)slab - (size_t)this) >> SLAB_BITS);
auto res = ((address_cast(slab) - address_cast(this)) >> SLAB_BITS);
assert(res == (uint8_t)res);
return res;
}
@@ -67,7 +67,7 @@ namespace snmalloc
static Superslab* get(void* p)
{
return (Superslab*)((size_t)p & SUPERSLAB_MASK);
return pointer_cast<Superslab>(address_cast(p) & SUPERSLAB_MASK);
}
static bool is_short_sizeclass(uint8_t sizeclass)
@@ -166,7 +166,7 @@ namespace snmalloc
if constexpr (decommit_strategy == DecommitAll)
{
memory_provider.template notify_using<NoZero>(
(void*)((size_t)this + OS_PAGE_SIZE), SLAB_SIZE - OS_PAGE_SIZE);
pointer_offset(this, OS_PAGE_SIZE), SLAB_SIZE - OS_PAGE_SIZE);
}
used++;
@@ -177,8 +177,8 @@ namespace snmalloc
Slab* alloc_slab(uint8_t sizeclass, MemoryProvider& memory_provider)
{
uint8_t h = head;
Slab* slab =
(Slab*)((size_t)this + (static_cast<size_t>(h) << SLAB_BITS));
Slab* slab = pointer_cast<Slab>(
address_cast(this) + (static_cast<size_t>(h) << SLAB_BITS));
uint8_t n = meta[h].next;
@@ -229,7 +229,7 @@ namespace snmalloc
if constexpr (decommit_strategy == DecommitAll)
{
memory_provider.notify_not_using(
(void*)((size_t)this + OS_PAGE_SIZE), SLAB_SIZE - OS_PAGE_SIZE);
pointer_offset(this, OS_PAGE_SIZE), SLAB_SIZE - OS_PAGE_SIZE);
}
bool was_full = is_full();