fixup! [XRay] Add DSO support for XRay instrumentation on X86_64

Author: sebastiankreutzer

clang/lib/Driver/ToolChains/CommonArgs.cpp

@@ -1595,9 +1595,9 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
 bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
   if (Args.hasArg(options::OPT_shared)) {
     if (TC.getXRayArgs().needsXRayDSORt()) {
-      CmdArgs.push_back("-whole-archive");
+      CmdArgs.push_back("--whole-archive");
       CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray-dso"));
-      CmdArgs.push_back("-no-whole-archive");
+      CmdArgs.push_back("--no-whole-archive");
       return true;
     }
   } else if (TC.getXRayArgs().needsXRayRt()) {

clang/lib/Driver/XRayArgs.cpp

@@ -64,9 +64,21 @@ XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
   }
 
   if (Args.hasFlag(options::OPT_fxray_enable_shared,
-                   options::OPT_fno_xray_enable_shared, false))
+                   options::OPT_fno_xray_enable_shared, false)) {
     XRayEnableShared = true;
 
+    // DSO instrumentation is currently limited to x86_64
+    if (Triple.getArch() != llvm::Triple::x86_64) {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << "-fxray-enable-shared" << Triple.str();
+    }
+
+    unsigned PICLvl = std::get<1>(tools::ParsePICArgs(TC, Args));
+    if (!PICLvl) {
+      D.Diag(diag::err_opt_not_valid_without_opt) << "-fxray-enable-shared" << "-fPIC";
+    }
+  }
+
   // Both XRay and -fpatchable-function-entry use
   // TargetOpcode::PATCHABLE_FUNCTION_ENTER.
   if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry_EQ))

clang/test/Driver/XRay/xray-enable-shared.cpp

@@ -0,0 +1,17 @@
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fPIC -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fpic -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -fno-PIC -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR-PIC
+// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -fno-pic -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR-PIC
+
+// On 64 bit darwin, PIC is always enabled
+// RUN: %clang -### --target=x86_64-apple-darwin -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s
+
+// Check unsupported targets
+// RUN: not %clang -### --target=aarch64-pc-freebsd -fPIC -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR-TARGET
+// RUN: not %clang -### --target=arm64-apple-macos -fPIC -fxray-instrument -fxray-enable-shared -c %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR-TARGET
+
+// CHECK: "-cc1" {{.*}}"-fxray-instrument" {{.*}}"-fxray-enable-shared"
+// ERR-TARGET:   error: unsupported option '-fxray-enable-shared' for target
+// ERR-PIC:   error: option '-fxray-enable-shared' cannot be specified without '-fPIC'
+

compiler-rt/lib/xray/CMakeLists.txt

@@ -393,14 +393,14 @@ else() # not Apple
       PARENT_TARGET xray)
     # Profiler Mode runtime
     add_compiler_rt_runtime(clang_rt.xray-profiling
-     STATIC
-     ARCHS ${arch}
-     CFLAGS ${XRAY_CFLAGS}
-     LINK_FLAGS ${XRAY_LINK_FLAGS}
-     LINK_LIBS ${XRAY_LINK_LIBS}
-     DEFS ${XRAY_COMMON_DEFINITIONS}
-     OBJECT_LIBS RTXrayPROFILING
-     PARENT_TARGET xray)
+      STATIC
+      ARCHS ${arch}
+      CFLAGS ${XRAY_CFLAGS}
+      LINK_FLAGS ${XRAY_LINK_FLAGS}
+      LINK_LIBS ${XRAY_LINK_LIBS}
+      DEFS ${XRAY_COMMON_DEFINITIONS}
+      OBJECT_LIBS RTXrayPROFILING
+      PARENT_TARGET xray)
 
     if (${arch} IN_LIST XRAY_DSO_SUPPORTED_ARCH)
       # TODO: Only implemented for X86 at the moment

compiler-rt/lib/xray/xray_dso_init.cpp

@@ -28,7 +28,7 @@ __attribute__((visibility("hidden")));
 extern const XRayFunctionSledIndex __stop_xray_fn_idx[] __attribute__((weak))
 __attribute__((visibility("hidden")));
 
-#if SANITIZER_MAC
+#if SANITIZER_APPLE
 // HACK: This is a temporary workaround to make XRay build on
 // Darwin, but it will probably not work at runtime.
 extern const XRaySledEntry __start_xray_instr_map[] = {};

compiler-rt/lib/xray/xray_init.cpp

@@ -45,17 +45,15 @@ using namespace __xray;
 // the weak symbols defined above (__start_xray_inst_map and
 // __stop_xray_instr_map) to initialise the instrumentation map that XRay uses
 // for runtime patching/unpatching of instrumentation points.
-//
-// FIXME: Support DSO instrumentation maps too. The current solution only works
-// for statically linked executables.
 atomic_uint8_t XRayInitialized{0};
 
 // This should always be updated before XRayInitialized is updated.
 SpinMutex XRayInstrMapMutex;
-// XRaySledMap XRayInstrMap;
+
 //  Contains maps for the main executable as well as DSOs.
-// std::vector<XRaySledMap> XRayInstrMaps;
 XRaySledMap *XRayInstrMaps;
+
+// Number of binary objects registered.
 atomic_uint32_t XRayNumObjects{0};
 
 // Global flag to determine whether the flags have been initialized.
@@ -64,6 +62,9 @@ atomic_uint8_t XRayFlagsInitialized{0};
 // A mutex to allow only one thread to initialize the XRay data structures.
 SpinMutex XRayInitMutex;
 
+// Registers XRay sleds and trampolines coming from the main executable or one
+// of the linked DSOs.
+// Returns the object ID if registration is successful, -1 otherwise.
 int32_t
 __xray_register_sleds(const XRaySledEntry *SledsBegin,
                       const XRaySledEntry *SledsEnd,
@@ -145,8 +146,9 @@ void __xray_init() XRAY_NEVER_INSTRUMENT {
   // Pre-allocation takes up approx. 5kB for XRayMaxObjects=64.
   XRayInstrMaps = allocateBuffer<XRaySledMap>(XRayMaxObjects);
 
-  int MainBinaryId = __xray_register_sleds(__start_xray_instr_map, __stop_xray_instr_map,
-                        __start_xray_fn_idx, __stop_xray_fn_idx, false, {});
+  int MainBinaryId =
+      __xray_register_sleds(__start_xray_instr_map, __stop_xray_instr_map,
+                            __start_xray_fn_idx, __stop_xray_fn_idx, false, {});
 
   // The executable should always get ID 0.
   if (MainBinaryId != 0) {
@@ -162,6 +164,9 @@ void __xray_init() XRAY_NEVER_INSTRUMENT {
 #endif
 }
 
+// Registers XRay sleds and trampolines of an instrumented DSO.
+// Returns the object ID if registration is successful, -1 otherwise.
+//
 // Default visibility is hidden, so we have to explicitly make it visible to
 // DSO.
 SANITIZER_INTERFACE_ATTRIBUTE int32_t __xray_register_dso(
@@ -191,6 +196,11 @@ SANITIZER_INTERFACE_ATTRIBUTE int32_t __xray_register_dso(
   return ObjId;
 }
 
+// Deregisters a DSO from the main XRay runtime.
+// Called from the DSO-local runtime when the library is unloaded (e.g. if
+// dlclose is called).
+// Returns true if the object ID is valid and the DSO was successfully
+// deregistered.
 SANITIZER_INTERFACE_ATTRIBUTE bool
 __xray_deregister_dso(int32_t ObjId) XRAY_NEVER_INSTRUMENT {
 
@@ -220,7 +230,7 @@ __xray_deregister_dso(int32_t ObjId) XRAY_NEVER_INSTRUMENT {
       if (Verbosity())
         Report("Can't deregister object with ID %d: object is not loaded.\n",
                ObjId);
-      return false;
+      return true;
     }
     // Mark DSO as unloaded. No need to unpatch.
     Entry.Loaded = false;

compiler-rt/lib/xray/xray_interface.cpp

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "xray_interface_internal.h"
+#include "llvm/Support/ErrorHandling.h"
 
 #include <cinttypes>
 #include <cstdio>
@@ -289,24 +290,9 @@ XRayPatchingStatus patchFunction(int32_t FuncId, int32_t ObjId,
 // controlPatching implements the common internals of the patching/unpatching
 // implementation. |Enable| defines whether we're enabling or disabling the
 // runtime XRay instrumentation.
-XRayPatchingStatus controlPatching(bool Enable,
-                                   int32_t ObjId) XRAY_NEVER_INSTRUMENT {
-
-  if (!atomic_load(&XRayInitialized, memory_order_acquire))
-    return XRayPatchingStatus::NOT_INITIALIZED; // Not initialized.
-
-  uint8_t NotPatching = false;
-  if (!atomic_compare_exchange_strong(
-          &XRayPatching, &NotPatching, true, memory_order_acq_rel))
-    return XRayPatchingStatus::ONGOING; // Already patching.
-
-  uint8_t PatchingSuccess = false;
-  auto XRayPatchingStatusResetter =
-      at_scope_exit([&PatchingSuccess] {
-        if (!PatchingSuccess)
-          atomic_store(&XRayPatching, false, memory_order_release);
-      });
-
+// This function should only be called after ensuring that XRay is initialized
+// and no other thread is currently patching.
+XRayPatchingStatus controlPatchingObjectUnchecked(bool Enable, int32_t ObjId) {
   XRaySledMap InstrMap;
   {
     SpinMutexLock Guard(&XRayInstrMapMutex);
@@ -382,10 +368,80 @@ XRayPatchingStatus controlPatching(bool Enable,
     patchSled(Sled, Enable, PackedId, InstrMap.Trampolines);
   }
   atomic_store(&XRayPatching, false, memory_order_release);
-  PatchingSuccess = true;
   return XRayPatchingStatus::SUCCESS;
 }
 
+
+// Controls patching for all registered objects.
+// Returns: SUCCESS, if patching succeeds for all objects.
+//          NOT_INITIALIZED, if one or more objects returned NOT_INITIALIZED
+//             but none failed.
+//          FAILED, if patching of one or more objects failed.
+XRayPatchingStatus controlPatching(bool Enable) XRAY_NEVER_INSTRUMENT {
+  if (!atomic_load(&XRayInitialized, memory_order_acquire))
+    return XRayPatchingStatus::NOT_INITIALIZED; // Not initialized.
+
+  uint8_t NotPatching = false;
+  if (!atomic_compare_exchange_strong(
+          &XRayPatching, &NotPatching, true, memory_order_acq_rel))
+    return XRayPatchingStatus::ONGOING; // Already patching.
+
+  auto XRayPatchingStatusResetter =
+      at_scope_exit([] {
+        atomic_store(&XRayPatching, false, memory_order_release);
+      });
+
+  unsigned NumObjects = __xray_num_objects();
+
+  XRayPatchingStatus CombinedStatus{NOT_INITIALIZED};
+  for (unsigned I = 0; I < NumObjects; ++I) {
+    if (!isObjectLoaded(I))
+      continue;
+    auto LastStatus = controlPatchingObjectUnchecked(Enable, I);
+    switch (LastStatus) {
+    case SUCCESS:
+      if (CombinedStatus == NOT_INITIALIZED)
+        CombinedStatus = SUCCESS;
+      break;
+    case FAILED:
+      // Report failure, but try to patch the remaining objects
+      CombinedStatus = FAILED;
+      break;
+    case NOT_INITIALIZED:
+      // XRay has been initialized but there are no sleds available for this
+      // object. Try to patch remaining objects.
+      if (CombinedStatus != FAILED)
+        CombinedStatus = NOT_INITIALIZED;
+      break;
+    case ONGOING:
+      llvm_unreachable("Status ONGOING should not appear at this point");
+    default:
+      llvm_unreachable("Unhandled patching status");
+    }
+  }
+  return CombinedStatus;
+}
+
+// Controls patching for one object.
+XRayPatchingStatus controlPatching(bool Enable,
+                                   int32_t ObjId) XRAY_NEVER_INSTRUMENT {
+
+  if (!atomic_load(&XRayInitialized, memory_order_acquire))
+    return XRayPatchingStatus::NOT_INITIALIZED; // Not initialized.
+
+  uint8_t NotPatching = false;
+  if (!atomic_compare_exchange_strong(
+          &XRayPatching, &NotPatching, true, memory_order_acq_rel))
+    return XRayPatchingStatus::ONGOING; // Already patching.
+
+  auto XRayPatchingStatusResetter =
+      at_scope_exit([] {
+          atomic_store(&XRayPatching, false, memory_order_release);
+      });
+
+  return controlPatchingObjectUnchecked(Enable, ObjId);
+}
+
 XRayPatchingStatus mprotectAndPatchFunction(int32_t FuncId, int32_t ObjId,
                                             bool Enable) XRAY_NEVER_INSTRUMENT {
   XRaySledMap InstrMap;
@@ -420,7 +476,7 @@ XRayPatchingStatus mprotectAndPatchFunction(int32_t FuncId, int32_t ObjId,
     return XRayPatchingStatus::FAILED;
   }
 
-  // Here we compute the minumum sled and maximum sled associated with a
+  // Here we compute the minimum sled and maximum sled associated with a
   // particular function ID.
   XRayFunctionSledIndex SledRange;
   if (InstrMap.SledsIndex) {
@@ -518,57 +574,15 @@ uint16_t __xray_register_event_type(
 }
 
 XRayPatchingStatus __xray_patch() XRAY_NEVER_INSTRUMENT {
-  XRayPatchingStatus CombinedStatus{SUCCESS};
-  for (size_t I = 0; I < __xray_num_objects(); ++I) {
-    if (!isObjectLoaded(I))
-      continue;
-    auto LastStatus = controlPatching(true, I);
-    switch (LastStatus) {
-    case FAILED:
-      CombinedStatus = FAILED;
-      break;
-    case NOT_INITIALIZED:
-      if (CombinedStatus != FAILED)
-        CombinedStatus = NOT_INITIALIZED;
-      break;
-    case ONGOING:
-      if (CombinedStatus != FAILED && CombinedStatus != NOT_INITIALIZED)
-        CombinedStatus = ONGOING;
-      break;
-    default:
-      break;
-    }
-  }
-  return CombinedStatus;
+  return controlPatching(true);
 }
 
 XRayPatchingStatus __xray_patch_object(int32_t ObjId) XRAY_NEVER_INSTRUMENT {
   return controlPatching(true, ObjId);
 }
 
 XRayPatchingStatus __xray_unpatch() XRAY_NEVER_INSTRUMENT {
-  XRayPatchingStatus CombinedStatus{SUCCESS};
-  for (size_t I = 0; I < __xray_num_objects(); ++I) {
-    if (!isObjectLoaded(I))
-      continue;
-    auto LastStatus = controlPatching(false, I);
-    switch (LastStatus) {
-    case FAILED:
-      CombinedStatus = FAILED;
-      break;
-    case NOT_INITIALIZED:
-      if (CombinedStatus != FAILED)
-        CombinedStatus = NOT_INITIALIZED;
-      break;
-    case ONGOING:
-      if (CombinedStatus != FAILED && CombinedStatus != NOT_INITIALIZED)
-        CombinedStatus = ONGOING;
-      break;
-    default:
-      break;
-    }
-  }
-  return CombinedStatus;
+  return controlPatching(false);
 }
 
 XRayPatchingStatus __xray_unpatch_object(int32_t ObjId) XRAY_NEVER_INSTRUMENT {

compiler-rt/test/xray/TestCases/Posix/dlopen.cpp

@@ -4,7 +4,7 @@
 // RUN: split-file %s %t
 // RUN: %clangxx_xray -g -fPIC -fxray-instrument -fxray-enable-shared -shared -std=c++11 %t/testlib.cpp -o %t/testlib.so
 // RUN: %clangxx_xray -g -fPIC -rdynamic -fxray-instrument -fxray-enable-shared -std=c++11 %t/main.cpp -o %t/main.o
-
+//
 // RUN: XRAY_OPTIONS="patch_premain=true" %run %t/main.o %t/testlib.so 2>&1 | FileCheck %s
 
 // REQUIRES: target=x86_64{{.*}}
@@ -16,11 +16,8 @@
 #include <cstdio>
 #include <dlfcn.h>
 
-bool called = false;
-
 void test_handler(int32_t fid, XRayEntryType type) {
   printf("called: %d, type=%d\n", fid, static_cast<int32_t>(type));
-  called = true;
 }
 
 [[clang::xray_always_instrument]] void instrumented_in_executable() {