diff --git a/src/quick-lint-js/container/flexible-array.h b/src/quick-lint-js/container/flexible-array.h index 94bb0f1838..f1dcb5812e 100644 --- a/src/quick-lint-js/container/flexible-array.h +++ b/src/quick-lint-js/container/flexible-array.h @@ -30,6 +30,12 @@ class alignas(maximum(alignof(T), alignof(Flexible_Array_Base
))) Flexible_Array : public Flexible_Array_Base
{ public: + // The guaranteed alignment for the capacity. + // + // Invariant: capacity_alignment >= alignof(T) + static inline constexpr std::size_t capacity_alignment = + alignof(Flexible_Array); + T* flexible_capacity_begin() { return reinterpret_cast(&this[1]); } T* flexible_capacity_end() { diff --git a/src/quick-lint-js/container/linked-bump-allocator.h b/src/quick-lint-js/container/linked-bump-allocator.h index c649c95e38..e5fc831fc7 100644 --- a/src/quick-lint-js/container/linked-bump-allocator.h +++ b/src/quick-lint-js/container/linked-bump-allocator.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -32,9 +33,10 @@ namespace quick_lint_js { // * no per-object free()/delete // * bulk free() via rewind() (doesn't call destructors) // * in-place growing of allocations -// * all allocations have the same alignment // // Internally, Linked_Bump_Allocator maintains a linked list of chunks. +// +// TODO(strager): Remove alignment parameter. template class Linked_Bump_Allocator final : public Memory_Resource { private: @@ -133,11 +135,8 @@ class Linked_Bump_Allocator final : public Memory_Resource { template T* new_object(Args&&... args) { - static_assert(alignof(T) <= alignment, - "T is not allowed by this allocator; this allocator's " - "alignment is insufficient for T"); - constexpr std::size_t byte_size = align_up(sizeof(T)); - return new (this->allocate_bytes(byte_size)) T(std::forward(args)...); + return new (this->allocate_bytes(sizeof(T), alignof(T))) + T(std::forward(args)...); } template @@ -156,11 +155,9 @@ class Linked_Bump_Allocator final : public Memory_Resource { template [[nodiscard]] Span allocate_uninitialized_span(std::size_t size) { - static_assert(alignof(T) <= alignment, - "T is not allowed by this allocator; this allocator's " - "alignment is insufficient for T"); - std::size_t byte_size = this->align_up(size * sizeof(T)); - T* items = reinterpret_cast(this->allocate_bytes(byte_size)); + std::size_t byte_size = size * sizeof(T); + T* items = + reinterpret_cast(this->allocate_bytes(byte_size, alignof(T))); return Span(items, narrow_cast(size)); } @@ -185,7 +182,7 @@ class Linked_Bump_Allocator final : public Memory_Resource { std::size_t new_size) { this->assert_not_disabled(); QLJS_ASSERT(new_size > old_size); - std::size_t old_byte_size = this->align_up(old_size * sizeof(T)); + std::size_t old_byte_size = old_size * sizeof(T); bool array_is_last_allocation = reinterpret_cast(array) + old_byte_size == this->next_allocation_; @@ -194,7 +191,7 @@ class Linked_Bump_Allocator final : public Memory_Resource { return false; } - std::size_t extra_bytes = this->align_up((new_size - old_size) * sizeof(T)); + std::size_t extra_bytes = (new_size - old_size) * sizeof(T); if (extra_bytes > this->remaining_bytes_in_current_chunk()) { return false; } @@ -235,17 +232,16 @@ class Linked_Bump_Allocator final : public Memory_Resource { protected: void* do_allocate(std::size_t bytes, std::size_t align) override { - QLJS_ASSERT(align <= alignment); - return this->allocate_bytes(this->align_up(bytes)); + return this->allocate_bytes(bytes, align); } - void do_deallocate(void* p, std::size_t bytes, std::size_t align) override { - QLJS_ASSERT(align <= alignment); + void do_deallocate(void* p, std::size_t bytes, + [[maybe_unused]] std::size_t align) override { this->deallocate_bytes(p, bytes); } private: - struct alignas(maximum(alignment, alignof(void*))) Chunk_Header { + struct alignas(alignof(void*)) Chunk_Header { explicit Chunk_Header(Chunk* previous) : previous(previous) {} Chunk* previous; // Linked list. @@ -253,19 +249,30 @@ class Linked_Bump_Allocator final : public Memory_Resource { static inline constexpr std::size_t default_chunk_size = 4096 - sizeof(Chunk); - static constexpr std::size_t align_up(std::size_t size) { - return (size + alignment - 1) & ~(alignment - 1); - } - - [[nodiscard]] void* allocate_bytes(std::size_t size) { + [[nodiscard]] void* allocate_bytes(std::size_t size, std::size_t align) { this->assert_not_disabled(); - QLJS_SLOW_ASSERT(size % alignment == 0); - if (this->remaining_bytes_in_current_chunk() < size) { - this->append_chunk(maximum(size, this->default_chunk_size)); - QLJS_ASSERT(this->remaining_bytes_in_current_chunk() >= size); - } - void* result = this->next_allocation_; - this->next_allocation_ += size; + QLJS_SLOW_ASSERT(size % align == 0); + + std::uintptr_t next_allocation_int = + reinterpret_cast(this->next_allocation_); + // NOTE(strager): align_up might overflow. If it does, the allocation + // couldn't fit due to alignment alone + // (alignment_padding > this->remaining_bytes_in_current_chunk()). + std::uintptr_t result_int = align_up(next_allocation_int, align); + char* result = reinterpret_cast(result_int); + + // alignment_padding is how much the pointer moved due to alignment, i.e. + // how much padding is necessary due to alignment. + std::uintptr_t alignment_padding = result_int - next_allocation_int; + bool have_enough_space = + (alignment_padding + size) <= this->remaining_bytes_in_current_chunk(); + if (!have_enough_space) [[unlikely]] { + this->append_chunk(maximum(size, this->default_chunk_size), align); + result = this->next_allocation_; + QLJS_ASSERT(is_aligned(result, align)); + } + + this->next_allocation_ = result + size; this->did_allocate_bytes(result, size); return result; } @@ -291,11 +298,25 @@ class Linked_Bump_Allocator final : public Memory_Resource { // TODO(strager): Mark memory as unusable for Valgrind. } - void append_chunk(std::size_t size) { - this->chunk_ = Chunk::allocate_and_construct_header(new_delete_resource(), - size, this->chunk_); - this->next_allocation_ = this->chunk_->flexible_capacity_begin(); + void append_chunk(std::size_t size, std::size_t align) { + // Over-alignment is tested by + // NOTE[Linked_Bump_Allocator-append_chunk-alignment]. + bool is_over_aligned = align > Chunk::capacity_alignment; + std::size_t allocated_size = size; + if (is_over_aligned) { + allocated_size += align; + } + this->chunk_ = Chunk::allocate_and_construct_header( + new_delete_resource(), allocated_size, this->chunk_); + std::uintptr_t next_allocation_int = reinterpret_cast( + this->chunk_->flexible_capacity_begin()); + if (is_over_aligned) { + next_allocation_int = align_up(next_allocation_int, align); + } + this->next_allocation_ = reinterpret_cast(next_allocation_int); this->chunk_end_ = this->chunk_->flexible_capacity_end(); + QLJS_ASSERT(is_aligned(this->next_allocation_, align)); + QLJS_ASSERT(this->remaining_bytes_in_current_chunk() >= size); } void assert_not_disabled() const { diff --git a/src/quick-lint-js/util/pointer.h b/src/quick-lint-js/util/pointer.h index 127981995b..824faab783 100644 --- a/src/quick-lint-js/util/pointer.h +++ b/src/quick-lint-js/util/pointer.h @@ -12,8 +12,10 @@ inline bool is_aligned(void* p, std::size_t alignment) { return (reinterpret_cast(p) & alignment_mask) == 0; } +// Does not check for pointer overflow. template -Integer_Pointer align_up(Integer_Pointer p, std::size_t alignment) { +[[nodiscard]] Integer_Pointer align_up(Integer_Pointer p, + std::size_t alignment) { Integer_Pointer alignment_mask = static_cast(alignment - 1); // TODO(strager): What about integer overflow? return ((p - 1) | alignment_mask) + 1; diff --git a/test/test-linked-bump-allocator.cpp b/test/test-linked-bump-allocator.cpp index f08c7ec5ff..c2003808b3 100644 --- a/test/test-linked-bump-allocator.cpp +++ b/test/test-linked-bump-allocator.cpp @@ -81,7 +81,7 @@ TEST(Test_Linked_Bump_Allocator, TEST(Test_Linked_Bump_Allocator, less_aligned_pre_grown_and_grown_array_keeps_next_allocation_aligned) { - Linked_Bump_Allocator<4> alloc("test"); + Linked_Bump_Allocator<1> alloc("test"); Span chars = alloc.allocate_uninitialized_span(3); bool grew = alloc.try_grow_array_in_place(chars.data(), 3, 6); @@ -136,6 +136,26 @@ TEST(Test_Linked_Bump_Allocator, filling_first_chunk_allocates_second_chunk) { assert_valid_memory(new_chunk_object); } +TEST(Test_Linked_Bump_Allocator, + allocate_new_chunk_with_over_alignment_aligns) { + constexpr std::size_t chunk_size = + 4096 - sizeof(void*) * 2; // Implementation detail. + + Linked_Bump_Allocator alloc("test"); + [[maybe_unused]] Span padding = alloc.allocate_span(chunk_size); + ASSERT_EQ(alloc.remaining_bytes_in_current_chunk(), 0) + << "First chunk should be consumed entirely"; + + // NOTE[Linked_Bump_Allocator-append_chunk-alignment]: This should allocate a + // brand new chunk, thus exercising the alignment code in + // Linked_Bump_Allocator::append_chunk. + struct alignas(128) Over_Aligned_Struct { + char data[chunk_size]; + }; + Over_Aligned_Struct* s = alloc.new_object(); + assert_valid_memory(s); +} + TEST(Test_Linked_Bump_Allocator, rewinding_within_chunk_reuses_memory) { Linked_Bump_Allocator<1> alloc("test");