Skip to content

Commit 4702385

Browse files
committed
[libc] Fix malloc Block alignment issue on riscv32
Aligning blocks to max_align_t is neither necessary nor sufficient to ensure that the usable_space() is so aligned. Instead, we make this an invariant of Block and maintain it in init() and split(). This allows targets like riscv32 with small pointers and large max_align_t to maintain the property that all available blocks are aligned for malloc(). This change adjusts the tests to match and also updates them closer to llvm-libc style.
1 parent 0f4fcca commit 4702385

File tree

8 files changed

+297
-430
lines changed

8 files changed

+297
-430
lines changed

libc/fuzzing/__support/freelist_heap_fuzz.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,13 @@ optional<AllocType> choose<AllocType>(const uint8_t *&data, size_t &remainder) {
100100
*raw % static_cast<uint8_t>(AllocType::NUM_ALLOC_TYPES));
101101
}
102102

103-
constexpr size_t heap_size = 64 * 1024;
103+
constexpr size_t HEAP_SIZE = 64 * 1024;
104104

105105
optional<size_t> choose_size(const uint8_t *&data, size_t &remainder) {
106106
auto raw = choose<size_t>(data, remainder);
107107
if (!raw)
108108
return nullopt;
109-
return *raw % heap_size;
109+
return *raw % HEAP_SIZE;
110110
}
111111

112112
optional<size_t> choose_alloc_idx(const AllocVec &allocs, const uint8_t *&data,
@@ -126,7 +126,7 @@ optional<size_t> choose_alloc_idx(const AllocVec &allocs, const uint8_t *&data,
126126
TYPE NAME = *maybe_##NAME
127127

128128
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t remainder) {
129-
LIBC_NAMESPACE::FreeListHeapBuffer<heap_size> heap;
129+
LIBC_NAMESPACE::FreeListHeapBuffer<HEAP_SIZE> heap;
130130
AllocVec allocs(heap);
131131

132132
uint8_t canary = 0;

libc/src/__support/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_header_library(
1212
libc.src.__support.CPP.optional
1313
libc.src.__support.CPP.span
1414
libc.src.__support.CPP.type_traits
15+
libc.src.__support.math_extras
1516
)
1617

1718
add_object_library(

libc/src/__support/block.h

Lines changed: 111 additions & 132 deletions
Large diffs are not rendered by default.

libc/src/__support/freelist_heap.h

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,28 +89,14 @@ LIBC_INLINE void *FreeListHeap::allocate_impl(size_t alignment, size_t size) {
8989
if (!is_initialized)
9090
init();
9191

92-
size_t request_size = size;
93-
94-
// TODO: usable_space should always be aligned to max_align_t.
95-
if (alignment > alignof(max_align_t) ||
96-
(Block::BLOCK_OVERHEAD % alignof(max_align_t) != 0)) {
97-
// TODO: This bound isn't precisely calculated yet. It assumes one extra
98-
// Block::ALIGNMENT to accomodate the possibility for padding block
99-
// overhead. (alignment - 1) ensures that there is an aligned point
100-
// somewhere in usable_space, but this isn't tight either, since
101-
// usable_space is also already somewhat aligned.
102-
if (add_overflow(size, (alignment - 1) + Block::ALIGNMENT, request_size))
103-
return nullptr;
104-
}
92+
size_t request_size = Block::min_size_for_allocation(alignment, size);
93+
if (!request_size)
94+
return nullptr;
10595

10696
Block *block = free_store.remove_best_fit(request_size);
10797
if (!block)
10898
return nullptr;
10999

110-
LIBC_ASSERT(block->can_allocate(alignment, size) &&
111-
"block should always be large enough to allocate at the correct "
112-
"alignment");
113-
114100
auto block_info = Block::allocate(block, alignment, size);
115101
if (block_info.next)
116102
free_store.insert(block_info.next);
@@ -135,6 +121,9 @@ LIBC_INLINE void *FreeListHeap::aligned_allocate(size_t alignment,
135121
if (size % alignment != 0)
136122
return nullptr;
137123

124+
// The minimum alignment supported by Block is max_align_t.
125+
alignment = cpp::max(alignment, alignof(max_align_t));
126+
138127
return allocate_impl(alignment, size);
139128
}
140129

libc/src/__support/freestore.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,12 @@ class FreeStore {
4040
Block *remove_best_fit(size_t size);
4141

4242
private:
43-
static constexpr size_t ALIGNMENT = alignof(max_align_t);
4443
static constexpr size_t MIN_OUTER_SIZE =
45-
align_up(Block::BLOCK_OVERHEAD + sizeof(FreeList::Node), ALIGNMENT);
44+
align_up(sizeof(Block) + sizeof(FreeList::Node), alignof(max_align_t));
4645
static constexpr size_t MIN_LARGE_OUTER_SIZE =
47-
align_up(Block::BLOCK_OVERHEAD + sizeof(FreeTrie::Node), ALIGNMENT);
46+
align_up(sizeof(Block) + sizeof(FreeTrie::Node), alignof(max_align_t));
4847
static constexpr size_t NUM_SMALL_SIZES =
49-
(MIN_LARGE_OUTER_SIZE - MIN_OUTER_SIZE) / ALIGNMENT;
48+
(MIN_LARGE_OUTER_SIZE - MIN_OUTER_SIZE) / alignof(max_align_t);
5049

5150
LIBC_INLINE static bool too_small(Block *block) {
5251
return block->outer_size() < MIN_OUTER_SIZE;
@@ -99,7 +98,8 @@ LIBC_INLINE Block *FreeStore::remove_best_fit(size_t size) {
9998

10099
LIBC_INLINE FreeList &FreeStore::small_list(Block *block) {
101100
LIBC_ASSERT(is_small(block) && "only legal for small blocks");
102-
return small_lists[(block->outer_size() - MIN_OUTER_SIZE) / ALIGNMENT];
101+
return small_lists[(block->outer_size() - MIN_OUTER_SIZE) /
102+
alignof(max_align_t)];
103103
}
104104

105105
LIBC_INLINE FreeList *FreeStore::find_best_small_fit(size_t size) {

0 commit comments

Comments
 (0)