[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/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)

 #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
 }
 
 // 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.
static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
   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
+#undef findTsanFunction
 
   SET_CALLBACK(thread_begin);
   SET_CALLBACK(thread_end);
   SET_CALLBACK(parallel_begin);
   SET_CALLBACK(implicit_task);
   SET_CALLBACK(sync_region);
   SET_CALLBACK(parallel_end);