[XRay] [compiler-rt] Move machine-dependent code into machine-dependent files.

[pelikan]

Summary

Include the necessary headers while there.

[dberris]

lib/xray/xray_arm.cc

23 ↗

(On Diff #73885)

Style guide recommends functions start with lowercase letters.

37 ↗

(On Diff #73885)

Same here.

[pelikan]

style guide fixes

[dberris]

So, these additional function calls replacing the in-lined code will incur non-trivial overheads in the fast path. As discussed offline we should do this refactoring differently or at a smaller scale.

[pelikan]

[XRay] [NFC] [compiler-rt] Move machine-dependent code into machine-dependent files.

[pelikan]

[XRay] [NFC] [compiler-rt] Move machine-dependent code into machine-dependent files.

[pelikan]

[XRay] [NFC] [compiler-rt] Move machine-dependent code into machine-dependent files.

[pelikan]

[XRay] [NFC] [compiler-rt] Move machine-dependent code into machine-dependent files.

[pelikan]

will incur non-trivial overheads in the fast path. As discussed offline
we should do this refactoring differently or at a smaller scale.

Fixed. The fast path is now more clear and marked as never-instrument to for bootstrapping LLVM with XRay enabled.

[dberris]

lib/xray/xray_arm.cc

23 ↗

(On Diff #76292)

Probably want constexpr here.

lib/xray/xray_inmemory_log.cc

141 ↗

(On Diff #76292)

We cannot actually assume that the compiler used to build compiler-rt actually supports this attribute. Let's defer this for later, when we have a check in CMake to figure out whether the compiler we're using supports XRay explicitly, then make this conditional on a macro.

lib/xray/xray_x86_64.h

6 ↗

(On Diff #76292)

Consider adding an attribute that forces this to be always inline, if not have a //TODO: or //FIXME: that indicates so.

[pelikan]

I've still kept this as a NFC change, despite the wrong strncat usage and Report() format strings. It's mostly shuffling code around.

I'm still waiting on an ARM build machine, so tested only on amd64.

[dberris]

lib/xray/xray_inmemory_log.cc

108 ↗

(On Diff #81868)

Probably want to add XRAY_NEVER_INSTRUMENT as an attribute, because of weird recursion problems.

lib/xray/xray_x86_64.cc

66 ↗

(On Diff #81868)

Did you miss an \n in the string?

[pelikan]

fix newline and a never instrument annotation

[dberris]

Just a few more...

lib/xray/xray_emulate_tsc.h

8 ↗

(On Diff #81902)

You can't mark this always inline as well? Also probably want the never instrument attribute here.

20 ↗

(On Diff #81902)

Same here, always inline and never instrument?

lib/xray/xray_x86_64.cc

56 ↗

(On Diff #81902)

Always inline, never instrument?

lib/xray/xray_x86_64.h

8 ↗

(On Diff #81902)

Also never instrument?

[pelikan]

annotated functions as requested

[dberris]

Local testing implies that there's a build break with this change.

../lib/clang/4.0.0/lib/linux/libclang_rt.xray-x86_64.a(xray_inmemory_log.cc.o): In function `__xray_OpenLogFile':
.../llvm/projects/compiler-rt/lib/xray/xray_inmemory_log.cc:138: undefined reference to `__xray::cycleFrequency()'
clang-4.0: error: linker command failed with exit code 1 (use -v to see invocation)

[dberris]

Please fix before I land?

[pelikan]

remove ALWAYS_INLINE from a function which is forward-declared

[pelikan]

ARM cycleFrequency() moved to .cc just like amd64 has it.

[dberris]

Thanks @pelikan -- trying again now.

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

 //===-- xray_AArch64.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.
 //
 // Implementation of AArch64-specific routines (64-bit).
 //
 //===----------------------------------------------------------------------===//
 #include "sanitizer_common/sanitizer_common.h"
 #include "xray_defs.h"
+#include "xray_emulate_tsc.h"
 #include "xray_interface_internal.h"
 #include <atomic>
 #include <cassert>
 
 namespace __xray {
 
+uint64_t cycleFrequency() XRAY_NEVER_INSTRUMENT {
+  // There is no instruction like RDTSCP in user mode on ARM.  ARM's CP15 does
+  //   not have a constant frequency like TSC on x86[_64]; it may go faster or
+  //   slower depending on CPU's turbo or power saving modes.  Furthermore, to
+  //   read from CP15 on ARM a kernel modification or a driver is needed.
+  //   We can not require this from users of compiler-rt.
+  // So on ARM we use clock_gettime(2) which gives the result in nanoseconds.
+  //   To get the measurements per second, we scale this by the number of
+  //   nanoseconds per second, pretending that the TSC frequency is 1GHz and
+  //   one TSC tick is 1 nanosecond.
+  return NanosecondsPerSecond;
+}
+
 // The machine codes for some instructions used in runtime patching.
 enum class PatchOpcodes : uint32_t {
   PO_StpX0X30SP_m16e = 0xA9BF7BE0, // STP X0, X30, [SP, #-16]!
   PO_LdrW0_12 = 0x18000060,        // LDR W0, #12
   PO_LdrX16_12 = 0x58000070,       // LDR X16, #12
   PO_BlrX16 = 0xD63F0200,          // BLR X16
   PO_LdpX0X30SP_16 = 0xA8C17BE0,   // LDP X0, X30, [SP], #16
   PO_B32 = 0x14000008              // B #32

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

 //===-- xray_arm.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.
 //
 // Implementation of ARM-specific routines (32-bit).
 //
 //===----------------------------------------------------------------------===//
 #include "sanitizer_common/sanitizer_common.h"
 #include "xray_defs.h"
+#include "xray_emulate_tsc.h"
 #include "xray_interface_internal.h"
 #include <atomic>
 #include <cassert>
 
 namespace __xray {
 
+uint64_t cycleFrequency() XRAY_NEVER_INSTRUMENT {
+  // There is no instruction like RDTSCP in user mode on ARM.  ARM's CP15 does
+  //   not have a constant frequency like TSC on x86[_64]; it may go faster or
+  //   slower depending on CPU's turbo or power saving modes.  Furthermore, to
+  //   read from CP15 on ARM a kernel modification or a driver is needed.
+  //   We can not require this from users of compiler-rt.
+  // So on ARM we use clock_gettime(2) which gives the result in nanoseconds.
+  //   To get the measurements per second, we scale this by the number of
+  //   nanoseconds per second, pretending that the TSC frequency is 1GHz and
+  //   one TSC tick is 1 nanosecond.
+  return NanosecondsPerSecond;
+}
+
 // The machine codes for some instructions used in runtime patching.
 enum class PatchOpcodes : uint32_t {
   PO_PushR0Lr = 0xE92D4001, // PUSH {r0, lr}
   PO_BlxIp = 0xE12FFF3C,    // BLX ip
   PO_PopR0Lr = 0xE8BD4001,  // POP {r0, lr}
   PO_B20 = 0xEA000005       // B #20
 };
 

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

+#pragma once
+#include <time.h>
+
+#include "sanitizer_common/sanitizer_internal_defs.h"
+#include "xray_defs.h"
+
+namespace __xray {
+
+static constexpr uint64_t NanosecondsPerSecond = 1000ULL * 1000 * 1000;
+
+ALWAYS_INLINE uint64_t readTSC(uint8_t &CPU) XRAY_NEVER_INSTRUMENT {
+  timespec TS;
+  int result = clock_gettime(CLOCK_REALTIME, &TS);
+  if (result != 0) {
+    Report("clock_gettime(2) returned %d, errno=%d.", result, int(errno));
+    TS.tv_sec = 0;
+    TS.tv_nsec = 0;
+  }
+  CPU = 0;
+  return TS.tv_sec * NanosecondsPerSecond + TS.tv_nsec;
+}
+
+}

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

 #include <mutex>
 #include <sys/stat.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include <thread>
 #include <unistd.h>
 
 #if defined(__x86_64__)
-#include <x86intrin.h>
+#include "xray_x86_64.h"
 #elif defined(__arm__) || defined(__aarch64__)
-static const int64_t NanosecondsPerSecond = 1000LL * 1000 * 1000;
+#include "xray_emulate_tsc.h"
 #else
 #error "Unsupported CPU Architecture"
-#endif /* CPU architecture */
+#endif /* Architecture-specific inline intrinsics */
 
 #include "sanitizer_common/sanitizer_libc.h"
 #include "xray/xray_records.h"
 #include "xray_defs.h"
 #include "xray_flags.h"
 #include "xray_interface_internal.h"
 
 // __xray_InMemoryRawLog will use a thread-local aligned buffer capped to a
while (auto Written = write(Fd, Begin, TotalBytes)) {
     }
     TotalBytes -= Written;
     if (TotalBytes == 0)
       break;
     Begin += Written;
   }
 }
 
-#if defined(__x86_64__)
-static std::pair<ssize_t, bool>
-retryingReadSome(int Fd, char *Begin, char *End) XRAY_NEVER_INSTRUMENT {
-  auto BytesToRead = std::distance(Begin, End);
-  ssize_t BytesRead;
-  ssize_t TotalBytesRead = 0;
-  while (BytesToRead && (BytesRead = read(Fd, Begin, BytesToRead))) {
-    if (BytesRead == -1) {
-      if (errno == EINTR)
-        continue;
-      Report("Read error; errno = %d\n", errno);
-      return std::make_pair(TotalBytesRead, false);
-    }
-
-    TotalBytesRead += BytesRead;
-    BytesToRead -= BytesRead;
-    Begin += BytesRead;
-  }
-  return std::make_pair(TotalBytesRead, true);
-}
-
-static bool readValueFromFile(const char *Filename,
-                              long long *Value) XRAY_NEVER_INSTRUMENT {
-  int Fd = open(Filename, O_RDONLY | O_CLOEXEC);
-  if (Fd == -1)
-    return false;
-  static constexpr size_t BufSize = 256;
-  char Line[BufSize] = {};
-  ssize_t BytesRead;
-  bool Success;
-  std::tie(BytesRead, Success) = retryingReadSome(Fd, Line, Line + BufSize);
-  if (!Success)
-    return false;
-  close(Fd);
-  char *End = nullptr;
-  long long Tmp = internal_simple_strtoll(Line, &End, 10);
-  bool Result = false;
-  if (Line[0] != '\0' && (*End == '\n' || *End == '\0')) {
-    *Value = Tmp;
-    Result = true;
-  }
-  return Result;
-}
-
-#endif /* CPU architecture */
-
 class ThreadExitFlusher {
   int Fd;
   XRayRecord *Start;
   size_t &Offset;
 
 public:
   explicit ThreadExitFlusher(int Fd, XRayRecord *Start,
                              size_t &Offset) XRAY_NEVER_INSTRUMENT

 } // namespace __xray
 
 using namespace __xray;
 
 void PrintToStdErr(const char *Buffer) XRAY_NEVER_INSTRUMENT {
   fprintf(stderr, "%s", Buffer);
 }
 
-void __xray_InMemoryRawLog(int32_t FuncId,
-                           XRayEntryType Type) XRAY_NEVER_INSTRUMENT {
-  using Buffer =
-      std::aligned_storage<sizeof(XRayRecord), alignof(XRayRecord)>::type;
-  static constexpr size_t BuffLen = 1024;
-  thread_local static Buffer InMemoryBuffer[BuffLen] = {};
-  thread_local static size_t Offset = 0;
-  static int Fd = [] {
-    // FIXME: Figure out how to make this less stderr-dependent.
-    SetPrintfAndReportCallback(PrintToStdErr);
-    // Open a temporary file once for the log.
-    static char TmpFilename[256] = {};
-    static char TmpWildcardPattern[] = "XXXXXX";
-    auto E = internal_strncat(TmpFilename, flags()->xray_logfile_base,
-                              sizeof(TmpFilename) - 10);
-    if (static_cast<size_t>((E + 6) - TmpFilename) >
-        (sizeof(TmpFilename) - 1)) {
-      Report("XRay log file base too long: %s\n", flags()->xray_logfile_base);
-      return -1;
-    }
-    internal_strncat(TmpFilename, TmpWildcardPattern,
-                     sizeof(TmpWildcardPattern) - 1);
-    int Fd = mkstemp(TmpFilename);
-    if (Fd == -1) {
-      Report("XRay: Failed opening temporary file '%s'; not logging events.\n",
-             TmpFilename);
-      return -1;
-    }
-    if (Verbosity())
-      fprintf(stderr, "XRay: Log file in '%s'\n", TmpFilename);
+static int __xray_OpenLogFile() XRAY_NEVER_INSTRUMENT {
 
-    // Get the cycle frequency from SysFS on Linux.
-    long long CPUFrequency = -1;
-#if defined(__x86_64__)
-    if (readValueFromFile("/sys/devices/system/cpu/cpu0/tsc_freq_khz",
-                          &CPUFrequency)) {
-      CPUFrequency *= 1000;
-    } else if (readValueFromFile(
-                   "/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq",
-                   &CPUFrequency)) {
-      CPUFrequency *= 1000;
-    } else {
-      Report("Unable to determine CPU frequency for TSC accounting.\n");
-    }
-#elif defined(__arm__) || defined(__aarch64__)
-    // There is no instruction like RDTSCP in user mode on ARM. ARM's CP15 does
-    //   not have a constant frequency like TSC on x86(_64), it may go faster
-    //   or slower depending on CPU turbo or power saving mode. Furthermore,
-    //   to read from CP15 on ARM a kernel modification or a driver is needed.
-    //   We can not require this from users of compiler-rt.
-    // So on ARM we use clock_gettime() which gives the result in nanoseconds.
-    //   To get the measurements per second, we scale this by the number of
-    //   nanoseconds per second, pretending that the TSC frequency is 1GHz and
-    //   one TSC tick is 1 nanosecond.
-    CPUFrequency = NanosecondsPerSecond;
-#else
-#error "Unsupported CPU Architecture"
-#endif /* CPU architecture */
-
-    // Since we're here, we get to write the header. We set it up so that the
-    // header will only be written once, at the start, and let the threads
-    // logging do writes which just append.
-    XRayFileHeader Header;
-    Header.Version = 1;
-    Header.Type = FileTypes::NAIVE_LOG;
-    Header.CycleFrequency =
-        CPUFrequency == -1 ? 0 : static_cast<uint64_t>(CPUFrequency);
+  Header.CycleFrequency = __xray::cycleFrequency();
 
-    // FIXME: Actually check whether we have 'constant_tsc' and 'nonstop_tsc'
-    // before setting the values in the header.
-    Header.ConstantTSC = 1;
-    Header.NonstopTSC = 1;
-    retryingWriteAll(Fd, reinterpret_cast<char *>(&Header),
-                     reinterpret_cast<char *>(&Header) + sizeof(Header));
-    return Fd;
-  }();
+}
+
+void __xray_InMemoryRawLog(int32_t FuncId,
+                           XRayEntryType Type) XRAY_NEVER_INSTRUMENT {
+  using Buffer =
+      std::aligned_storage<sizeof(XRayRecord), alignof(XRayRecord)>::type;
+  static constexpr size_t BuffLen = 1024;
+  thread_local static Buffer InMemoryBuffer[BuffLen] = {};
+  thread_local static size_t Offset = 0;
+  static int Fd = __xray_OpenLogFile();
   if (Fd == -1)
     return;
   thread_local __xray::ThreadExitFlusher Flusher(
       Fd, reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer), Offset);
   thread_local pid_t TId = syscall(SYS_gettid);
 
   // First we get the useful data, and stuff it into the already aligned buffer
   // through a pointer offset.
   auto &R = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer)[Offset];
   R.RecordType = RecordTypes::NORMAL;
-#if defined(__x86_64__)
-  {
-    unsigned CPU;
-    R.TSC = __rdtscp(&CPU);
-    R.CPU = CPU;
-  }
-#elif defined(__arm__) || defined(__aarch64__)
-  {
-    timespec TS;
-    int result = clock_gettime(CLOCK_REALTIME, &TS);
-    if (result != 0) {
-      Report("clock_gettime() returned %d, errno=%d.\n", result, int(errno));
-      TS.tv_sec = 0;
-      TS.tv_nsec = 0;
-    }
-    R.TSC = TS.tv_sec * NanosecondsPerSecond + TS.tv_nsec;
-    R.CPU = 0;
-  }
-#else
-#error "Unsupported CPU Architecture"
-#endif /* CPU architecture */
+  R.TSC = __xray::readTSC(R.CPU);
   R.TId = TId;
   R.Type = Type;
   R.FuncId = FuncId;
   ++Offset;
   if (Offset == BuffLen) {
     std::lock_guard<std::mutex> L(LogMutex);
     auto RecordBuffer = reinterpret_cast<__xray::XRayRecord *>(InMemoryBuffer);
     retryingWriteAll(Fd, reinterpret_cast<char *>(RecordBuffer),

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

 
 namespace __xray {
 
 struct XRaySledMap {
   const XRaySledEntry *Sleds;
   size_t Entries;
 };
 
+uint64_t cycleFrequency();
+
 bool patchFunctionEntry(bool Enable, uint32_t FuncId,
                         const XRaySledEntry &Sled);
 bool patchFunctionExit(bool Enable, uint32_t FuncId, const XRaySledEntry &Sled);
 bool patchFunctionTailExit(bool Enable, uint32_t FuncId,
                            const XRaySledEntry &Sled);
 
 } // namespace __xray
 

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

+#pragma once
+#include <x86intrin.h>
+
+#include "sanitizer_common/sanitizer_internal_defs.h"
+#include "xray_defs.h"
+
+namespace __xray {
+
+ALWAYS_INLINE uint64_t readTSC(uint8_t &CPU) XRAY_NEVER_INSTRUMENT {
+  unsigned LongCPU;
+  uint64_t TSC = __rdtscp(&LongCPU);
+  CPU = LongCPU;
+  return TSC;
+}
+
+}

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

 #include "sanitizer_common/sanitizer_common.h"
 #include "xray_defs.h"
 #include "xray_interface_internal.h"
 #include <atomic>
 #include <cstdint>
+#include <fcntl.h>
 #include <limits>
+#include <tuple>
+#include <unistd.h>
 
 namespace __xray {
 
+static std::pair<ssize_t, bool>
+retryingReadSome(int Fd, char *Begin, char *End) XRAY_NEVER_INSTRUMENT {
+  auto BytesToRead = std::distance(Begin, End);
+  ssize_t BytesRead;
+  ssize_t TotalBytesRead = 0;
+  while (BytesToRead && (BytesRead = read(Fd, Begin, BytesToRead))) {
+    if (BytesRead == -1) {
+      if (errno == EINTR)
+        continue;
+      Report("Read error; errno = %d\n", errno);
+      return std::make_pair(TotalBytesRead, false);
+    }
+
+    TotalBytesRead += BytesRead;
+    BytesToRead -= BytesRead;
+    Begin += BytesRead;
+  }
+  return std::make_pair(TotalBytesRead, true);
+}
+
+static bool readValueFromFile(const char *Filename,
+                              long long *Value) XRAY_NEVER_INSTRUMENT {
+  int Fd = open(Filename, O_RDONLY | O_CLOEXEC);
+  if (Fd == -1)
+    return false;
+  static constexpr size_t BufSize = 256;
+  char Line[BufSize] = {};
+  ssize_t BytesRead;
+  bool Success;
+  std::tie(BytesRead, Success) = retryingReadSome(Fd, Line, Line + BufSize);
+  if (!Success)
+    return false;
+  close(Fd);
+  char *End = nullptr;
+  long long Tmp = internal_simple_strtoll(Line, &End, 10);
+  bool Result = false;
+  if (Line[0] != '\0' && (*End == '\n' || *End == '\0')) {
+    *Value = Tmp;
+    Result = true;
+  }
+  return Result;
+}
+
+uint64_t cycleFrequency() XRAY_NEVER_INSTRUMENT {
+  long long CPUFrequency = -1;
+  if (readValueFromFile("/sys/devices/system/cpu/cpu0/tsc_freq_khz",
+                        &CPUFrequency)) {
+    CPUFrequency *= 1000;
+  } else if (readValueFromFile(
+      "/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq",
+      &CPUFrequency)) {
+    CPUFrequency *= 1000;
+  } else {
+    Report("Unable to determine CPU frequency for TSC accounting.\n");
+  }
+  return CPUFrequency == -1 ? 0 : static_cast<uint64_t>(CPUFrequency);
+}
+
 static constexpr uint8_t CallOpCode = 0xe8;
 static constexpr uint16_t MovR10Seq = 0xba41;
 static constexpr uint16_t Jmp9Seq = 0x09eb;
 static constexpr uint8_t JmpOpCode = 0xe9;
 static constexpr uint8_t RetOpCode = 0xc3;
 
 static constexpr int64_t MinOffset{std::numeric_limits<int32_t>::min()};
 static constexpr int64_t MaxOffset{std::numeric_limits<int32_t>::max()};