[OpenMP][Archer] Use dlsym rather than weak symbols for TSan annotations

[protze.joachim]

Summary

This is to fix issues reported for Ubuntu and possibly other platforms:
https://github.com/llvm/llvm-project/issues/45290

The latest comment on this issue points out that using dlsym rather than the weak symbol approach to call TSan annotation functions fixes the issue for Ubuntu.

[Hahnfeld]

LG, assuming it fixes the problem and is not too much of a performance blow (the weak symbols will be resolved by the runtime loader, whereas we now do indirect function calls through pointers).

Out of curiosity (I came across this, but doesn't need to be addressed here): Is there a technical reason we implement RunningOnValgrind separately, not using the exact same dlsym infrastructure as for the TSan annotation?

[protze.joachim]

Fix the special case handling for RunningOnValgrind pointed out by @Hahnfeld . We don't want to have output for a missing call, so it still has a separate macro.

[Hahnfeld]

Thanks!

[vzakhari]

This breaks my build. It looks like CMAKE_DL_LIBS dependency is missing:

CMakeFiles/archer.dir/ompt-tsan.cpp.o: In function `ompt_start_tool':
ompt-tsan.cpp:(.text.ompt_start_tool+0x55): undefined reference to `dlsym'
CMakeFiles/archer.dir/ompt-tsan.cpp.o: In function `ompt_tsan_initialize(void (*(*)(char const*))(), int, ompt_data_t*)':
ompt-tsan.cpp:(.text._ZL20ompt_tsan_initializePFPFvvEPKcEiP11ompt_data_t+0x277): undefined reference to `dlsym'
ompt-tsan.cpp:(.text._ZL20ompt_tsan_initializePFPFvvEPKcEiP11ompt_data_t+0x295): undefined reference to `dlsym'
ompt-tsan.cpp:(.text._ZL20ompt_tsan_initializePFPFvvEPKcEiP11ompt_data_t+0x2b3): undefined reference to `dlsym'
ompt-tsan.cpp:(.text._ZL20ompt_tsan_initializePFPFvvEPKcEiP11ompt_data_t+0x2d1): undefined reference to `dlsym'
CMakeFiles/archer.dir/ompt-tsan.cpp.o:ompt-tsan.cpp:(.text._ZL20ompt_tsan_initializePFPFvvEPKcEiP11ompt_data_t+0x2ef): more undefined references to `dlsym' follow
collect2: error: ld returned 1 exit status

[vzakhari]

Same here: https://lab.llvm.org/buildbot/#/builders/193/builds/25434

[estewart08]

Also broke the AMD OpenMP buildbot:
https://lab.llvm.org/buildbot/#/builders/193/builds/25420

[vzakhari]

I reverted this to fix the buildbots. Please consider the following change to avoid the issue:

diff --git a/openmp/tools/archer/CMakeLists.txt b/openmp/tools/archer/CMakeLists.txt
index 85405af..46f38e2 100644
--- a/openmp/tools/archer/CMakeLists.txt
+++ b/openmp/tools/archer/CMakeLists.txt
@@ -12,6 +12,7 @@ if(LIBOMP_OMPT_SUPPORT)
   include_directories(${LIBOMP_INCLUDE_DIR})

   add_library(archer SHARED ompt-tsan.cpp)
+  target_link_libraries(archer ${CMAKE_DL_LIBS})
   add_library(archer_static STATIC ompt-tsan.cpp)

   install(TARGETS archer archer_static

[protze.joachim]

Explicitly link libdl as suggested by @vzakhari.

openmp/tools/archer/CMakeLists.txt

 # //===----------------------------------------------------------------------===//
 # //
 # // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # // See https://llvm.org/LICENSE.txt for details.
 # // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 # //
 # //===----------------------------------------------------------------------===//
   
   
 
 if(LIBOMP_OMPT_SUPPORT)
   include_directories(${LIBOMP_INCLUDE_DIR})
 
   add_library(archer SHARED ompt-tsan.cpp)
+  target_link_libraries(archer ${CMAKE_DL_LIBS})
   add_library(archer_static STATIC ompt-tsan.cpp)
 
   install(TARGETS archer archer_static
     LIBRARY DESTINATION ${OPENMP_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${OPENMP_INSTALL_LIBDIR})
 
   add_subdirectory(tests)
 endif()

openmp/tools/archer/ompt-tsan.cpp

 #include <list>
 #include <mutex>
 #include <sstream>
 #include <string>
 #include <sys/resource.h>
 #include <unistd.h>
 #include <unordered_map>
 #include <vector>
-
-#if (defined __APPLE__ && defined __MACH__)
 #include <dlfcn.h>
-#endif
 
 #include "omp-tools.h"
 
 // Define attribute that indicates that the fall through from the previous
 // case label is intentional and should not be diagnosed by a compiler
 //   Code from libcxx/include/__config
 // Use a function like macro to imply that it must be followed by a semicolon
 #if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
 #define KMP_FALLTHROUGH() [[fallthrough]]
 // icc cannot properly tell this attribute is absent so force off
 #elif defined(__INTEL_COMPILER)
 #define KMP_FALLTHROUGH() ((void)0)
 #elif __has_cpp_attribute(clang::fallthrough)
 #define KMP_FALLTHROUGH() [[clang::fallthrough]]
 #elif __has_attribute(fallthrough) || __GNUC__ >= 7
 #define KMP_FALLTHROUGH() __attribute__((__fallthrough__))
 #else
 #define KMP_FALLTHROUGH() ((void)0)
 #endif
 
-static int runOnTsan;
 static int hasReductionCallback;
 
 namespace {
 class ArcherFlags {
 public:
 #if (LLVM_VERSION) >= 40
   int flush_shadow{0};
 #endif

 #endif
 static ArcherFlags *archer_flags;
 
 #ifndef TsanHappensBefore
 // Thread Sanitizer is a tool that finds races in code.
 // See http://code.google.com/p/data-race-test/wiki/DynamicAnnotations .
 // tsan detects these exact functions by name.
 extern "C" {
-#if (defined __APPLE__ && defined __MACH__)
 static void (*AnnotateHappensAfter)(const char *, int, const volatile void *);
 static void (*AnnotateHappensBefore)(const char *, int, const volatile void *);
 static void (*AnnotateIgnoreWritesBegin)(const char *, int);
 static void (*AnnotateIgnoreWritesEnd)(const char *, int);
 static void (*AnnotateNewMemory)(const char *, int, const volatile void *,
                                  size_t);
 static void (*__tsan_func_entry)(const void *);
 static void (*__tsan_func_exit)(void);
-
-static int RunningOnValgrind() {
-  int (*fptr)();
-
-  fptr = (int (*)())dlsym(RTLD_DEFAULT, "RunningOnValgrind");
-  // If we found RunningOnValgrind other than this function, we assume
-  // Annotation functions present in this execution and leave runOnTsan=1
-  // otherwise we change to runOnTsan=0
-  if (!fptr || fptr == RunningOnValgrind)
-    runOnTsan = 0;
-  return 0;
-}
-#else
-void __attribute__((weak))
-AnnotateHappensAfter(const char *file, int line, const volatile void *cv) {}
-void __attribute__((weak))
-AnnotateHappensBefore(const char *file, int line, const volatile void *cv) {}
-void __attribute__((weak))
-AnnotateIgnoreWritesBegin(const char *file, int line) {}
-void __attribute__((weak)) AnnotateIgnoreWritesEnd(const char *file, int line) {
-}
-void __attribute__((weak))
-AnnotateNewMemory(const char *file, int line, const volatile void *cv,
-                  size_t size) {}
-int __attribute__((weak)) RunningOnValgrind() {
-  runOnTsan = 0;
-  return 0;
-}
-void __attribute__((weak)) __tsan_func_entry(const void *call_pc) {}
-void __attribute__((weak)) __tsan_func_exit(void) {}
-#endif
+static int (*RunningOnValgrind)(void);
 }
 
 // This marker is used to define a happens-before arc. The race detector will
 // infer an arc from the begin to the end when they share the same pointer
 // argument.
 #define TsanHappensBefore(cv) AnnotateHappensBefore(__FILE__, __LINE__, cv)
 
 // This marker defines the destination of a happens-before arc.

 #define SET_CALLBACK_T(event, type)                                            \
   do {                                                                         \
     int res;                                                                   \
     SET_OPTIONAL_CALLBACK_T(event, type, res, ompt_set_always);                \
   } while (0)
 
 #define SET_CALLBACK(event) SET_CALLBACK_T(event, event)
 
+#define findTsanFunction(f, fSig)                                              \
+  do {                                                                         \
+    if (NULL == (f = fSig dlsym(RTLD_DEFAULT, #f)))                            \
+      printf("Unable to find TSan function " #f ".\n");                        \
+  } while (0)
+
+#define findTsanFunctionSilent(f, fSig) f = fSig dlsym(RTLD_DEFAULT, #f)
+
 static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
                                 ompt_data_t *tool_data) {
   const char *options = getenv("TSAN_OPTIONS");
   TsanFlags tsan_flags(options);
 
   ompt_set_callback_t ompt_set_callback =
       (ompt_set_callback_t)lookup("ompt_set_callback");
   if (ompt_set_callback == NULL) {
     std::cerr << "Could not set callback, exiting..." << std::endl;
     std::exit(1);
   }
   ompt_get_parallel_info =
       (ompt_get_parallel_info_t)lookup("ompt_get_parallel_info");
   ompt_get_thread_data = (ompt_get_thread_data_t)lookup("ompt_get_thread_data");
 
   if (ompt_get_parallel_info == NULL) {
     fprintf(stderr, "Could not get inquiry function 'ompt_get_parallel_info', "
                     "exiting...\n");
     exit(1);
   }
 
-#if (defined __APPLE__ && defined __MACH__)
-#define findTsanFunction(f, fSig)                                              \
-  do {                                                                         \
-    if (NULL == (f = fSig dlsym(RTLD_DEFAULT, #f)))                            \
-      printf("Unable to find TSan function " #f ".\n");                        \
-  } while (0)
-
   findTsanFunction(AnnotateHappensAfter,
                    (void (*)(const char *, int, const volatile void *)));
   findTsanFunction(AnnotateHappensBefore,
                    (void (*)(const char *, int, const volatile void *)));
   findTsanFunction(AnnotateIgnoreWritesBegin, (void (*)(const char *, int)));
   findTsanFunction(AnnotateIgnoreWritesEnd, (void (*)(const char *, int)));
   findTsanFunction(
       AnnotateNewMemory,
       (void (*)(const char *, int, const volatile void *, size_t)));
   findTsanFunction(__tsan_func_entry, (void (*)(const void *)));
   findTsanFunction(__tsan_func_exit, (void (*)(void)));
-#endif
 
   SET_CALLBACK(thread_begin);
   SET_CALLBACK(thread_end);
   SET_CALLBACK(parallel_begin);
   SET_CALLBACK(implicit_task);
   SET_CALLBACK(sync_region);
   SET_CALLBACK(parallel_end);
 
static ompt_start_tool_result_t ompt_start_tool_result = {
       &ompt_tsan_initialize, &ompt_tsan_finalize, {0}};
 
   // The OMPT start-up code uses dlopen with RTLD_LAZY. Therefore, we cannot
   // rely on dlopen to fail if TSan is missing, but would get a runtime error
   // for the first TSan call. We use RunningOnValgrind to detect whether
   // an implementation of the Annotation interface is available in the
   // execution or disable the tool (by returning NULL).
 
-  runOnTsan = 1;
-  RunningOnValgrind();
-  if (!runOnTsan) // if we are not running on TSAN, give a different tool the
-                  // chance to be loaded
+  findTsanFunctionSilent(RunningOnValgrind, (int (*)(void)));
+  if (!RunningOnValgrind) // if we are not running on TSAN, give a different
+                          // tool the chance to be loaded
   {
     if (archer_flags->verbose)
       std::cout << "Archer detected OpenMP application without TSan "
                    "stopping operation"
                 << std::endl;
     delete archer_flags;
     return NULL;
   }
 
   if (archer_flags->verbose)
     std::cout << "Archer detected OpenMP application with TSan, supplying "
                  "OpenMP synchronization semantics"
               << std::endl;
   return &ompt_start_tool_result;
 }