Index: compiler-rt/lib/xray/tests/unit/buffer_queue_test.cc =================================================================== --- compiler-rt/lib/xray/tests/unit/buffer_queue_test.cc +++ compiler-rt/lib/xray/tests/unit/buffer_queue_test.cc @@ -55,6 +55,7 @@ BufferQueue::Buffer Buf; Buf.Data = reinterpret_cast(0xdeadbeef); Buf.Size = kSize; + Buf.Generation = Buffers.generation(); EXPECT_EQ(BufferQueue::ErrorCode::UnrecognizedBuffer, Buffers.releaseBuffer(Buf)); } @@ -70,8 +71,7 @@ BufferQueue::Buffer OtherBuf; ASSERT_EQ(BufferQueue::ErrorCode::QueueFinalizing, Buffers.getBuffer(OtherBuf)); - ASSERT_EQ(BufferQueue::ErrorCode::QueueFinalizing, - Buffers.finalize()); + ASSERT_EQ(BufferQueue::ErrorCode::QueueFinalizing, Buffers.finalize()); ASSERT_EQ(Buffers.releaseBuffer(Buf), BufferQueue::ErrorCode::Ok); } @@ -111,4 +111,30 @@ ASSERT_EQ(Count, 10); } +TEST(BufferQueueTest, GenerationalSupport) { + bool Success = false; + BufferQueue Buffers(kSize, 10, Success); + ASSERT_TRUE(Success); + BufferQueue::Buffer B0; + ASSERT_EQ(Buffers.getBuffer(B0), BufferQueue::ErrorCode::Ok); + ASSERT_EQ(Buffers.finalize(), + BufferQueue::ErrorCode::Ok); // No more new buffers. + + // Re-initialise the queue. + ASSERT_EQ(Buffers.init(kSize, 10), BufferQueue::ErrorCode::Ok); + + BufferQueue::Buffer B1; + ASSERT_EQ(Buffers.getBuffer(B1), BufferQueue::ErrorCode::Ok); + + // Validate that the buffers come from different generations. + ASSERT_NE(B0.Generation, B1.Generation); + + // At this point, we want to ensure that we can return the buffer from the + // first "generation" would still be accepted in the new generation... + EXPECT_EQ(Buffers.releaseBuffer(B0), BufferQueue::ErrorCode::Ok); + + // ... and that the new buffer is also accepted. + EXPECT_EQ(Buffers.releaseBuffer(B1), BufferQueue::ErrorCode::Ok); +} + } // namespace __xray Index: compiler-rt/lib/xray/xray_buffer_queue.h =================================================================== --- compiler-rt/lib/xray/xray_buffer_queue.h +++ compiler-rt/lib/xray/xray_buffer_queue.h @@ -33,6 +33,7 @@ public: struct Buffer { atomic_uint64_t Extents{0}; + uint64_t Generation{0}; void *Data = nullptr; size_t Size = 0; }; @@ -130,6 +131,10 @@ // Count of buffers that have been handed out through 'getBuffer'. size_t LiveBuffers; + // We use a generation number to identify buffers and which generation they're + // associated with. + atomic_uint64_t Generation; + public: enum class ErrorCode : unsigned { Ok, @@ -137,6 +142,7 @@ QueueFinalizing, UnrecognizedBuffer, AlreadyFinalized, + AlreadyInitialized, }; static const char *getErrorString(ErrorCode E) { @@ -151,6 +157,8 @@ return "buffer being returned not owned by buffer queue"; case ErrorCode::AlreadyFinalized: return "queue already finalized"; + case ErrorCode::AlreadyInitialized: + return "queue already initialized"; } return "unknown error"; } @@ -181,10 +189,23 @@ /// the buffer being released. ErrorCode releaseBuffer(Buffer &Buf); + /// Initializes the buffer queue, starting a new generation. We can re-set the + /// size of buffers with |BS| along with the buffer count with |BC|. + /// + /// Returns: + /// - ErrorCode::Ok when we successfully initialize the buffer. This + /// requires that the buffer queue is previously finalized. + /// - ErrorCode::AlreadyInitialized when the buffer queue is not finalized. + ErrorCode init(size_t BS, size_t BC); + bool finalizing() const { return atomic_load(&Finalizing, memory_order_acquire); } + uint64_t generation() const { + return atomic_load(&Generation, memory_order_acquire); + } + /// Returns the configured size of the buffers in the buffer queue. size_t ConfiguredBufferSize() const { return BufferSize; } Index: compiler-rt/lib/xray/xray_buffer_queue.cc =================================================================== --- compiler-rt/lib/xray/xray_buffer_queue.cc +++ compiler-rt/lib/xray/xray_buffer_queue.cc @@ -24,58 +24,84 @@ using namespace __xray; using namespace __sanitizer; -BufferQueue::BufferQueue(size_t B, size_t N, - bool &Success) XRAY_NEVER_INSTRUMENT - : BufferSize(B), - BufferCount(N), - Mutex(), - Finalizing{0}, - BackingStore(allocateBuffer(B *N)), - Buffers(initArray(N)), - Next(Buffers), - First(Buffers), - LiveBuffers(0) { - if (BackingStore == nullptr) { - Success = false; - return; - } - if (Buffers == nullptr) { +BufferQueue::ErrorCode BufferQueue::init(size_t BS, size_t BC) { + if (!finalizing()) + return BufferQueue::ErrorCode::AlreadyInitialized; + + atomic_store(&Finalizing, 0, memory_order_release); + SpinMutexLock Guard(&Mutex); + bool Success = false; + BufferSize = BS; + BufferCount = BC; + BackingStore = allocateBuffer(BufferSize * BufferCount); + if (BackingStore == nullptr) + return BufferQueue::ErrorCode::NotEnoughMemory; + + auto CleanupBackingStore = __sanitizer::at_scope_exit([&, this] { + if (Success) + return; deallocateBuffer(BackingStore, BufferSize * BufferCount); - Success = false; - return; - } + }); + + Buffers = initArray(BufferCount); + if (Buffers == nullptr) + return BufferQueue::ErrorCode::NotEnoughMemory; - for (size_t i = 0; i < N; ++i) { + // At this point we increment the generation number to associate the buffers + // to the new generation. + atomic_fetch_add(&Generation, 1, memory_order_acq_rel); + + Success = true; + for (size_t i = 0; i < BufferCount; ++i) { auto &T = Buffers[i]; auto &Buf = T.Buff; - Buf.Data = reinterpret_cast(BackingStore) + (BufferSize * i); - Buf.Size = B; atomic_store(&Buf.Extents, 0, memory_order_release); + Buf.Generation = generation(); + Buf.Data = reinterpret_cast(BackingStore) + (BufferSize * i); + Buf.Size = BufferSize; T.Used = false; } - Success = true; + + Next = Buffers; + First = Buffers; + LiveBuffers = 0; + return BufferQueue::ErrorCode::Ok; +} + +BufferQueue::BufferQueue(size_t B, size_t N, + bool &Success) XRAY_NEVER_INSTRUMENT + : BufferSize(B), + BufferCount(N), + Mutex(), + Finalizing{1}, + BackingStore(nullptr), + Buffers(nullptr), + Next(Buffers), + First(Buffers), + LiveBuffers(0), + Generation{0} { + Success = init(B, N) == BufferQueue::ErrorCode::Ok; } BufferQueue::ErrorCode BufferQueue::getBuffer(Buffer &Buf) { if (atomic_load(&Finalizing, memory_order_acquire)) return ErrorCode::QueueFinalizing; - SpinMutexLock Guard(&Mutex); - if (LiveBuffers == BufferCount) - return ErrorCode::NotEnoughMemory; - - auto &T = *Next; - auto &B = T.Buff; - auto Extents = atomic_load(&B.Extents, memory_order_acquire); - atomic_store(&Buf.Extents, Extents, memory_order_release); - Buf.Data = B.Data; - Buf.Size = B.Size; - T.Used = true; - ++LiveBuffers; - - if (++Next == (Buffers + BufferCount)) - Next = Buffers; + BufferRep *B = nullptr; + { + SpinMutexLock Guard(&Mutex); + if (LiveBuffers == BufferCount) + return ErrorCode::NotEnoughMemory; + B = Next++; + if (Next == (Buffers + BufferCount)) + Next = Buffers; + ++LiveBuffers; + } + Buf.Data = B->Buff.Data; + Buf.Generation = generation(); + Buf.Size = B->Buff.Size; + B->Used = true; return ErrorCode::Ok; } @@ -84,29 +110,42 @@ // backing store's range. if (Buf.Data < BackingStore || Buf.Data > - reinterpret_cast(BackingStore) + (BufferCount * BufferSize)) - return ErrorCode::UnrecognizedBuffer; + reinterpret_cast(BackingStore) + (BufferCount * BufferSize)) { + if (Buf.Generation != generation()) { + Buf.Data = nullptr; + Buf.Size = 0; + Buf.Generation = 0; + return BufferQueue::ErrorCode::Ok; + } + return BufferQueue::ErrorCode::UnrecognizedBuffer; + } - SpinMutexLock Guard(&Mutex); + BufferRep *B = nullptr; + { + SpinMutexLock Guard(&Mutex); + + // This points to a semantic bug, we really ought to not be releasing more + // buffers than we actually get. + if (LiveBuffers == 0) + return ErrorCode::NotEnoughMemory; - // This points to a semantic bug, we really ought to not be releasing more - // buffers than we actually get. - if (LiveBuffers == 0) - return ErrorCode::NotEnoughMemory; + --LiveBuffers; + B = First++; + if (First == (Buffers + BufferCount)) + First = Buffers; + } // Now that the buffer has been released, we mark it as "used". - auto Extents = atomic_load(&Buf.Extents, memory_order_acquire); - atomic_store(&First->Buff.Extents, Extents, memory_order_release); - First->Buff.Data = Buf.Data; - First->Buff.Size = Buf.Size; - First->Used = true; + B->Buff.Data = Buf.Data; + B->Buff.Size = Buf.Size; + B->Buff.Generation = Buf.Generation; + B->Used = true; + atomic_store(&B->Buff.Extents, + atomic_load(&Buf.Extents, memory_order_acquire), + memory_order_release); Buf.Data = nullptr; Buf.Size = 0; - atomic_store(&Buf.Extents, 0, memory_order_release); - --LiveBuffers; - if (++First == (Buffers + BufferCount)) - First = Buffers; - + Buf.Generation = 0; return ErrorCode::Ok; } Index: compiler-rt/lib/xray/xray_fdr_logging.cc =================================================================== --- compiler-rt/lib/xray/xray_fdr_logging.cc +++ compiler-rt/lib/xray/xray_fdr_logging.cc @@ -1056,8 +1056,7 @@ endBufferIfFull(); } -XRayLogInitStatus fdrLoggingInit(UNUSED size_t BufferSize, - UNUSED size_t BufferMax, void *Options, +XRayLogInitStatus fdrLoggingInit(size_t, size_t, void *Options, size_t OptionsSize) XRAY_NEVER_INSTRUMENT { if (Options == nullptr) return XRayLogInitStatus::XRAY_LOG_UNINITIALIZED; @@ -1104,9 +1103,8 @@ // environment-variable defined options. FDRParser.ParseString(static_cast(Options)); *fdrFlags() = FDRFlags; - BufferSize = FDRFlags.buffer_size; - BufferMax = FDRFlags.buffer_max; - + auto BufferSize = FDRFlags.buffer_size; + auto BufferMax = FDRFlags.buffer_max; bool Success = false; if (BQ != nullptr) {