Browse Source

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
rdb 2 years ago
parent
commit
86aa437804

+ 1 - 17
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.
  */

+ 18 - 27
dtool/src/dtoolbase/deletedBufferChain.cxx

@@ -16,32 +16,10 @@
 
 #include <set>
 
-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<DeletedBufferChain> deleted_chains;

+ 4 - 12
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 &copy);
 
@@ -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;
 };