Skip to content

Commit

Permalink
dtoolbase: Fix static init ordering regression
Browse files Browse the repository at this point in the history
This was a regression in bf65624 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
  • Loading branch information
rdb committed Oct 9, 2023
1 parent 1ca0e3f commit 86aa437
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 56 deletions.
18 changes: 1 addition & 17 deletions dtool/src/dtoolbase/deletedBufferChain.I
Expand Up @@ -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) {
}

/**
Expand Down Expand Up @@ -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.
*/
Expand Down
45 changes: 18 additions & 27 deletions dtool/src/dtoolbase/deletedBufferChain.cxx
Expand Up @@ -16,39 +16,19 @@

#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
* no greater than the fixed size associated with the 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);
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 4 additions & 12 deletions dtool/src/dtoolbase/deletedBufferChain.h
Expand Up @@ -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);

Expand All @@ -72,11 +71,9 @@ class EXPCL_DTOOL_DTOOLBASE DeletedBufferChain {

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
Expand All @@ -99,7 +96,7 @@ class EXPCL_DTOOL_DTOOLBASE DeletedBufferChain {
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.
Expand All @@ -110,11 +107,6 @@ class EXPCL_DTOOL_DTOOLBASE DeletedBufferChain {
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;
};

Expand Down

0 comments on commit 86aa437

Please sign in to comment.