[Openmp-commits] [PATCH] D103607: [OpenMP][Tools] Fix Archer for MACOS

Sun Jun 6 09:14:14 PDT 2021

protze.joachim added inline comments.

================
Comment at: openmp/tools/archer/ompt-tsan.cpp:164
   fptr = (int (*)())dlsym(RTLD_DEFAULT, "RunningOnValgrind");
-  if (fptr && fptr != RunningOnValgrind)
+  if (!fptr || fptr == RunningOnValgrind)
     runOnTsan = 0;
----------------
Hahnfeld wrote:
> Can you explain this change? Why do we disable TSan if `dlsym` finds the current function? (`RunningOnValgrind` resolves to the current function, right?)
I never had a chance to test the library on Mac. While preparing this patch, I tested the library on my Linux system by manually enabling the ifdefs for mac. From my understanding the library should never have worked on Mac.

Line 1090 sets `runOnTsan=1`. If any other `RunningOnValgrind` is found (and executed in the weak function approach), the function will not modify `runOnTsan`. If no other `RunningOnValgrind` is found, the function should set `runOnTsan=0`. In this case, Archer will return NULL from ompt_start_tool and allow other tools to get loaded.

I think, the general question is whether it's worth to still have all the symbols in the library or whether we should allow `dlopen` to fail if the application is not built with TSan? The OpenMP runtime now has `OMP_TOOL_VERBOSE_INIT` which allows to understand why dlopen failed.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103607/new/

https://reviews.llvm.org/D103607