diff --git a/filament/backend/include/backend/Handle.h b/filament/backend/include/backend/Handle.h index 7b8846ba7b..ffc16133fd 100644 --- a/filament/backend/include/backend/Handle.h +++ b/filament/backend/include/backend/Handle.h @@ -62,14 +62,6 @@ public: // clear the handle, this doesn't free associated resources void clear() noexcept { object = nullid; } - // compare handles - bool operator==(const HandleBase& rhs) const noexcept { return object == rhs.object; } - bool operator!=(const HandleBase& rhs) const noexcept { return object != rhs.object; } - bool operator<(const HandleBase& rhs) const noexcept { return object < rhs.object; } - bool operator<=(const HandleBase& rhs) const noexcept { return object <= rhs.object; } - bool operator>(const HandleBase& rhs) const noexcept { return object > rhs.object; } - bool operator>=(const HandleBase& rhs) const noexcept { return object >= rhs.object; } - // get this handle's handleId HandleId getId() const noexcept { return object; } @@ -101,6 +93,14 @@ struct Handle : public HandleBase { explicit Handle(HandleId id) noexcept : HandleBase(id) { } + // compare handles of the same type + bool operator==(const Handle& rhs) const noexcept { return getId() == rhs.getId(); } + bool operator!=(const Handle& rhs) const noexcept { return getId() != rhs.getId(); } + bool operator<(const Handle& rhs) const noexcept { return getId() < rhs.getId(); } + bool operator<=(const Handle& rhs) const noexcept { return getId() <= rhs.getId(); } + bool operator>(const Handle& rhs) const noexcept { return getId() > rhs.getId(); } + bool operator>=(const Handle& rhs) const noexcept { return getId() >= rhs.getId(); } + // type-safe Handle cast template::value> > Handle(Handle const& base) noexcept : HandleBase(base) { } // NOLINT(hicpp-explicit-conversions,google-explicit-constructor) diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h index 2e7c8d1530..04e66d8577 100644 --- a/filament/backend/include/private/backend/HandleAllocator.h +++ b/filament/backend/include/private/backend/HandleAllocator.h @@ -24,22 +24,19 @@ #include #include #include +#include #include +#include #include #include #include +#include #include #include -#if !defined(NDEBUG) && UTILS_HAS_RTTI -# define HANDLE_TYPE_SAFETY 1 -#else -# define HANDLE_TYPE_SAFETY 0 -#endif - #define HandleAllocatorGL HandleAllocator<16, 64, 208> // ~3640 / pool / MiB #define HandleAllocatorVK HandleAllocator<80, 176, 320> // ~1820 / pool / MiB #define HandleAllocatorMTL HandleAllocator<48, 160, 592> // ~1310 / pool / MiB @@ -52,11 +49,6 @@ namespace filament::backend { template class HandleAllocator { public: - - static_assert(P0 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); - static_assert(P1 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); - static_assert(P2 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); - HandleAllocator(const char* name, size_t size) noexcept; HandleAllocator(HandleAllocator const& rhs) = delete; HandleAllocator& operator=(HandleAllocator const& rhs) = delete; @@ -74,14 +66,9 @@ public: */ template Handle allocateAndConstruct(ARGS&& ... args) noexcept { - Handle h{ allocateHandle() }; + Handle h{ allocateHandle() }; D* addr = handle_cast(h); new(addr) D(std::forward(args)...); -#if HANDLE_TYPE_SAFETY - mLock.lock(); - mHandleTypeId[addr] = typeid(D).name(); - mLock.unlock(); -#endif return h; } @@ -97,13 +84,7 @@ public: */ template Handle allocate() noexcept { - Handle h{ allocateHandle() }; -#if HANDLE_TYPE_SAFETY - D* addr = handle_cast(h); - mLock.lock(); - mHandleTypeId[addr] = typeid(D).name(); - mLock.unlock(); -#endif + Handle h{ allocateHandle() }; return h; } @@ -120,17 +101,10 @@ public: assert_invariant(handle); D* addr = handle_cast(const_cast&>(handle)); assert_invariant(addr); - // currently we implement construct<> with dtor+ctor, we could use operator= also // but all our dtors are trivial, ~D() is actually a noop. addr->~D(); new(addr) D(std::forward(args)...); - -#if HANDLE_TYPE_SAFETY - mLock.lock(); - mHandleTypeId[addr] = typeid(D).name(); - mLock.unlock(); -#endif return addr; } @@ -147,12 +121,6 @@ public: D* addr = handle_cast(const_cast&>(handle)); assert_invariant(addr); new(addr) D(std::forward(args)...); - -#if HANDLE_TYPE_SAFETY - mLock.lock(); - mHandleTypeId[addr] = typeid(D).name(); - mLock.unlock(); -#endif return addr; } @@ -168,19 +136,8 @@ public: void deallocate(Handle& handle, D const* p) noexcept { // allow to destroy the nullptr, similarly to operator delete if (p) { -#if HANDLE_TYPE_SAFETY - mLock.lock(); - auto typeId = mHandleTypeId[p]; - mHandleTypeId.erase(p); - mLock.unlock(); - if (UTILS_UNLIKELY(typeId != typeid(D).name())) { - utils::slog.e << "Destroying handle " << handle.getId() << ", type " << typeid(D).name() - << ", but handle's actual type is " << typeId << utils::io::endl; - std::terminate(); - } -#endif p->~D(); - deallocateHandle(handle.getId()); + deallocateHandle(handle.getId()); } } @@ -208,7 +165,17 @@ public: std::is_base_of_v>, Dp> handle_cast(Handle& handle) noexcept { assert_invariant(handle); - void* const p = handleToPointer(handle.getId()); + auto [p, tag] = handleToPointer(handle.getId()); + + if (isPoolHandle(handle.getId())) { + // check for use after free + uint8_t const age = (tag & HANDLE_AGE_MASK) >> HANDLE_AGE_SHIFT; + auto const pNode = static_cast(p); + uint8_t const expectedAge = pNode[-1].age; + ASSERT_POSTCONDITION(expectedAge == age, + "use-after-free of Handle with id=%d", handle.getId()); + } + return static_cast(p); } @@ -223,29 +190,57 @@ public: private: - // template + template + static constexpr size_t getBucketSize() noexcept { + if constexpr (sizeof(D) <= P0) { return P0; } + if constexpr (sizeof(D) <= P1) { return P1; } + static_assert(sizeof(D) <= P2); + return P2; + } + class Allocator { friend class HandleAllocator; - utils::PoolAllocator mPool0; - utils::PoolAllocator mPool1; - utils::PoolAllocator mPool2; + static constexpr size_t MIN_ALIGNMENT = alignof(std::max_align_t); + struct Node { uint8_t age; }; + // Note: using the `extra` parameter of PoolAllocator<>, even with a 1-byte structure, + // generally increases all pool allocations by 8-bytes because of alignment restrictions. + template + using Pool = utils::PoolAllocator; + Pool mPool0; + Pool mPool1; + Pool mPool2; UTILS_UNUSED_IN_RELEASE const utils::AreaPolicy::HeapArea& mArea; public: - static constexpr size_t MIN_ALIGNMENT_SHIFT = 4; explicit Allocator(const utils::AreaPolicy::HeapArea& area); + static constexpr size_t getAlignment() noexcept { return MIN_ALIGNMENT; } + // this is in fact always called with a constexpr size argument - [[nodiscard]] inline void* alloc(size_t size, size_t, size_t extra = 0) noexcept { + [[nodiscard]] inline void* alloc(size_t size, size_t, size_t, uint8_t* outAge) noexcept { void* p = nullptr; - if (size <= mPool0.getSize()) p = mPool0.alloc(size, 16, extra); - else if (size <= mPool1.getSize()) p = mPool1.alloc(size, 16, extra); - else if (size <= mPool2.getSize()) p = mPool2.alloc(size, 16, extra); + if (size <= mPool0.getSize()) p = mPool0.alloc(size); + else if (size <= mPool1.getSize()) p = mPool1.alloc(size); + else if (size <= mPool2.getSize()) p = mPool2.alloc(size); + if (UTILS_LIKELY(p)) { + Node const* const pNode = static_cast(p); + // we are guaranteed to have at least sizeof bytes of extra storage before + // the allocation address. + *outAge = pNode[-1].age; + } return p; } // this is in fact always called with a constexpr size argument - inline void free(void* p, size_t size) noexcept { + inline void free(void* p, size_t size, uint8_t age) noexcept { assert_invariant(p >= mArea.begin() && (char*)p + size <= (char*)mArea.end()); + + // check for double-free + Node* const pNode = static_cast(p); + uint8_t& expectedAge = pNode[-1].age; + ASSERT_POSTCONDITION(expectedAge == age, + "double-free of Handle of size %d at %p", size, p); + expectedAge = (expectedAge + 1) & 0xF; // fixme + if (size <= mPool0.getSize()) { mPool0.free(p); return; } if (size <= mPool1.getSize()) { mPool1.free(p); return; } if (size <= mPool2.getSize()) { mPool2.free(p); return; } @@ -267,24 +262,16 @@ private: // allocateHandle()/deallocateHandle() selects the pool to use at compile-time based on the // allocation size this is always inlined, because all these do is to call // allocateHandleInPool()/deallocateHandleFromPool() with the right pool size. - template + template HandleBase::HandleId allocateHandle() noexcept { - if constexpr (SIZE <= P0) { return allocateHandleInPool(); } - if constexpr (SIZE <= P1) { return allocateHandleInPool(); } - static_assert(SIZE <= P2); - return allocateHandleInPool(); + constexpr size_t BUCKET_SIZE = getBucketSize(); + return allocateHandleInPool(); } - template + template void deallocateHandle(HandleBase::HandleId id) noexcept { - if constexpr (SIZE <= P0) { - deallocateHandleFromPool(id); - } else if constexpr (SIZE <= P1) { - deallocateHandleFromPool(id); - } else { - static_assert(SIZE <= P2); - deallocateHandleFromPool(id); - } + constexpr size_t BUCKET_SIZE = getBucketSize(); + deallocateHandleFromPool(id); } // allocateHandleInPool()/deallocateHandleFromPool() is NOT inlined, which will cause three @@ -293,9 +280,11 @@ private: template UTILS_NOINLINE HandleBase::HandleId allocateHandleInPool() noexcept { - void* p = mHandleArena.alloc(SIZE); + uint8_t age; + void* p = mHandleArena.alloc(SIZE, alignof(std::max_align_t), 0, &age); if (UTILS_LIKELY(p)) { - return pointerToHandle(p); + uint32_t const tag = (uint32_t(age) << HANDLE_AGE_SHIFT) & HANDLE_AGE_MASK; + return arenaPointerToHandle(p, tag); } else { return allocateHandleSlow(SIZE); } @@ -305,42 +294,51 @@ private: UTILS_NOINLINE void deallocateHandleFromPool(HandleBase::HandleId id) noexcept { if (UTILS_LIKELY(isPoolHandle(id))) { - void* p = handleToPointer(id); - mHandleArena.free(p, SIZE); + auto [p, tag] = handleToPointer(id); + uint8_t const age = (tag & HANDLE_AGE_MASK) >> HANDLE_AGE_SHIFT; + mHandleArena.free(p, SIZE, age); } else { deallocateHandleSlow(id, SIZE); } } - static constexpr uint32_t HEAP_HANDLE_FLAG = 0x80000000u; + // we handle a 4 bits age per address + static constexpr uint32_t HANDLE_HEAP_FLAG = 0x80000000u; // pool vs heap handle + static constexpr uint32_t HANDLE_AGE_MASK = 0x78000000u; // handle's age + static constexpr uint32_t HANDLE_INDEX_MASK = 0x07FFFFFFu; // handle index + static constexpr uint32_t HANDLE_TAG_MASK = HANDLE_AGE_MASK; + static constexpr uint32_t HANDLE_AGE_SHIFT = 27; static bool isPoolHandle(HandleBase::HandleId id) noexcept { - return (id & HEAP_HANDLE_FLAG) == 0u; + return (id & HANDLE_HEAP_FLAG) == 0u; } HandleBase::HandleId allocateHandleSlow(size_t size) noexcept; void deallocateHandleSlow(HandleBase::HandleId id, size_t size) noexcept; // We inline this because it's just 4 instructions in the fast case - inline void* handleToPointer(HandleBase::HandleId id) const noexcept { + inline std::pair handleToPointer(HandleBase::HandleId id) const noexcept { // note: the null handle will end-up returning nullptr b/c it'll be handled as // a non-pool handle. if (UTILS_LIKELY(isPoolHandle(id))) { char* const base = (char*)mHandleArena.getArea().begin(); - size_t offset = id << Allocator::MIN_ALIGNMENT_SHIFT; - return static_cast(base + offset); + uint32_t const tag = id & HANDLE_TAG_MASK; + size_t const offset = (id & HANDLE_INDEX_MASK) * Allocator::getAlignment(); + return { static_cast(base + offset), tag }; } - return handleToPointerSlow(id); + return { handleToPointerSlow(id), 0 }; } void* handleToPointerSlow(HandleBase::HandleId id) const noexcept; // We inline this because it's just 3 instructions - inline HandleBase::HandleId pointerToHandle(void* p) const noexcept { + inline HandleBase::HandleId arenaPointerToHandle(void* p, uint32_t tag) const noexcept { char* const base = (char*)mHandleArena.getArea().begin(); - size_t offset = (char*)p - base; - auto id = HandleBase::HandleId(offset >> Allocator::MIN_ALIGNMENT_SHIFT); - assert_invariant((id & HEAP_HANDLE_FLAG) == 0); + size_t const offset = (char*)p - base; + assert_invariant((offset % Allocator::getAlignment()) == 0); + auto id = HandleBase::HandleId(offset / Allocator::getAlignment()); + id |= tag & HANDLE_TAG_MASK; + assert_invariant((id & HANDLE_HEAP_FLAG) == 0); return id; } @@ -350,9 +348,6 @@ private: mutable utils::Mutex mLock; tsl::robin_map mOverflowMap; HandleBase::HandleId mId = 0; -#if HANDLE_TYPE_SAFETY - mutable std::unordered_map mHandleTypeId; -#endif }; } // namespace filament::backend diff --git a/filament/backend/src/HandleAllocator.cpp b/filament/backend/src/HandleAllocator.cpp index 7362066ca7..12bd26b84c 100644 --- a/filament/backend/src/HandleAllocator.cpp +++ b/filament/backend/src/HandleAllocator.cpp @@ -19,12 +19,17 @@ #include #include +#include +#include #include #include -#include +#include #include +#include +#include +#include #include namespace filament::backend { @@ -36,15 +41,29 @@ UTILS_NOINLINE HandleAllocator::Allocator::Allocator(AreaPolicy::HeapArea const& area) : mArea(area) { + // The largest handle this allocator can generate currently depends on the architecture's + // min alignment, typically 8 or 16 bytes. + // e.g. On Android armv8, the alignment is 16 bytes, so for a 1 MiB heap, the largest handle + // index will be 65536. Note that this is not the same as the number of handles (which + // will always be less). + // Because our maximum representable handle currently is 0x07FFFFFF, the maximum no-nonsensical + // heap size is 2 GiB, which amounts to 7.6 millions handles per pool (in the GL case). + size_t const maxHeapSize = std::min(area.size(), HANDLE_INDEX_MASK * getAlignment()); + + if (UTILS_UNLIKELY(maxHeapSize != area.size())) { + slog.w << "HandleAllocator heap size reduced to " + << maxHeapSize << " from " << area.size() << io::endl; + } + // size the different pools so that they can all contain the same number of handles - size_t const count = area.size() / (P0 + P1 + P2); + size_t const count = maxHeapSize / (P0 + P1 + P2); char* const p0 = static_cast(area.begin()); char* const p1 = p0 + count * P0; char* const p2 = p1 + count * P1; - mPool0 = PoolAllocator(p0, count * P0); - mPool1 = PoolAllocator(p1, count * P1); - mPool2 = PoolAllocator(p2, count * P2); + mPool0 = Pool(p0, count * P0); + mPool1 = Pool(p1, count * P1); + mPool2 = Pool(p2, count * P2); } // ------------------------------------------------------------------------------------------------ @@ -82,11 +101,17 @@ template HandleBase::HandleId HandleAllocator::allocateHandleSlow(size_t size) noexcept { void* p = ::malloc(size); std::unique_lock lock(mLock); - HandleBase::HandleId id = (++mId) | HEAP_HANDLE_FLAG; + + HandleBase::HandleId id = (++mId) | HANDLE_HEAP_FLAG; + + ASSERT_POSTCONDITION(mId < HANDLE_HEAP_FLAG, + "No more Handle ids available! This can happen if HandleAllocator arena has been full" + " for a while. Please increase FILAMENT_OPENGL_HANDLE_ARENA_SIZE_IN_MB"); + mOverflowMap.emplace(id, p); lock.unlock(); - if (UTILS_UNLIKELY(id == (HEAP_HANDLE_FLAG|1u))) { // meaning id was zero + if (UTILS_UNLIKELY(id == (HANDLE_HEAP_FLAG | 1u))) { // meaning id was zero PANIC_LOG("HandleAllocator arena is full, using slower system heap. Please increase " "the appropriate constant (e.g. FILAMENT_OPENGL_HANDLE_ARENA_SIZE_IN_MB)."); } @@ -95,7 +120,7 @@ HandleBase::HandleId HandleAllocator::allocateHandleSlow(size_t size template void HandleAllocator::deallocateHandleSlow(HandleBase::HandleId id, size_t) noexcept { - assert_invariant(id & HEAP_HANDLE_FLAG); + assert_invariant(id & HANDLE_HEAP_FLAG); void* p = nullptr; auto& overflowMap = mOverflowMap; diff --git a/libs/utils/include/utils/Allocator.h b/libs/utils/include/utils/Allocator.h index 2b02eb4cc0..073206f48b 100644 --- a/libs/utils/include/utils/Allocator.h +++ b/libs/utils/include/utils/Allocator.h @@ -250,11 +250,11 @@ public: return mHead; } -private: struct Node { Node* next; }; +private: static Node* init(void* begin, void* end, size_t elementSize, size_t alignment, size_t extra) noexcept; @@ -272,8 +272,8 @@ public: AtomicFreeList() noexcept = default; AtomicFreeList(void* begin, void* end, size_t elementSize, size_t alignment, size_t extra) noexcept; - AtomicFreeList(const FreeList& rhs) = delete; - AtomicFreeList& operator=(const FreeList& rhs) = delete; + AtomicFreeList(const AtomicFreeList& rhs) = delete; + AtomicFreeList& operator=(const AtomicFreeList& rhs) = delete; void* pop() noexcept { Node* const pStorage = mStorage; @@ -319,7 +319,6 @@ public: return mStorage + mHead.load(std::memory_order_relaxed).offset; } -private: struct Node { // This should be a regular (non-atomic) pointer, but this causes TSAN to complain // about a data-race that exists but is benin. We always use this atomic<> in @@ -350,6 +349,7 @@ private: std::atomic next; }; +private: // This struct is using a 32-bit offset into the arena rather than // a direct pointer, because together with the 32-bit tag, it needs to // fit into 8 bytes. If it was any larger, it would not be possible to @@ -372,7 +372,8 @@ template < size_t OFFSET = 0, typename FREELIST = FreeList> class PoolAllocator { - static_assert(ELEMENT_SIZE >= sizeof(void*), "ELEMENT_SIZE must accommodate at least a pointer"); + static_assert(ELEMENT_SIZE >= sizeof(typename FREELIST::Node), + "ELEMENT_SIZE must accommodate at least a FreeList::Node"); public: // our allocator concept void* alloc(size_t size = ELEMENT_SIZE, @@ -636,6 +637,15 @@ public: mListener(name, mArea.data(), mArea.size()) { } + template + void* alloc(size_t size, size_t alignment, size_t extra, ARGS&& ... args) noexcept { + std::lock_guard guard(mLock); + void* p = mAllocator.alloc(size, alignment, extra, std::forward(args) ...); + mListener.onAlloc(p, size, alignment, extra); + return p; + } + + // allocate memory from arena with given size and alignment // (acceptable size/alignment may depend on the allocator provided) void* alloc(size_t size, size_t alignment, size_t extra) noexcept { @@ -668,13 +678,13 @@ public: return (T*)alloc(count * sizeof(T), alignment); } - // return memory pointed by p to the arena - // (actual behaviour may depend on allocator provided) - void free(void* p) noexcept { + // some allocators require more parameters + template + void free(void* p, size_t size, ARGS&& ... args) noexcept { if (p) { std::lock_guard guard(mLock); - mListener.onFree(p); - mAllocator.free(p); + mListener.onFree(p, size); + mAllocator.free(p, size, std::forward(args) ...); } } @@ -687,6 +697,16 @@ public: } } + // return memory pointed by p to the arena + // (actual behaviour may depend on allocator provided) + void free(void* p) noexcept { + if (p) { + std::lock_guard guard(mLock); + mListener.onFree(p); + mAllocator.free(p); + } + } + // some allocators don't have a free() call, but a single reset() or rewind() instead void reset() noexcept { std::lock_guard guard(mLock);