[PATCH] D36080: [XRay][compiler-rt] Allow for building the XRay runtime without PREINIT initialization.

[Keith via Phabricator via llvm-commits]

kpw added inline comments.


================
Comment at: include/xray/xray_interface.h:109-113
+/// Initialize the required XRay data structures. This is useful in cases where
+/// users want to control precisely when the XRay instrumentation data
+/// structures are initialized, for example when the XRay library is built with
+/// the XRAY_NO_PREINIT preprocessor definition.
+extern void __xray_init();
----------------
Are callers expected to synchronize before calling __xray_init()? Is it even legal to call this function more than once? This should be documented in the header.


================
Comment at: lib/xray/xray_init.cc:50-52
+  if (!__sanitizer::atomic_load(&XRayInitialized,
+                                __sanitizer::memory_order_acquire))
+    initializeFlags();
----------------
Is it safe for initializeFlags() to be called twice? Called by different callers at the same time? Depending on what you expect of callers invoking __xray_init, these things can happen.

I suggest that we add an atomic_uint8_t XRayFlagsInitialized above and check that within a XrayFlagInitMutex, then it will be safe for callers to make multiple unsynchronized invocations of __xray_init(). 

I will also suggest early return if this atomic_load is true. As written, this just wraps the initializeFlags() call. XRayInstrMap is still reloaded even if XRayInitialized is true.


https://reviews.llvm.org/D36080