[XRay][compiler-rt] Support string-based config for FDR mode
authorDean Michael Berris <dberris@google.com>
Fri, 4 May 2018 06:13:35 +0000 (06:13 +0000)
committerDean Michael Berris <dberris@google.com>
Fri, 4 May 2018 06:13:35 +0000 (06:13 +0000)
Summary:
In this chage we add support for the string-based configuration
mechanism for configuring FDR mode.

We deprecate most of the `xray_fdr_log_*` flags that are set with the
`XRAY_OPTIONS` environment variable. Instead we make the FDR
implementation take defaults from the `XRAY_FDR_OPTIONS` environment
variable, and use the flags defined in `xray_fdr_flags.{h,cc,inc}` for
the options we support.

This change addresses http://llvm.org/PR36790.

Depends on D46173.

Reviewers: eizan, pelikan, kpw, echristo

Subscribers: llvm-commits, mgorny

Differential Revision: https://reviews.llvm.org/D46174

llvm-svn: 331506

compiler-rt/lib/xray/CMakeLists.txt
compiler-rt/lib/xray/xray_fdr_flags.cc [new file with mode: 0644]
compiler-rt/lib/xray/xray_fdr_flags.h [new file with mode: 0644]
compiler-rt/lib/xray/xray_fdr_flags.inc [new file with mode: 0644]
compiler-rt/lib/xray/xray_fdr_logging.cc
compiler-rt/lib/xray/xray_flags.cc
compiler-rt/lib/xray/xray_flags.h
compiler-rt/lib/xray/xray_flags.inc
compiler-rt/test/xray/TestCases/Posix/fdr-mode.cc

index 22aa4d3..58b5665 100644 (file)
@@ -10,6 +10,7 @@ set(XRAY_SOURCES
 
 # Implementation files for all XRay modes.
 set(XRAY_FDR_MODE_SOURCES
+    xray_fdr_flags.cc
     xray_buffer_queue.cc
     xray_fdr_logging.cc)
 
diff --git a/compiler-rt/lib/xray/xray_fdr_flags.cc b/compiler-rt/lib/xray/xray_fdr_flags.cc
new file mode 100644 (file)
index 0000000..a14851b
--- /dev/null
@@ -0,0 +1,48 @@
+//===-- xray_fdr_flags.cc ---------------------------------------*- C++ -*-===//
+//
+//                     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 dynamic runtime instrumentation system.
+//
+// XRay FDR flag parsing logic.
+//===----------------------------------------------------------------------===//
+
+#include "xray_fdr_flags.h"
+#include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_flag_parser.h"
+#include "sanitizer_common/sanitizer_libc.h"
+#include "xray_defs.h"
+
+using namespace __sanitizer;
+
+namespace __xray {
+
+FDRFlags xray_fdr_flags_dont_use_directly; // use via fdrFlags().
+
+void FDRFlags::setDefaults() XRAY_NEVER_INSTRUMENT {
+#define XRAY_FLAG(Type, Name, DefaultValue, Description) Name = DefaultValue;
+#include "xray_fdr_flags.inc"
+#undef XRAY_FLAG
+}
+
+void registerXRayFDRFlags(FlagParser *P, FDRFlags *F) XRAY_NEVER_INSTRUMENT {
+#define XRAY_FLAG(Type, Name, DefaultValue, Description)                       \
+  RegisterFlag(P, #Name, Description, &F->Name);
+#include "xray_fdr_flags.inc"
+#undef XRAY_FLAG
+}
+
+const char *useCompilerDefinedFDRFlags() XRAY_NEVER_INSTRUMENT {
+#ifdef XRAY_FDR_OPTIONS
+  return SANITIZER_STRINGIFY(XRAY_FDR_OPTIONS);
+#else
+  return "";
+#endif
+}
+
+} // namespace __xray
diff --git a/compiler-rt/lib/xray/xray_fdr_flags.h b/compiler-rt/lib/xray/xray_fdr_flags.h
new file mode 100644 (file)
index 0000000..9c953f1
--- /dev/null
@@ -0,0 +1,38 @@
+//===-- xray_fdr_flags.h ---------------------------------------*- C++ -*-===//
+//
+//                     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 dynamic runtime instrumentation system.
+//
+// This file defines the flags for the flight-data-recorder mode implementation.
+//
+//===----------------------------------------------------------------------===//
+#ifndef XRAY_FDR_FLAGS_H
+#define XRAY_FDR_FLAGS_H
+
+#include "sanitizer_common/sanitizer_flag_parser.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
+
+namespace __xray {
+
+struct FDRFlags {
+#define XRAY_FLAG(Type, Name, DefaultValue, Description) Type Name;
+#include "xray_fdr_flags.inc"
+#undef XRAY_FLAG
+
+  void setDefaults();
+};
+
+extern FDRFlags xray_fdr_flags_dont_use_directly;
+extern void registerXRayFDRFlags(FlagParser *P, FDRFlags *F);
+const char *useCompilerDefinedFDRFlags();
+inline FDRFlags *fdrFlags() { return &xray_fdr_flags_dont_use_directly; }
+
+} // namespace __xray
+
+#endif // XRAY_FDR_FLAGS_H
diff --git a/compiler-rt/lib/xray/xray_fdr_flags.inc b/compiler-rt/lib/xray/xray_fdr_flags.inc
new file mode 100644 (file)
index 0000000..7815738
--- /dev/null
@@ -0,0 +1,27 @@
+//===-- xray_fdr_flags.inc --------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// XRay FDR Mode runtime flags.
+//
+//===----------------------------------------------------------------------===//
+#ifndef XRAY_FLAG
+#error "Define XRAY_FLAG prior to including this file!"
+#endif
+
+// FDR (Flight Data Recorder) Mode logging options.
+XRAY_FLAG(int, func_duration_threshold_us, 5,
+          "FDR logging will try to skip functions that execute for fewer "
+          "microseconds than this threshold.")
+XRAY_FLAG(int, grace_period_ms, 100,
+          "FDR logging will wait this much time in milliseconds before "
+          "actually flushing the log; this gives a chance for threads to "
+          "notice that the log has been finalized and clean up.")
+XRAY_FLAG(int, buffer_size, 16384,
+          "Size of buffers in the circular buffer queue.")
+XRAY_FLAG(int, buffer_max, 100, "Maximum number of buffers in the queue.")
index fcb33fc..d666733 100644 (file)
@@ -27,6 +27,7 @@
 #include "xray/xray_records.h"
 #include "xray_buffer_queue.h"
 #include "xray_defs.h"
+#include "xray_fdr_flags.h"
 #include "xray_fdr_logging_impl.h"
 #include "xray_flags.h"
 #include "xray_tsc.h"
@@ -72,7 +73,7 @@ XRayLogFlushStatus fdrLoggingFlush() XRAY_NEVER_INSTRUMENT {
 
   // We wait a number of milliseconds to allow threads to see that we've
   // finalised before attempting to flush the log.
-  __sanitizer::SleepForMillis(flags()->xray_fdr_log_grace_period_ms);
+  __sanitizer::SleepForMillis(fdrFlags()->grace_period_ms);
 
   // We write out the file in the following format:
   //
@@ -83,8 +84,12 @@ XRayLogFlushStatus fdrLoggingFlush() XRAY_NEVER_INSTRUMENT {
   //      (fixed-sized) and let the tools reading the buffers deal with the data
   //      afterwards.
   //
+  // FIXME: Support the case for letting users handle the data through
+  // __xray_process_buffers(...) and provide an option to skip writing files.
   int Fd = -1;
   {
+    // FIXME: Remove this section of the code, when we remove the struct-based
+    // configuration API.
     __sanitizer::SpinMutexLock Guard(&FDROptionsMutex);
     Fd = FDROptions.Fd;
   }
@@ -168,33 +173,6 @@ XRayLogInitStatus fdrLoggingFinalize() XRAY_NEVER_INSTRUMENT {
   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.
-  delete BQ;
-  BQ = nullptr;
-
-  // 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)
-      break;
-    CurrentFlushingStatus = XRayLogFlushStatus::XRAY_LOG_FLUSHED;
-  }
-
-  // At this point, we know that the status is flushed, and that we can assume
-  return XRayLogInitStatus::XRAY_LOG_UNINITIALIZED;
-}
-
 struct TSCAndCPU {
   uint64_t TSC = 0;
   unsigned char CPU = 0;
@@ -346,16 +324,12 @@ void fdrLoggingHandleTypedEvent(
   endBufferIfFull();
 }
 
-XRayLogInitStatus fdrLoggingInit(std::size_t BufferSize, std::size_t BufferMax,
+XRayLogInitStatus fdrLoggingInit(size_t BufferSize, size_t BufferMax,
                                  void *Options,
                                  size_t OptionsSize) XRAY_NEVER_INSTRUMENT {
-  if (OptionsSize != sizeof(FDRLoggingOptions)) {
-    if (__sanitizer::Verbosity())
-      Report("Cannot initialize FDR logging; wrong size for options: %d\n",
-             OptionsSize);
-    return static_cast<XRayLogInitStatus>(__sanitizer::atomic_load(
-        &LoggingStatus, __sanitizer::memory_order_acquire));
-  }
+  if (Options == nullptr)
+    return XRayLogInitStatus::XRAY_LOG_UNINITIALIZED;
+
   s32 CurrentStatus = XRayLogInitStatus::XRAY_LOG_UNINITIALIZED;
   if (!__sanitizer::atomic_compare_exchange_strong(
           &LoggingStatus, &CurrentStatus,
@@ -366,7 +340,56 @@ XRayLogInitStatus fdrLoggingInit(std::size_t BufferSize, std::size_t BufferMax,
     return static_cast<XRayLogInitStatus>(CurrentStatus);
   }
 
-  {
+  // Because of __xray_log_init_mode(...) which guarantees that this will be
+  // called with BufferSize == 0 and BufferMax == 0 we parse the configuration
+  // provided in the Options pointer as a string instead.
+  if (BufferSize == 0 && BufferMax == 0) {
+    if (__sanitizer::Verbosity())
+      Report("Initializing FDR mode with options: %s\n",
+             static_cast<const char *>(Options));
+
+    // TODO: Factor out the flags specific to the FDR mode implementation. For
+    // now, use the global/single definition of the flags, since the FDR mode
+    // flags are already defined there.
+    FlagParser FDRParser;
+    FDRFlags FDRFlags;
+    registerXRayFDRFlags(&FDRParser, &FDRFlags);
+    FDRFlags.setDefaults();
+
+    // Override first from the general XRAY_DEFAULT_OPTIONS compiler-provided
+    // options until we migrate everyone to use the XRAY_FDR_OPTIONS
+    // compiler-provided options.
+    FDRParser.ParseString(useCompilerDefinedFlags());
+    FDRParser.ParseString(useCompilerDefinedFDRFlags());
+    auto *EnvOpts = GetEnv("XRAY_FDR_OPTIONS");
+    if (EnvOpts == nullptr)
+      EnvOpts = "";
+    FDRParser.ParseString(EnvOpts);
+    FDRParser.ParseString(static_cast<const char *>(Options));
+    *fdrFlags() = FDRFlags;
+
+    BufferSize = FDRFlags.buffer_size;
+    BufferMax = FDRFlags.buffer_max;
+
+    // FIXME: Remove this when we fully remove the deprecated flags.
+    if (internal_strlen(EnvOpts) != 0) {
+      flags()->xray_fdr_log_func_duration_threshold_us =
+          FDRFlags.func_duration_threshold_us;
+      flags()->xray_fdr_log_grace_period_ms = FDRFlags.grace_period_ms;
+    }
+  } else if (OptionsSize != sizeof(FDRLoggingOptions)) {
+    // FIXME: This is deprecated, and should really be removed.
+    // At this point we use the flag parser specific to the FDR mode
+    // implementation.
+    if (__sanitizer::Verbosity())
+      Report("Cannot initialize FDR logging; wrong size for options: %d\n",
+             OptionsSize);
+    return static_cast<XRayLogInitStatus>(__sanitizer::atomic_load(
+        &LoggingStatus, __sanitizer::memory_order_acquire));
+  } else {
+    if (__sanitizer::Verbosity())
+      Report("XRay FDR: struct-based init is deprecated, please use "
+             "string-based configuration instead.\n");
     __sanitizer::SpinMutexLock Guard(&FDROptionsMutex);
     memcpy(&FDROptions, Options, OptionsSize);
   }
index ced1a52..b50b686 100644 (file)
@@ -30,7 +30,7 @@ void Flags::setDefaults() XRAY_NEVER_INSTRUMENT {
 #undef XRAY_FLAG
 }
 
-static void registerXRayFlags(FlagParser *P, Flags *F) XRAY_NEVER_INSTRUMENT {
+void registerXRayFlags(FlagParser *P, Flags *F) XRAY_NEVER_INSTRUMENT {
 #define XRAY_FLAG(Type, Name, DefaultValue, Description)                       \
   RegisterFlag(P, #Name, Description, &F->Name);
 #include "xray_flags.inc"
@@ -42,12 +42,13 @@ static void registerXRayFlags(FlagParser *P, Flags *F) XRAY_NEVER_INSTRUMENT {
 // options that control XRay. This means users/deployments can tweak the
 // defaults that override the hard-coded defaults in the xray_flags.inc at
 // compile-time using the XRAY_DEFAULT_OPTIONS macro.
-static const char *useCompilerDefinedFlags() XRAY_NEVER_INSTRUMENT {
+const char *useCompilerDefinedFlags() XRAY_NEVER_INSTRUMENT {
 #ifdef XRAY_DEFAULT_OPTIONS
-// Do the double-layered string conversion to prevent badly crafted strings
-// provided through the XRAY_DEFAULT_OPTIONS from causing compilation issues (or
-// changing the semantics of the implementation through the macro). This ensures
-// that we convert whatever XRAY_DEFAULT_OPTIONS is defined as a string literal.
+  // Do the double-layered string conversion to prevent badly crafted strings
+  // provided through the XRAY_DEFAULT_OPTIONS from causing compilation issues
+  // (or changing the semantics of the implementation through the macro). This
+  // ensures that we convert whatever XRAY_DEFAULT_OPTIONS is defined as a
+  // string literal.
   return SANITIZER_STRINGIFY(XRAY_DEFAULT_OPTIONS);
 #else
   return "";
index 3ed5b88..7c1ba94 100644 (file)
@@ -29,6 +29,8 @@ struct Flags {
 };
 
 extern Flags xray_flags_dont_use_directly;
+extern void registerXRayFlags(FlagParser *P, Flags *F);
+const char *useCompilerDefinedFlags();
 inline Flags *flags() { return &xray_flags_dont_use_directly; }
 
 void initializeFlags();
index 181ed3d..746051c 100644 (file)
@@ -39,11 +39,11 @@ XRAY_FLAG(int, xray_naive_log_thread_buffer_size, 1024,
 XRAY_FLAG(bool, xray_fdr_log, false,
           "DEPRECATED: Use xray_mode=xray-fdr instead.")
 XRAY_FLAG(int, xray_fdr_log_func_duration_threshold_us, 5,
-          "FDR logging will try to skip functions that execute for fewer "
-          "microseconds than this threshold.")
+          "DEPRECATED: use the environment variable XRAY_FDR_OPTIONS and set "
+          "func_duration_threshold_us instead.")
 XRAY_FLAG(int, xray_fdr_log_grace_period_us, 0,
-          "DEPRECATED: use xray_fdr_log_grace_period_ms instead.")
+          "DEPRECATED: use the environment variable XRAY_FDR_OPTIONS and set "
+          "grace_period_ms instead.")
 XRAY_FLAG(int, xray_fdr_log_grace_period_ms, 100,
-          "FDR logging will wait this much time in microseconds before "
-          "actually flushing the log; this gives a chance for threads to "
-          "notice that the log has been finalized and clean up.")
+          "DEPRECATED: use the environment variable XRAY_FDR_OPTIONS and set "
+          "grace_period_ms instead.")
index 926037f..64a4642 100644 (file)
@@ -1,10 +1,21 @@
 // RUN: %clangxx_xray -g -std=c++11 %s -o %t
 // RUN: rm fdr-logging-test-* || true
 // RUN: rm fdr-unwrite-test-* || true
-// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false xray_logfile_base=fdr-logging-test- xray_fdr_log=true verbosity=1 xray_fdr_log_func_duration_threshold_us=0" %run %t 2>&1 | FileCheck %s
-// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false xray_logfile_base=fdr-unwrite-test- xray_fdr_log=true verbosity=1 xray_fdr_log_func_duration_threshold_us=5000" %run %t 2>&1 | FileCheck %s
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t "`ls fdr-logging-test-* | head -1`" | FileCheck %s --check-prefix=TRACE
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t "`ls fdr-unwrite-test-* | head -1`" | FileCheck %s --check-prefix=UNWRITE
+// RUN: XRAY_OPTIONS="patch_premain=false xray_logfile_base=fdr-logging-test- \
+// RUN:     xray_mode=xray-fdr verbosity=1" \
+// RUN: XRAY_FDR_OPTIONS="func_duration_threshold_us=0" \
+// RUN:     %run %t 2>&1 | FileCheck %s
+// RUN: XRAY_OPTIONS="patch_premain=false \
+// RUN:     xray_logfile_base=fdr-unwrite-test- xray_mode=xray-fdr \
+// RUN:     verbosity=1" \
+// RUN: XRAY_FDR_OPTIONS="func_duration_threshold_us=5000" \
+// RUN:     %run %t 2>&1 | FileCheck %s
+// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
+// RUN:     "`ls fdr-logging-test-* | head -1`" \
+// RUN:     | FileCheck %s --check-prefix=TRACE
+// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
+// RUN:     "`ls fdr-unwrite-test-* | head -1`" \
+// RUN:     | FileCheck %s --check-prefix=UNWRITE
 // RUN: rm fdr-logging-test-*
 // RUN: rm fdr-unwrite-test-*
 // FIXME: Make llvm-xray work on non-x86_64 as well.
@@ -20,9 +31,6 @@
 #include <thread>
 #include <time.h>
 
-constexpr auto kBufferSize = 16384;
-constexpr auto kBufferMax = 10;
-
 thread_local uint64_t var = 0;
 [[clang::xray_always_instrument]] void __attribute__((noinline)) fC() { ++var; }
 
@@ -35,11 +43,12 @@ void __attribute__((noinline)) fArg(int) { }
 
 int main(int argc, char *argv[]) {
   using namespace __xray;
-  FDRLoggingOptions Options;
   std::cout << "Logging before init." << std::endl;
   // CHECK: Logging before init.
-  auto status = __xray_log_init(kBufferSize, kBufferMax, &Options,
-                                sizeof(FDRLoggingOptions));
+  assert(__xray_log_select_mode("xray-fdr") ==
+         XRayLogRegisterStatus::XRAY_REGISTRATION_OK);
+  auto status =
+      __xray_log_init_mode("xray-fdr", "buffer_size=16384:buffer_max=10");
   assert(status == XRayLogInitStatus::XRAY_LOG_INITIALIZED);
   std::cout << "Init status " << status << std::endl;
   // CHECK: Init status {{.*}}