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

[Dean Michael Berris via Phabricator via llvm-commits]

dberris 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();
----------------
kpw wrote:
> 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.
Good point. Updated doc and implementation to allow for multiple calls (and documenting the semantics of that).


================
Comment at: lib/xray/xray_init.cc:50-52
+  if (!__sanitizer::atomic_load(&XRayInitialized,
+                                __sanitizer::memory_order_acquire))
+    initializeFlags();
----------------
kpw wrote:
> 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.
Good point! Done, PTAL.


https://reviews.llvm.org/D36080