[XRay][compiler-rt] Remove use of std::mutex and std::shared_ptr from global scope.

[dberris]

Summary

This change attempts to remove all the dependencies we have on
std::mutex and any std::shared_ptr construction in global variables. We
instead use raw pointers to these objects, and construct them on the
heap. In cases where it's possible, we lazily initialize these pointers.

While we do not have a replacement for std::shared_ptr yet in
compiler-rt, we use this work-around to avoid having to statically
initialize the objects as globals. Subsequent changes should allow us to
completely remove our dependency on std::shared_ptr and instead have our
own implementation of the std::shared_ptr and std::weak_ptr semantics
(or completely rewrite the implementaton to not need these
standard-library provided abstractions).

[dblaikie]

lib/xray/xray_fdr_logging.cc

152 ↗

(On Diff #108884)

->

275 ↗

(On Diff #108884)

Could drop the nullptr here

lib/xray/xray_fdr_logging_impl.h

173 ↗

(On Diff #108884)

and here

471 ↗

(On Diff #108884)

Might be a good opportunity to change releaseThreadLocalBuffer take the BufferQueeu by mutable reference, since it's required to be nonnull anyway? Then this parameter would be '**LocalBQ' (which at least I treat with less suspicion than a function called 'release' that takes a raw pointer to data that's owned by a smart pointer... )

lib/xray/xray_inmemory_log.cc

46 ↗

(On Diff #108884)

Might be worth separating these two changes (the mutex from the shared-ptr) - they seem pretty independent & this one looks like it's good forward progress to move away from standard library dependencies as is required by compiler-rt, the other's more of a stop-gap (or at least neutral on the progress in that direction).

85–86 ↗

(On Diff #108884)

unnecessary reformatting? or did something change in this line that I can't quite spot.

[dberris]

Address some comments.

lib/xray/xray_inmemory_log.cc

46 ↗

(On Diff #108884)

Thought about it, but for efficiency's sake, I suspect I'd like to make both changes as a group. They're both trying to achieve the same thing (no complex/atomic initialisation at init-time) and are working towards the same (partial) goal.

85–86 ↗

(On Diff #108884)

Looks like this wasn't formatted correctly before, and clang-format did the needful for me. :)

[dberris]

Fix build break.

[kpw]

What's the primary motivation? Are we trying to avoid the upfront cost of global initialization? The unspecified destructor ordering with multithreaded programs?

[dberris]

In D36078#826962, @kpw wrote:

What's the primary motivation? Are we trying to avoid the upfront cost of global initialization? The unspecified destructor ordering with multithreaded programs?

The primary motivation is to avoid the initialization of atomics at initialization time. This has caused some issues in the past with dynamic linking and relocations.

It also allows us to start removing the dependencies on the C++ standard library in the implementation. Using raw pointers gets us there, and not using std::mutex gets us there closer.

[kpw]

Thanks for the clarification of intent Dean. LGTM, but I'd like to clear up one source of confusion with name collisions.

lib/xray/xray_fdr_logging_impl.h

172 ↗

(On Diff #109044)

Not a big deal, but earlier you declare "shared_ptr<T>* name" and here you have "shared_ptr<T> *name".

Not sure which is preferred in LLVM.

497 ↗

(On Diff #109044)

Here LocalBQ is a shared_ptr& that shadows the thread local shared_ptr* of the same name. The shadowing is not new, but the types used to match.

I find this confusing. Can we rename the param LocalBQ to ParamBQ? Although I don't think dblakie's suggestion to pass \*\*LocalBQ from the call sites applies here too because \*\*LocalBQ is allowed to be nullptr if RecordPtr == nullptr or xray status is XRAY_LOG_INITIALIZING.

508 ↗

(On Diff #109044)

Changing .get()s to *s for the smart ptr parameter is unnecessary and contributes to the confusion caused by the shadowing.

[dberris]

• fixup: avoid shadowing global variable

[dberris]

Thanks @kpw!

lib/xray/xray_fdr_logging_impl.h

172 ↗

(On Diff #109044)

The former is LLVM preferred. Somehow my local formatter keeps ignoring the LLVM style. :(

497 ↗

(On Diff #109044)

Good point. Renamed to LBQ instead in this function.

compiler-rt/trunk/include/xray/xray_records.h

 // This header exposes some record types useful for the XRay in-memory logging
 // implementation.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef XRAY_XRAY_RECORDS_H
 #define XRAY_XRAY_RECORDS_H
 
+#include <cstdint>
+
 namespace __xray {
 
 enum FileTypes {
   NAIVE_LOG = 0,
   FDR_LOG = 1,
 };
 
 // FDR mode use of the union field in the XRayFileHeader.

compiler-rt/trunk/lib/xray/xray_fdr_logging.cc

 #include "xray_fdr_logging_impl.h"
 #include "xray_flags.h"
 #include "xray_tsc.h"
 #include "xray_utils.h"
 
 namespace __xray {
 
 // Global BufferQueue.
-std::shared_ptr<BufferQueue> BQ;
+// NOTE: This is a pointer to avoid having to do atomic operations at
+// initialization time. This is OK to leak as there will only be one bufferqueue
+// for the runtime, initialized once through the fdrInit(...) sequence.
+std::shared_ptr<BufferQueue>* BQ = nullptr;
 
 __sanitizer::atomic_sint32_t LogFlushStatus = {
     XRayLogFlushStatus::XRAY_LOG_NOT_FLUSHING};
 
 FDRLoggingOptions FDROptions;
 
 __sanitizer::SpinMutex FDROptionsMutex;
 
 // Must finalize before flushing.
 XRayLogFlushStatus fdrLoggingFlush() XRAY_NEVER_INSTRUMENT {
   if (__sanitizer::atomic_load(&LoggingStatus,
                                __sanitizer::memory_order_acquire) !=
       XRayLogInitStatus::XRAY_LOG_FINALIZED)
     return XRayLogFlushStatus::XRAY_LOG_NOT_FLUSHING;
 
   s32 Result = XRayLogFlushStatus::XRAY_LOG_NOT_FLUSHING;
   if (!__sanitizer::atomic_compare_exchange_strong(
           &LogFlushStatus, &Result, XRayLogFlushStatus::XRAY_LOG_FLUSHING,
           __sanitizer::memory_order_release))
     return static_cast<XRayLogFlushStatus>(Result);
 
   // Make a copy of the BufferQueue pointer to prevent other threads that may be
   // resetting it from blowing away the queue prematurely while we're dealing
   // with it.
-  auto LocalBQ = BQ;
+  auto LocalBQ = *BQ;
 
   // We write out the file in the following format:
   //
   //   1) We write down the XRay file header with version 1, type FDR_LOG.
   //   2) Then we use the 'apply' member of the BufferQueue that's live, to
   //      ensure that at this point in time we write down the buffers that have
   //      been released (and marked "used") -- we dump the full buffer for now
   //      (fixed-sized) and let the tools reading the buffers deal with the data
XRayLogInitStatus fdrLoggingFinalize() XRAY_NEVER_INSTRUMENT {
   if (!__sanitizer::atomic_compare_exchange_strong(
           &LoggingStatus, &CurrentStatus,
           XRayLogInitStatus::XRAY_LOG_FINALIZING,
           __sanitizer::memory_order_release))
     return static_cast<XRayLogInitStatus>(CurrentStatus);
 
   // Do special things to make the log finalize itself, and not allow any more
   // operations to be performed until re-initialized.
-  BQ->finalize();
+  (*BQ)->finalize();
 
   __sanitizer::atomic_store(&LoggingStatus,
                             XRayLogInitStatus::XRAY_LOG_FINALIZED,
                             __sanitizer::memory_order_release);
   return XRayLogInitStatus::XRAY_LOG_FINALIZED;
 }
 
 XRayLogInitStatus fdrLoggingReset() XRAY_NEVER_INSTRUMENT {
   s32 CurrentStatus = XRayLogInitStatus::XRAY_LOG_FINALIZED;
   if (__sanitizer::atomic_compare_exchange_strong(
           &LoggingStatus, &CurrentStatus,
           XRayLogInitStatus::XRAY_LOG_INITIALIZED,
           __sanitizer::memory_order_release))
     return static_cast<XRayLogInitStatus>(CurrentStatus);
 
   // Release the in-memory buffer queue.
-  BQ.reset();
+  BQ->reset();
 
   // Spin until the flushing status is flushed.
   s32 CurrentFlushingStatus = XRayLogFlushStatus::XRAY_LOG_FLUSHED;
   while (__sanitizer::atomic_compare_exchange_weak(
       &LogFlushStatus, &CurrentFlushingStatus,
       XRayLogFlushStatus::XRAY_LOG_NOT_FLUSHING,
       __sanitizer::memory_order_release)) {
     if (CurrentFlushingStatus == XRayLogFlushStatus::XRAY_LOG_NOT_FLUSHING)
getTimestamp() XRAY_NEVER_INSTRUMENT {
   return std::make_tuple(TSC, CPU);
 }
 
 void fdrLoggingHandleArg0(int32_t FuncId,
                           XRayEntryType Entry) XRAY_NEVER_INSTRUMENT {
   auto TSC_CPU = getTimestamp();
   __xray_fdr_internal::processFunctionHook(FuncId, Entry, std::get<0>(TSC_CPU),
                                            std::get<1>(TSC_CPU), clock_gettime,
-                                           LoggingStatus, BQ);
+                                           LoggingStatus, *BQ);
 }
 
 void fdrLoggingHandleCustomEvent(void *Event,
                                  std::size_t EventSize) XRAY_NEVER_INSTRUMENT {
   using namespace __xray_fdr_internal;
   auto TSC_CPU = getTimestamp();
   auto &TSC = std::get<0>(TSC_CPU);
   auto &CPU = std::get<1>(TSC_CPU);
   thread_local bool Running = false;
   RecursionGuard Guard{Running};
   if (!Guard) {
     assert(Running && "RecursionGuard is buggy!");
     return;
   }
   if (EventSize > std::numeric_limits<int32_t>::max()) {
     using Empty = struct {};
     static Empty Once = [&] {
       Report("Event size too large = %zu ; > max = %d\n", EventSize,
              std::numeric_limits<int32_t>::max());
       return Empty();
     }();
     (void)Once;
   }
   int32_t ReducedEventSize = static_cast<int32_t>(EventSize);
-  if (!isLogInitializedAndReady(LocalBQ, TSC, CPU, clock_gettime))
+  if (!isLogInitializedAndReady(*LocalBQ, TSC, CPU, clock_gettime))
     return;
 
   // Here we need to prepare the log to handle:
   //   - The metadata record we're going to write. (16 bytes)
   //   - The additional data we're going to write. Currently, that's the size of
   //   the event we're going to dump into the log as free-form bytes.
   if (!prepareBuffer(clock_gettime, MetadataRecSize + EventSize)) {
     LocalBQ = nullptr;
if (!__sanitizer::atomic_compare_exchange_strong(
     return static_cast<XRayLogInitStatus>(CurrentStatus);
 
   {
     __sanitizer::SpinMutexLock Guard(&FDROptionsMutex);
     memcpy(&FDROptions, Options, OptionsSize);
   }
 
   bool Success = false;
-  BQ = std::make_shared<BufferQueue>(BufferSize, BufferMax, Success);
+  if (BQ == nullptr)
+    BQ = new std::shared_ptr<BufferQueue>();
+
+  *BQ = std::make_shared<BufferQueue>(BufferSize, BufferMax, Success);
   if (!Success) {
     Report("BufferQueue init failed.\n");
     return XRayLogInitStatus::XRAY_LOG_UNINITIALIZED;
   }
 
   // Install the actual handleArg0 handler after initialising the buffers.
   __xray_set_handler(fdrLoggingHandleArg0);
   __xray_set_customevent_handler(fdrLoggingHandleCustomEvent);

compiler-rt/trunk/lib/xray/xray_fdr_logging_impl.h

if (Buffers) {
       return;
     }
   }
 };
 
 // Make sure a thread that's ever called handleArg0 has a thread-local
 // live reference to the buffer queue for this particular instance of
 // FDRLogging, and that we're going to clean it up when the thread exits.
-thread_local std::shared_ptr<BufferQueue> LocalBQ = nullptr;
-thread_local ThreadExitBufferCleanup Cleanup(LocalBQ, Buffer);
+thread_local std::shared_ptr<BufferQueue>* LocalBQ =
+    new std::shared_ptr<BufferQueue>();
+thread_local ThreadExitBufferCleanup Cleanup(*LocalBQ, Buffer);
 
 class RecursionGuard {
   bool &Running;
   const bool Valid;
 
 public:
   explicit RecursionGuard(bool &R) : Running(R), Valid(!R) {
     if (Valid)
while (NumTailCalls > 0) {
     --NumTailCalls;
     RewindingRecordPtr -= FunctionRecSize;
     RewindingTSC -= ExpectedFunctionEntry.TSCDelta;
     RecordPtr -= 2 * FunctionRecSize;
     LastTSC = RewindingTSC;
   }
 }
 
-inline bool releaseThreadLocalBuffer(BufferQueue *BQ) {
-  auto EC = BQ->releaseBuffer(Buffer);
+inline bool releaseThreadLocalBuffer(BufferQueue &BQArg) {
+  auto EC = BQArg.releaseBuffer(Buffer);
   if (EC != BufferQueue::ErrorCode::Ok) {
     Report("Failed to release buffer at %p; error=%s\n", Buffer.Buffer,
            BufferQueue::getErrorString(EC));
     return false;
   }
   return true;
 }
 
 inline bool prepareBuffer(int (*wall_clock_reader)(clockid_t,
                                                    struct timespec *),
                           size_t MaxSize) XRAY_NEVER_INSTRUMENT {
   char *BufferStart = static_cast<char *>(Buffer.Buffer);
   if ((RecordPtr + MaxSize) > (BufferStart + Buffer.Size - MetadataRecSize)) {
     writeEOBMetadata();
-    if (!releaseThreadLocalBuffer(LocalBQ.get()))
+    if (!releaseThreadLocalBuffer(**LocalBQ))
       return false;
-    auto EC = LocalBQ->getBuffer(Buffer);
+    auto EC = (*LocalBQ)->getBuffer(Buffer);
     if (EC != BufferQueue::ErrorCode::Ok) {
       Report("Failed to acquire a buffer; error=%s\n",
              BufferQueue::getErrorString(EC));
       return false;
     }
     setupNewBuffer(wall_clock_reader);
   }
   return true;
 }
 
 inline bool isLogInitializedAndReady(
-    std::shared_ptr<BufferQueue> &LocalBQ, uint64_t TSC, unsigned char CPU,
+    std::shared_ptr<BufferQueue> &LBQ, uint64_t TSC, unsigned char CPU,
     int (*wall_clock_reader)(clockid_t,
                              struct timespec *)) XRAY_NEVER_INSTRUMENT {
   // Bail out right away if logging is not initialized yet.
   // We should take the opportunity to release the buffer though.
   auto Status = __sanitizer::atomic_load(&LoggingStatus,
                                          __sanitizer::memory_order_acquire);
   if (Status != XRayLogInitStatus::XRAY_LOG_INITIALIZED) {
     if (RecordPtr != nullptr &&
         (Status == XRayLogInitStatus::XRAY_LOG_FINALIZING ||
          Status == XRayLogInitStatus::XRAY_LOG_FINALIZED)) {
       writeEOBMetadata();
-      if (!releaseThreadLocalBuffer(LocalBQ.get()))
+      if (!releaseThreadLocalBuffer(*LBQ))
         return false;
       RecordPtr = nullptr;
-      LocalBQ = nullptr;
+      LBQ = nullptr;
       return false;
     }
     return false;
   }
 
-  if (!loggingInitialized(LoggingStatus) || LocalBQ->finalizing()) {
+  if (!loggingInitialized(LoggingStatus) || LBQ->finalizing()) {
     writeEOBMetadata();
-    if (!releaseThreadLocalBuffer(LocalBQ.get()))
+    if (!releaseThreadLocalBuffer(*LBQ))
       return false;
     RecordPtr = nullptr;
   }
 
   if (Buffer.Buffer == nullptr) {
-    auto EC = LocalBQ->getBuffer(Buffer);
+    auto EC = LBQ->getBuffer(Buffer);
     if (EC != BufferQueue::ErrorCode::Ok) {
       auto LS = __sanitizer::atomic_load(&LoggingStatus,
                                          __sanitizer::memory_order_acquire);
       if (LS != XRayLogInitStatus::XRAY_LOG_FINALIZING &&
           LS != XRayLogInitStatus::XRAY_LOG_FINALIZED)
         Report("Failed to acquire a buffer; error=%s\n",
                BufferQueue::getErrorString(EC));
       return false;
inline bool isLogInitializedAndReady(
 
   return true;
 } // namespace __xray_fdr_internal
 
 inline void endBufferIfFull() XRAY_NEVER_INSTRUMENT {
   auto BufferStart = static_cast<char *>(Buffer.Buffer);
   if ((RecordPtr + MetadataRecSize) - BufferStart == MetadataRecSize) {
     writeEOBMetadata();
-    if (!releaseThreadLocalBuffer(LocalBQ.get()))
+    if (!releaseThreadLocalBuffer(**LocalBQ))
       return;
     RecordPtr = nullptr;
   }
 }
 
 inline void processFunctionHook(
     int32_t FuncId, XRayEntryType Entry, uint64_t TSC, unsigned char CPU,
     int (*wall_clock_reader)(clockid_t, struct timespec *),
     __sanitizer::atomic_sint32_t &LoggingStatus,
     const std::shared_ptr<BufferQueue> &BQ) XRAY_NEVER_INSTRUMENT {
   // Prevent signal handler recursion, so in case we're already in a log writing
   // mode and the signal handler comes in (and is also instrumented) then we
   // don't want to be clobbering potentially partial writes already happening in
   // the thread. We use a simple thread_local latch to only allow one on-going
   // handleArg0 to happen at any given time.
   thread_local bool Running = false;
   RecursionGuard Guard{Running};
   if (!Guard) {
     assert(Running == true && "RecursionGuard is buggy!");
     return;
   }
 
   // In case the reference has been cleaned up before, we make sure we
   // initialize it to the provided BufferQueue.
-  if (LocalBQ == nullptr)
-    LocalBQ = BQ;
+  if ((*LocalBQ) == nullptr)
+    *LocalBQ = BQ;
 
-  if (!isLogInitializedAndReady(LocalBQ, TSC, CPU, wall_clock_reader))
+  if (!isLogInitializedAndReady(*LocalBQ, TSC, CPU, wall_clock_reader))
     return;
 
   // Before we go setting up writing new function entries, we need to be really
   // careful about the pointer math we're doing. This means we need to ensure
   // that the record we are about to write is going to fit into the buffer,
   // without overflowing the buffer.
   //
   // To do this properly, we use the following assumptions:
inline void processFunctionHook(
   //
   // So the math we need to do is to determine whether writing 24 bytes past the
   // current pointer leaves us with enough bytes to write the EOB
   // MetadataRecord. If we don't have enough space after writing as much as 24
   // bytes in the end of the buffer, we need to write out the EOB, get a new
   // Buffer, set it up properly before doing any further writing.
   //
   if (!prepareBuffer(wall_clock_reader, FunctionRecSize + MetadataRecSize)) {
-    LocalBQ = nullptr;
+    *LocalBQ = nullptr;
     return;
   }
 
   // By this point, we are now ready to write at most 24 bytes (one metadata
   // record and one function record).
   assert((RecordPtr + (MetadataRecSize + FunctionRecSize)) -
                  static_cast<char *>(Buffer.Buffer) >=
              static_cast<ptrdiff_t>(MetadataRecSize) &&

compiler-rt/trunk/lib/xray/xray_inmemory_log.cc

 //
 // Implementation of a simple in-memory log of XRay events. This defines a
 // logging function that's compatible with the XRay handler interface, and
 // routines for exporting data to files.
 //
 //===----------------------------------------------------------------------===//
 
 #include <cassert>
+#include <errno.h>
 #include <fcntl.h>
-#include <mutex>
 #include <sys/stat.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
-#include <thread>
+#include <time.h>
 #include <unistd.h>
 
 #include "sanitizer_common/sanitizer_libc.h"
 #include "xray/xray_records.h"
 #include "xray_defs.h"
 #include "xray_flags.h"
 #include "xray_interface_internal.h"
 #include "xray_tsc.h"
 #include "xray_utils.h"
 
 // __xray_InMemoryRawLog will use a thread-local aligned buffer capped to a
 // certain size (32kb by default) and use it as if it were a circular buffer for
 // events. We store simple fixed-sized entries in the log for external analysis.
 
 extern "C" {
 void __xray_InMemoryRawLog(int32_t FuncId,
                            XRayEntryType Type) XRAY_NEVER_INSTRUMENT;
 }
 
 namespace __xray {
 
-std::mutex LogMutex;
+__sanitizer::SpinMutex LogMutex;
 
 class ThreadExitFlusher {
   int Fd;
   XRayRecord *Start;
   size_t &Offset;
 
 public:
   explicit ThreadExitFlusher(int Fd, XRayRecord *Start,
                              size_t &Offset) XRAY_NEVER_INSTRUMENT
       : Fd(Fd),
         Start(Start),
         Offset(Offset) {}
 
   ~ThreadExitFlusher() XRAY_NEVER_INSTRUMENT {
-    std::lock_guard<std::mutex> L(LogMutex);
+    __sanitizer::SpinMutexLock L(&LogMutex);
     if (Fd > 0 && Start != nullptr) {
       retryingWriteAll(Fd, reinterpret_cast<char *>(Start),
                        reinterpret_cast<char *>(Start + Offset));
       // Because this thread's exit could be the last one trying to write to the
       // file and that we're not able to close out the file properly, we sync
       // instead and hope that the pending writes are flushed as the thread
       // exits.
       fsync(Fd);
void __xray_InMemoryRawLog(int32_t FuncId, XRayEntryType Type,
   auto &R = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer)[Offset];
   R.RecordType = RecordTypes::NORMAL;
   R.TSC = ReadTSC(R.CPU);
   R.TId = TId;
   R.Type = Type;
   R.FuncId = FuncId;
   ++Offset;
   if (Offset == BuffLen) {
-    std::lock_guard<std::mutex> L(LogMutex);
+    __sanitizer::SpinMutexLock L(&LogMutex);
     auto RecordBuffer = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer);
     retryingWriteAll(Fd, reinterpret_cast<char *>(RecordBuffer),
                      reinterpret_cast<char *>(RecordBuffer + Offset));
     Offset = 0;
   }
 }
 
 void __xray_InMemoryRawLogRealTSC(int32_t FuncId,