From 86aa437804888d8a126875437b4f1cc4431d4e07 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 9 Oct 2023 12:49:04 +0200 Subject: [PATCH] dtoolbase: Fix static init ordering regression This was a regression in bf65624298b9a5ea49cb637fadb4fc6f7c85ce9b that caused crashes on startup in static builds due to the "small" DeletedBufferChain array not being initialized early enough For some reason it wasn't being constant-initialized, it is now by setting the _buffer_size field to 0 initially and changing it later in get_deleted_chain --- dtool/src/dtoolbase/deletedBufferChain.I | 18 +-------- dtool/src/dtoolbase/deletedBufferChain.cxx | 45 +++++++++------------- dtool/src/dtoolbase/deletedBufferChain.h | 16 ++------ 3 files changed, 23 insertions(+), 56 deletions(-) diff --git a/dtool/src/dtoolbase/deletedBufferChain.I b/dtool/src/dtoolbase/deletedBufferChain.I index f74c9dd5eb..2b79283ea8 100644 --- a/dtool/src/dtoolbase/deletedBufferChain.I +++ b/dtool/src/dtoolbase/deletedBufferChain.I @@ -16,7 +16,7 @@ * size. */ constexpr DeletedBufferChain:: -DeletedBufferChain(size_t buffer_size) : _buffer_size(buffer_size) { +DeletedBufferChain(size_t buffer_size) noexcept : _buffer_size(buffer_size) { } /** @@ -77,22 +77,6 @@ operator < (const DeletedBufferChain &other) const { return _buffer_size < other._buffer_size; } -/** - * Returns a deleted chain of the given size. - */ -INLINE DeletedBufferChain *DeletedBufferChain:: -get_deleted_chain(size_t buffer_size) { - // We must allocate at least this much space for bookkeeping reasons. - buffer_size = (std::max)(buffer_size, sizeof(ObjectNode)); - - size_t index = ((buffer_size + sizeof(void *) - 1) / sizeof(void *)) - 1; - if (index < num_small_deleted_chains) { - return &_small_deleted_chains[index]; - } else { - return get_large_deleted_chain((index + 1) * sizeof(void *)); - } -} - /** * Casts an ObjectNode* to a void* buffer. */ diff --git a/dtool/src/dtoolbase/deletedBufferChain.cxx b/dtool/src/dtoolbase/deletedBufferChain.cxx index de74173175..9821129254 100644 --- a/dtool/src/dtoolbase/deletedBufferChain.cxx +++ b/dtool/src/dtoolbase/deletedBufferChain.cxx @@ -16,32 +16,10 @@ #include -DeletedBufferChain DeletedBufferChain::_small_deleted_chains[DeletedBufferChain::num_small_deleted_chains] = { - DeletedBufferChain(sizeof(void *)), - DeletedBufferChain(sizeof(void *) * 2), - DeletedBufferChain(sizeof(void *) * 3), - DeletedBufferChain(sizeof(void *) * 4), - DeletedBufferChain(sizeof(void *) * 5), - DeletedBufferChain(sizeof(void *) * 6), - DeletedBufferChain(sizeof(void *) * 7), - DeletedBufferChain(sizeof(void *) * 8), - DeletedBufferChain(sizeof(void *) * 9), - DeletedBufferChain(sizeof(void *) * 10), - DeletedBufferChain(sizeof(void *) * 11), - DeletedBufferChain(sizeof(void *) * 12), - DeletedBufferChain(sizeof(void *) * 13), - DeletedBufferChain(sizeof(void *) * 14), - DeletedBufferChain(sizeof(void *) * 15), - DeletedBufferChain(sizeof(void *) * 16), - DeletedBufferChain(sizeof(void *) * 17), - DeletedBufferChain(sizeof(void *) * 18), - DeletedBufferChain(sizeof(void *) * 19), - DeletedBufferChain(sizeof(void *) * 20), - DeletedBufferChain(sizeof(void *) * 21), - DeletedBufferChain(sizeof(void *) * 22), - DeletedBufferChain(sizeof(void *) * 23), - DeletedBufferChain(sizeof(void *) * 24), -}; +// This array stores the deleted chains for smaller sizes, starting with +// sizeof(void *) and increasing in multiples thereof. +static const size_t num_small_deleted_chains = 24; +static DeletedBufferChain small_deleted_chains[num_small_deleted_chains] = {}; /** * Allocates the memory for a new buffer of the indicated size (which must be @@ -49,6 +27,8 @@ DeletedBufferChain DeletedBufferChain::_small_deleted_chains[DeletedBufferChain: */ void *DeletedBufferChain:: allocate(size_t size, TypeHandle type_handle) { + assert(_buffer_size > 0); + #ifdef USE_DELETED_CHAIN // TAU_PROFILE("void *DeletedBufferChain::allocate(size_t, TypeHandle)", " // ", TAU_USER); @@ -161,7 +141,18 @@ deallocate(void *ptr, TypeHandle type_handle) { * Returns a new DeletedBufferChain. */ DeletedBufferChain *DeletedBufferChain:: -get_large_deleted_chain(size_t buffer_size) { +get_deleted_chain(size_t buffer_size) { + // Common, smaller sized chains avoid the expensive locking and set + // manipulation code further down. + size_t index = ((buffer_size + sizeof(void *) - 1) / sizeof(void *)); + buffer_size = index * sizeof(void *); + index--; + if (index < num_small_deleted_chains) { + DeletedBufferChain *chain = &small_deleted_chains[index]; + chain->_buffer_size = buffer_size; + return chain; + } + static MutexImpl lock; lock.lock(); static std::set deleted_chains; diff --git a/dtool/src/dtoolbase/deletedBufferChain.h b/dtool/src/dtoolbase/deletedBufferChain.h index 25adf6ce5e..1e47adc81b 100644 --- a/dtool/src/dtoolbase/deletedBufferChain.h +++ b/dtool/src/dtoolbase/deletedBufferChain.h @@ -57,10 +57,9 @@ enum DeletedChainFlag : unsigned int { * Use MemoryHook to get a new DeletedBufferChain of a particular size. */ class EXPCL_DTOOL_DTOOLBASE DeletedBufferChain { -protected: - constexpr explicit DeletedBufferChain(size_t buffer_size); - public: + constexpr DeletedBufferChain() = default; + constexpr explicit DeletedBufferChain(size_t buffer_size) noexcept; INLINE DeletedBufferChain(DeletedBufferChain &&from) noexcept; INLINE DeletedBufferChain(const DeletedBufferChain ©); @@ -72,11 +71,9 @@ public: INLINE bool operator < (const DeletedBufferChain &other) const; - static INLINE DeletedBufferChain *get_deleted_chain(size_t buffer_size); + static DeletedBufferChain *get_deleted_chain(size_t buffer_size); private: - static DeletedBufferChain *get_large_deleted_chain(size_t buffer_size); - class ObjectNode { public: #ifdef USE_DELETEDCHAINFLAG @@ -99,7 +96,7 @@ private: ObjectNode *_deleted_chain = nullptr; MutexImpl _lock; - const size_t _buffer_size; + size_t _buffer_size = 0; #ifndef USE_DELETEDCHAINFLAG // Without DELETEDCHAINFLAG, we don't even store the _flag member at all. @@ -110,11 +107,6 @@ private: static const size_t flag_reserved_bytes = sizeof(AtomicAdjust::Integer); #endif // USE_DELETEDCHAINFLAG - // This array stores the deleted chains for smaller sizes, starting with - // sizeof(void *) and increasing in multiples thereof. - static const size_t num_small_deleted_chains = 24; - static DeletedBufferChain _small_deleted_chains[num_small_deleted_chains]; - friend class MemoryHook; };