[XRay][compiler-rt] Document and update the XRay Logging API

[dberris]

Summary

In this patch we document the requirements for implementations that want
to install handlers for the dynamically-controlled XRay "framework".
This clarifies what the expectations are for implementations that
want to install their handlers using this API (similar to how the FDR
logging implementation does so). It also gives users some guarantees on
semantics for the APIs.

If all goes well, users can decide to use the XRay APIs to control the
tracing/logging at the application level, without having to depend on
implementation details of the installed logging implementation. This
lets users choose the implementation that comes with compiler-rt, or
potentially multiple other implementations that use the same APIs.

We also add one convenience function (__xray_remove_log_impl()) for
explicitly removing the currently installed log implementation.

[kpw]

Thanks for thinking this through Dean. Here's a few suggestions I'd like to hear your thoughts on.

include/xray/xray_log_interface.h

15 ↗

(On Diff #96866)

Is this comment block intended to show up in Doxygen? I think you'll want an additional slash for these notes.

62 ↗

(On Diff #96866)

I suggest a brief word of caution that before patching again, users should call __xray_log_remove_impl and reinitialize. I can see it being a common error that once logs are finalized and/or flushed users think it's OK to patch directly.

For some implementations, it would nice to be able to skip reinitialization. If buffers are already allocated it's a shame to have to remove_impl and reallocate them. Perhaps we should add a __xray_log_reset_impl that by default calls remove_impl and initializes with the same arguments. Log implementations could provide a function ptr to short circuit with appropriate state management.

138–140 ↗

(On Diff #96866)

Indicating where readers can find an arg1 installation could be helpful. (xray_interface.h or pointer to fdr logging log_init)

154–157 ↗

(On Diff #96866)

It's unclear whether the library prevents this from being called from any particular states.

Let's explicitly point out when this is guaranteed to be safe (no implementation installed and unitialized implementation?), when it is implementation defined, and when it is explicitly disallowed regardless of implementation.

I think it is defined by the implementations when this would be safe to call for an initialized log (or possibly a finalized log).

I can imagine some logging implementations that would want to chain together interceptions. E.g. once I see a request that seeds data for a particular id, install a tracer for subsequent operations that affect the data seeded for the id under inspection.

162–164 ↗

(On Diff #96866)

Same comment as for set_impl.

Also, is this a no-op or an error if no implementation is installed?

lib/xray/xray_log_interface.cc

30–38 ↗

(On Diff #96866)

Suspicious that the code ignores the return values from xray/xray_interface.h's xray_set_handler" and xray_remove_handler.

Looks like these have to do with checking that xray_init was called.

Should xray_set_log_impl return something other than void? Should implementations call xray_init if necessary?

I'm in favor returning an error rather than hidden runtime cost.

(P.S. lots of underscores missing from function names above. Phabricator was doing markdown stuff with them)

[dberris]

Address review comments.

include/xray/xray_log_interface.h

15 ↗

(On Diff #96866)

Wow, good catch -- yes, otherwise this would be all for nought! :)

62 ↗

(On Diff #96866)

Note, that __xray_log_remove_impl() doesn't need to be called at all -- some implementations can be re-initialized just by users directly calling __xray_log_init(...) again. __xray_log_remove_impl() is a "nuclear" option that uninstalls the implementation.

154–157 ↗

(On Diff #96866)

Good point. I've extended this to say when it's safe to call and when it's implementation defined.

162–164 ↗

(On Diff #96866)

I'm not sure whether mentioning whether it's a no-op is appropriate at this level -- it may be an implementation detail. The side-effect though would be that if there was something installed, then there's no longer something installed. :)

lib/xray/xray_log_interface.cc

30–38 ↗

(On Diff #96866)

I thought about whether we should return something for setting the log implementation (or removing it). Because XRay initialisation happens pre-main (see xray_init) and is linked for all xray-instrumented binaries, we let users discover the error when patching.

Semantically, changing the log implementation is not something that can "fail" -- you can set the logging implementation but not be able to patch sleds because there is no instrumentation map. Those two are completely different axes.

Do you have a suggestion on how/where that documentation should show up?

[kpw]

include/xray/xray_log_interface.h

62 ↗

(On Diff #96866)

I missed the state transition from finalized to initializing via xray_log_init. Having that transition serves the same purpose as the proposed xray_log_reset_impl without adding another method.

We might want to point out that this may block on the completion of a flush in the case that the log flush status is XRAY_LOG_FLUSHING.

lib/xray/xray_log_interface.cc

30–38 ↗

(On Diff #96866)

I see. Since xray_init is in .preinit_array, I have no problems with swallowing the error codes.

On a related note, do we have to revisit the patch_premain flag? AFAICT, it doesn't allow users to choose their implementation, and more importantly, it doesn't call __xray_log_init. We would have to do something clever to define an interface to have this flag choose the logging implementation. Maybe we could reserve a section header where we can check for user defined function pointers that set up their implementation and if we don't find anything, install and initialize the default.

[dberris]

Thanks @kpw -- landing now, we can iterate on documenting more if things become unclear/confusing.

include/xray/xray_log_interface.h

62 ↗

(On Diff #96866)

Yes, we don't need __xray_log_reset_impl() anymore. I thought about that in an early iteration of this API, but decided it's too hard to keep the invariants for that which calling __xray_log_init() already achieves.

__xray_log_remove_impl() does something very different though. It literally removes the implementation that's currently installed.

lib/xray/xray_log_interface.cc

30–38 ↗

(On Diff #96866)

We let users choose the implementation for patch_premain by letting users define either xray_naive_log= or xray_fdr_log=. This API allows users to install an implementation that can't be installed through the flags. It's also for controlling an implementation explicitly (init, finalize, flush, repeat).

We might want to consider an explicit registration/initialisation mechanism, but users can already do this at init-time, by doing a global initializer that runs before main() but after premain.

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

 //===-- xray_log_interface.h ----------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of XRay, a function call tracing system.
 //
 // APIs for installing a new logging implementation.
+//
 //===----------------------------------------------------------------------===//
+///
+/// XRay allows users to implement their own logging handlers and install them
+/// to replace the default runtime-controllable implementation that comes with
+/// compiler-rt/xray. The "flight data recorder" (FDR) mode implementation uses
+/// this API to install itself in an XRay-enabled binary. See
+/// compiler-rt/lib/xray_fdr_logging.{h,cc} for details of that implementation.
+///
+/// The high-level usage pattern for these APIs look like the following:
+///
+///   // Before we try initializing the log implementation, we must set it as
+///   // the log implementation. We provide the function pointers that define
+///   // the various initialization, finalization, and other pluggable hooks
+///   // that we need.
+///   __xray_set_log_impl({...});
+///
+///   // Once that's done, we can now initialize the implementation. Each
+///   // implementation has a chance to let users customize the implementation
+///   // with a struct that their implementation supports. Roughly this might
+///   // look like:
+///   MyImplementationOptions opts;
+///   opts.enable_feature = true;
+///   ...
+///   auto init_status = __xray_log_init(
+///       BufferSize, MaxBuffers, &opts, sizeof opts);
+///   if (init_status != XRayLogInitStatus::XRAY_LOG_INITIALIZED) {
+///     // deal with the error here, if there is one.
+///   }
+///
+///   // When the log implementation has had the chance to initialize, we can
+///   // now patch the sleds.
+///   auto patch_status = __xray_patch();
+///   if (patch_status != XRayPatchingStatus::SUCCESS) {
+///     // deal with the error here, if it is an error.
+///   }
+///
+///   // If we want to stop the implementation, we can then finalize it (before
+///   // optionally flushing the log).
+///   auto fin_status = __xray_log_finalize();
+///   if (fin_status != XRayLogInitStatus::XRAY_LOG_FINALIZED) {
+///     // deal with the error here, if it is an error.
+///   }
+///
+///   // We can optionally wait before flushing the log to give other threads a
+///   // chance to see that the implementation is already finalized. Also, at
+///   // this point we can optionally unpatch the sleds to reduce overheads at
+///   // runtime.
+///   auto unpatch_status = __xray_unpatch();
+///   if (unpatch_status != XRayPatchingStatus::SUCCESS) {
+//      // deal with the error here, if it is an error.
+//    }
+///
+///   // If there are logs or data to be flushed somewhere, we can do so only
+///   // after we've finalized the log. Some implementations may not actually
+///   // have anything to log (it might keep the data in memory, or periodically
+///   // be logging the data anyway).
+///   auto flush_status = __xray_log_flushLog();
+///   if (flush_status != XRayLogFlushStatus::XRAY_LOG_FLUSHED) {
+///     // deal with the error here, if it is an error.
+///   }
+///
+///
+/// NOTE: Before calling __xray_patch() again, consider re-initializing the
+/// implementation first. Some implementations might stay in an "off" state when
+/// they are finalized, while some might be in an invalid/unknown state.
+///
 #ifndef XRAY_XRAY_LOG_INTERFACE_H
 #define XRAY_XRAY_LOG_INTERFACE_H
 
 #include "xray/xray_interface.h"
 #include <stddef.h>
 
 extern "C" {
 
+/// This enum defines the valid states in which the logging implementation can
+/// be at.
 enum XRayLogInitStatus {
+  /// The default state is uninitialized, and in case there were errors in the
+  /// initialization, the implementation MUST return XRAY_LOG_UNINITIALIZED.
   XRAY_LOG_UNINITIALIZED = 0,
+
+  /// Some implementations support multi-stage init (or asynchronous init), and
+  /// may return XRAY_LOG_INITIALIZING to signal callers of the API that
+  /// there's an ongoing initialization routine running. This allows
+  /// implementations to support concurrent threads attempting to initialize,
+  /// while only signalling success in one.
   XRAY_LOG_INITIALIZING = 1,
+
+  /// When an implementation is done initializing, it MUST return
+  /// XRAY_LOG_INITIALIZED. When users call `__xray_patch()`, they are
+  /// guaranteed that the implementation installed with
+  /// `__xray_set_log_impl(...)` has been initialized.
   XRAY_LOG_INITIALIZED = 2,
+
+  /// Some implementations might support multi-stage finalization (or
+  /// asynchronous finalization), and may return XRAY_LOG_FINALIZING to signal
+  /// callers of the API that there's an ongoing finalization routine running.
+  /// This allows implementations to support concurrent threads attempting to
+  /// finalize, while only signalling success/completion in one.
   XRAY_LOG_FINALIZING = 3,
+
+  /// When an implementation is done finalizing, it MUST return
+  /// XRAY_LOG_FINALIZED. It is up to the implementation to determine what the
+  /// semantics of a finalized implementation is. Some implementations might
+  /// allow re-initialization once the log is finalized, while some might always
+  /// be on (and that finalization is a no-op).
   XRAY_LOG_FINALIZED = 4,
 };
 
+/// This enum allows an implementation to signal log flushing operations via
+/// `__xray_log_flushLog()`, and the state of flushing the log.
 enum XRayLogFlushStatus {
   XRAY_LOG_NOT_FLUSHING = 0,
   XRAY_LOG_FLUSHING = 1,
   XRAY_LOG_FLUSHED = 2,
 };
 
+/// A valid XRay logging implementation MUST provide all of the function
+/// pointers in XRayLogImpl when being installed through `__xray_set_log_impl`.
+/// To be precise, ALL the functions pointers MUST NOT be nullptr.
 struct XRayLogImpl {
+  /// The log initialization routine provided by the implementation, always
+  /// provided with the following parameters:
+  ///
+  ///   - buffer size
+  ///   - maximum number of buffers
+  ///   - a pointer to an argument struct that the implementation MUST handle
+  ///   - the size of the argument struct
+  ///
+  /// See XRayLogInitStatus for details on what the implementation MUST return
+  /// when called.
+  ///
+  /// If the implementation needs to install handlers aside from the 0-argument
+  /// function call handler, it MUST do so in this initialization handler.
+  ///
+  /// See xray_interface.h for available handler installation routines.
   XRayLogInitStatus (*log_init)(size_t, size_t, void *, size_t);
+
+  /// The log finalization routine provided by the implementation.
+  ///
+  /// See XRayLogInitStatus for details on what the implementation MUST return
+  /// when called.
   XRayLogInitStatus (*log_finalize)();
+
+  /// The 0-argument function call handler. XRay logging implementations MUST
+  /// always have a handler for function entry and exit events. In case the
+  /// implementation wants to support arg1 (or other future extensions to XRay
+  /// logging) those MUST be installed by the installed 'log_init' handler.
   void (*handle_arg0)(int32_t, XRayEntryType);
+
+  /// The log implementation provided routine for when __xray_log_flushLog() is
+  /// called.
+  ///
+  /// See XRayLogFlushStatus for details on what the implementation MUST return
+  /// when called.
   XRayLogFlushStatus (*flush_log)();
 };
 
+/// This function installs a new logging implementation that XRay will use. In
+/// case there are any nullptr members in Impl, XRay will *uninstall any
+/// existing implementations*. It does NOT patch the instrumentation sleds.
+///
+/// NOTE: This function does NOT attempt to finalize the currently installed
+/// implementation. Use with caution.
+///
+/// It is guaranteed safe to call this function in the following states:
+///
+///   - When the implementation is UNINITIALIZED.
+///   - When the implementation is FINALIZED.
+///   - When there is no current implementation installed.
+///
+/// It is logging implementation defined what happens when this function is
+/// called while in any other states.
 void __xray_set_log_impl(XRayLogImpl Impl);
+
+/// This function removes the currently installed implementation. It will also
+/// uninstall any handlers that have been previously installed. It does NOT
+/// unpatch the instrumentation sleds.
+///
+/// NOTE: This function does NOT attempt to finalize the currently installed
+/// implementation. Use with caution.
+///
+/// It is guaranteed safe to call this function in the following states:
+///
+///   - When the implementation is UNINITIALIZED.
+///   - When the implementation is FINALIZED.
+///   - When there is no current implementation installed.
+///
+/// It is logging implementation defined what happens when this function is
+/// called while in any other states.
+void __xray_remove_log_impl();
+
+/// Invokes the installed implementation initialization routine. See
+/// XRayLogInitStatus for what the return values mean.
 XRayLogInitStatus __xray_log_init(size_t BufferSize, size_t MaxBuffers,
                                   void *Args, size_t ArgsSize);
+
+/// Invokes the installed implementation finalization routine. See
+/// XRayLogInitStatus for what the return values mean.
 XRayLogInitStatus __xray_log_finalize();
+
+/// Invokes the install implementation log flushing routine. See
+/// XRayLogFlushStatus for what the return values mean.
 XRayLogFlushStatus __xray_log_flushLog();
 
 } // extern "C"
 
 namespace __xray {
+
 // Options used by the LLVM XRay FDR implementation.
 struct FDRLoggingOptions {
   bool ReportErrors = false;
   int Fd = -1;
 };
 
 } // namespace __xray
 
 #endif // XRAY_XRAY_LOG_INTERFACE_H

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

 __sanitizer::SpinMutex XRayImplMutex;
 std::unique_ptr<XRayLogImpl> GlobalXRayImpl;
 
 void __xray_set_log_impl(XRayLogImpl Impl) XRAY_NEVER_INSTRUMENT {
   if (Impl.log_init == nullptr || Impl.log_finalize == nullptr ||
       Impl.handle_arg0 == nullptr || Impl.flush_log == nullptr) {
     __sanitizer::SpinMutexLock Guard(&XRayImplMutex);
     GlobalXRayImpl.reset();
+    __xray_remove_handler();
+    __xray_remove_handler_arg1();
     return;
   }
 
   __sanitizer::SpinMutexLock Guard(&XRayImplMutex);
   GlobalXRayImpl.reset(new XRayLogImpl);
   *GlobalXRayImpl = Impl;
+  __xray_set_handler(Impl.handle_arg0);
+}
+
+void __xray_remove_log_impl() XRAY_NEVER_INSTRUMENT {
+  __sanitizer::SpinMutexLock Guard(&XRayImplMutex);
+  GlobalXRayImpl.reset();
+  __xray_remove_handler();
+  __xray_remove_handler_arg1();
 }
 
 XRayLogInitStatus __xray_log_init(size_t BufferSize, size_t MaxBuffers,
                                   void *Args,
                                   size_t ArgsSize) XRAY_NEVER_INSTRUMENT {
   __sanitizer::SpinMutexLock Guard(&XRayImplMutex);
   if (!GlobalXRayImpl)
     return XRayLogInitStatus::XRAY_LOG_UNINITIALIZED;