From 217fa406664c07b3d8ac579ea23cc2808841fe97 Mon Sep 17 00:00:00 2001 From: Per Mathisen Date: Fri, 27 Dec 2024 01:38:14 +0100 Subject: [PATCH] Add more sanity checking --- src/filereader.h | 5 +++++ src/filewriter.h | 12 +++++++++++- src/lavatube.h | 26 ++++++++++++++++++++++++++ src/read.h | 14 +++++++------- src/suballocator.cpp | 7 ++++++- src/write.cpp | 2 ++ src/write.h | 6 ++++++ 7 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/filereader.h b/src/filereader.h index 64c2ad3..32731a2 100644 --- a/src/filereader.h +++ b/src/filereader.h @@ -222,6 +222,11 @@ class file_reader chunk_mutex.unlock(); } + void self_test() const + { + if (done_decompressing.load()) assert(done_reading.load()); + } + private: void decompressor(); // runs in separate thread, moves chunks from file to uncompressed chunks diff --git a/src/filewriter.h b/src/filewriter.h index 84c0b15..3128f4a 100644 --- a/src/filewriter.h +++ b/src/filewriter.h @@ -31,7 +31,12 @@ class file_writer else { buffer compressed = compress_chunk(chunk); - if (multithreaded_write) compressed_chunks.push_front(compressed); + if (multithreaded_write) + { + chunk_mutex.lock(); + compressed_chunks.push_front(compressed); + chunk_mutex.unlock(); + } else write_chunk(compressed); } @@ -158,6 +163,11 @@ class file_writer chunk_mutex.unlock(); } + void self_test() const + { + if (done_compressing.load()) assert(done_feeding.load()); + } + protected: uint64_t uncompressed_bytes = 0; // total amount of uncompressed bytes written so far uint64_t checkpoint_bytes = 0; // bytes at freeze checkpoint diff --git a/src/lavatube.h b/src/lavatube.h index d514423..b2d5090 100644 --- a/src/lavatube.h +++ b/src/lavatube.h @@ -17,6 +17,11 @@ #include #include #include +#include + +// Basically just assuming this will be fixed in later versions of the standard and that existing +// implementations will continue doing the only sensible thing here. +#pragma GCC diagnostic ignored "-Winvalid-offsetof" class lava_file_reader; class lava_file_writer; @@ -52,6 +57,11 @@ struct trackable void self_test() const { + // We assume child classes put their fields after parent classes. This is not guaranteed + // by the standard. If this is not happening, we're in trouble, so assert on it. + static_assert(offsetof(trackable, magic) == 0, "ICD loader magic must be at offset zero!"); + static_assert(std::is_standard_layout_v == true); // only applies to base class + creation.self_test(); last_modified.self_test(); } @@ -91,6 +101,7 @@ struct trackedmemory : trackable void self_test() const { + static_assert(offsetof(trackedmemory, magic) == 0, "ICD loader magic must be at offset zero!"); \ assert(backing != VK_NULL_HANDLE); assert(offset + size <= allocationSize); assert(exposed.span().last <= allocationSize); @@ -127,6 +138,7 @@ struct trackedobject : trackable void self_test() const { + static_assert(offsetof(trackedobject, magic) == 0, "ICD loader magic must be at offset zero!"); \ assert(type != VK_OBJECT_TYPE_UNKNOWN); assert(size != VK_WHOLE_SIZE); trackable::self_test(); @@ -156,6 +168,7 @@ struct trackedbuffer : trackedmemoryobject void self_test() const { + static_assert(offsetof(trackedbuffer, magic) == 0, "ICD loader magic must be at offset zero!"); assert(flags != VK_BUFFER_CREATE_FLAG_BITS_MAX_ENUM); assert(sharingMode != VK_SHARING_MODE_MAX_ENUM); assert(usage != VK_BUFFER_USAGE_FLAG_BITS_MAX_ENUM); @@ -174,6 +187,7 @@ struct trackedaccelerationstructure : trackedmemoryobject void self_test() const { + static_assert(offsetof(trackedaccelerationstructure, magic) == 0, "ICD loader magic must be at offset zero!"); assert(buffer != VK_NULL_HANDLE); assert(buffer_index != CONTAINER_INVALID_INDEX); assert(type != VK_ACCELERATION_STRUCTURE_TYPE_MAX_ENUM_KHR); @@ -199,6 +213,7 @@ struct trackedimage : trackedobject void self_test() const { + static_assert(offsetof(trackedimage, magic) == 0, "ICD loader magic must be at offset zero!"); assert(tiling != VK_IMAGE_TILING_MAX_ENUM); assert(usage != VK_IMAGE_USAGE_FLAG_BITS_MAX_ENUM); assert(sharingMode != VK_SHARING_MODE_MAX_ENUM); @@ -225,6 +240,7 @@ struct trackedimageview : trackable void self_test() const { + static_assert(offsetof(trackedimageview, magic) == 0, "ICD loader magic must be at offset zero!"); assert(image != VK_NULL_HANDLE); assert(image_index != CONTAINER_INVALID_INDEX); assert(format != VK_FORMAT_UNDEFINED); @@ -244,6 +260,7 @@ struct trackedbufferview : trackable void self_test() const { + static_assert(offsetof(trackedbufferview, magic) == 0, "ICD loader magic must be at offset zero!"); assert(buffer != VK_NULL_HANDLE); assert(buffer_index != CONTAINER_INVALID_INDEX); assert(format != VK_FORMAT_UNDEFINED); @@ -258,6 +275,7 @@ struct trackedswapchain : trackable void self_test() const { + static_assert(offsetof(trackedswapchain, magic) == 0, "ICD loader magic must be at offset zero!"); assert(info.sType == VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR); trackable::self_test(); } @@ -282,6 +300,7 @@ struct trackedswapchain_replay : trackedswapchain void self_test() const { + static_assert(offsetof(trackedswapchain_replay, magic) == 0, "ICD loader magic must be at offset zero!"); if (!initialized) return; assert(swapchain != VK_NULL_HANDLE); if (p__virtualswap) @@ -314,6 +333,7 @@ struct trackedpipeline : trackable void self_test() const { + static_assert(offsetof(trackedpipeline, magic) == 0, "ICD loader magic must be at offset zero!"); assert(type != VK_PIPELINE_BIND_POINT_MAX_ENUM); trackable::self_test(); } @@ -339,6 +359,7 @@ struct trackedcmdbuffer : trackable void self_test() const { + static_assert(offsetof(trackedcmdbuffer, magic) == 0, "ICD loader magic must be at offset zero!"); assert(device != VK_NULL_HANDLE); assert(physicalDevice != VK_NULL_HANDLE); assert(pool_index != CONTAINER_INVALID_INDEX); @@ -403,6 +424,7 @@ struct trackedcmdbuffer_trace : trackedcmdbuffer void self_test() const { + static_assert(offsetof(trackedcmdbuffer_trace, magic) == 0, "ICD loader magic must be at offset zero!"); for (const auto& pair : touched) { assert(pair.first->accessible); pair.first->self_test(); pair.second.self_test(); } assert(pool != VK_NULL_HANDLE); assert(device != VK_NULL_HANDLE); @@ -421,6 +443,7 @@ struct trackeddescriptorset : trackable void self_test() const { + static_assert(offsetof(trackeddescriptorset, magic) == 0, "ICD loader magic must be at offset zero!"); assert(pool != VK_NULL_HANDLE); assert(pool != VK_NULL_HANDLE); assert(pool_index != CONTAINER_INVALID_INDEX); @@ -444,6 +467,7 @@ struct trackeddescriptorset_trace : trackeddescriptorset void self_test() const { + static_assert(offsetof(trackeddescriptorset_trace, magic) == 0, "ICD loader magic must be at offset zero!"); for (const auto& pair : touched) { pair.first->self_test(); pair.second.self_test(); } trackeddescriptorset::self_test(); } @@ -463,6 +487,7 @@ struct trackedqueue : trackable void self_test() const { + static_assert(offsetof(trackedqueue, magic) == 0, "ICD loader magic must be at offset zero!"); assert(device != VK_NULL_HANDLE); assert(physicalDevice != VK_NULL_HANDLE); assert(queueFlags != VK_QUEUE_FLAG_BITS_MAX_ENUM); @@ -509,6 +534,7 @@ struct trackedframebuffer : trackable void self_test() const { + static_assert(offsetof(trackedframebuffer, magic) == 0, "ICD loader magic must be at offset zero!"); for (const auto v : imageviews) v->self_test(); trackable::self_test(); } diff --git a/src/read.h b/src/read.h index 4c97aa4..1e6f6fc 100644 --- a/src/read.h +++ b/src/read.h @@ -143,7 +143,13 @@ class lava_file_reader : public file_reader change_source current; - inline void self_test(); + void self_test() const + { + assert(parent); + assert(run == parent->run); + assert(global_frames >= local_frames); + file_reader::self_test(); + } private: bool mPreload; @@ -159,12 +165,6 @@ class lava_file_reader : public file_reader unsigned local_frames = 0; }; -inline void lava_file_reader::self_test() -{ - assert(parent); - assert(run == parent->run); -} - inline void lava_file_reader::read_barrier() { const unsigned size = read_uint8_t(); diff --git a/src/suballocator.cpp b/src/suballocator.cpp index bb04159..b53237c 100644 --- a/src/suballocator.cpp +++ b/src/suballocator.cpp @@ -47,6 +47,11 @@ struct heap /// up deletes in this concurrency safe vector instead. tbb::concurrent_vector deletes; VkImageTiling tiling = VK_IMAGE_TILING_LINEAR; // buffers are assumed to be linear + + void self_test() const + { + assert(free <= total); + } }; struct lookup @@ -524,10 +529,10 @@ int suballoc_internal_test() // walk the heaps to check consistency for (const heap& h : heaps) { + h.self_test(); uint64_t freed = 0; if (h.subs.size() > 0) freed = h.subs.front().offset; uint64_t used = 0; - assert(h.free <= h.total); int64_t prev_end = -1; // end of previous allocation for (auto it = h.subs.cbegin(); it != h.subs.cend(); ++it) { diff --git a/src/write.cpp b/src/write.cpp index ba5417e..d4be4ce 100644 --- a/src/write.cpp +++ b/src/write.cpp @@ -94,6 +94,7 @@ void lava_file_writer::inject_thread_barrier() lava_file_writer::~lava_file_writer() { + self_test(); file_writer::finalize(); // Write out per-frame data @@ -128,6 +129,7 @@ debug_info lava_file_writer::new_frame(int global_frame) current.frame++; debug_info retval = debug; debug = {}; + self_test(); return retval; } diff --git a/src/write.h b/src/write.h index 4102188..23dd560 100644 --- a/src/write.h +++ b/src/write.h @@ -181,6 +181,12 @@ class lava_file_writer : public file_writer std::atomic_bool pending_barrier { false }; + void self_test() const + { + assert(parent != nullptr); + file_writer::self_test(); + } + private: std::string mPath; std::vector frames;