abseil / abseil-cpp

Abseil Common Libraries (C++)
https://abseil.io
Apache License 2.0
14.69k stars 2.57k forks source link

dynamic_annotations symbols in the global namespace #122

Open MirkoBonadei opened 6 years ago

MirkoBonadei commented 6 years ago

Abseil dynamic_annotations (https://github.com/abseil/abseil-cpp/blob/master/absl/base/dynamic_annotations.h, https://github.com/abseil/abseil-cpp/blob/master/absl/base/dynamic_annotations.cc) define macros and functions without ABSL_ or Absl prefix (this is C code, so we cannot use namespaces).

This behavior can cause ODR violations and macros clashing. A possible solution would be to prefix all the macros with ABSL_ and all the functions with Absl. This will make Abseil compatible also with projects that are using something like https://code.google.com/archive/p/data-race-test/wikis/DynamicAnnotations.wiki.

philsc commented 5 years ago

Any thoughts from the Abseil team on this?

tensorflow appears to pull this in. That means it clashes with gperftools which also provides the same symbols.

derekmauro commented 5 years ago

@MirkoBonadei is working on renaming the symbols in thread_annotations.h, I don't know if he is also planning on working on renaming the symbols in dynamic_annotations.h. I wouldn't count on an immediate fix in any case.

The symbols in gperftools don't appear to be part of the public API of gperftools (they are part of Abseil's public API). In that case renaming them in gperftools might be an easier short-term solution.

philsc commented 5 years ago

Thanks @derekmauro. I renamed the gperftools symbols and it appears to work fine now.

yzhao1012 commented 4 years ago

@philsc Could you share the patch you did for gperftools? I ran into this same problem. :)

philsc commented 4 years ago

Here's what I did:

diff --git a/third_party/gperftools/src/base/dynamic_annotations.c b/third_party/gperftools/src/base/dynamic_annotations.c
index 87bd2ecde9..1b3cbff8cd 100644
--- a/third_party/gperftools/src/base/dynamic_annotations.c
+++ b/third_party/gperftools/src/base/dynamic_annotations.c
@@ -149,7 +149,7 @@ static int GetRunningOnValgrind(void) {
 }

 /* See the comments in dynamic_annotations.h */
-int RunningOnValgrind(void) {
+int GperfRunningOnValgrind(void) {
   static volatile int running_on_valgrind = -1;
   int local_running_on_valgrind = running_on_valgrind;
   /* C doesn't have thread-safe initialization of statics, and we
@@ -163,12 +163,12 @@ int RunningOnValgrind(void) {
 #endif  /* DYNAMIC_ANNOTATIONS_EXTERNAL_IMPL == 0 */

 /* See the comments in dynamic_annotations.h */
-double ValgrindSlowdown(void) {
-  /* Same initialization hack as in RunningOnValgrind(). */
+double GperfValgrindSlowdown(void) {
+  /* Same initialization hack as in GperfRunningOnValgrind(). */
   static volatile double slowdown = 0.0;
   double local_slowdown = slowdown;
   ANNOTATE_BENIGN_RACE(&slowdown, "safe hack");
-  if (RunningOnValgrind() == 0) {
+  if (GperfRunningOnValgrind() == 0) {
     return 1.0;
   }
   if (local_slowdown == 0.0) {
diff --git a/third_party/gperftools/src/base/dynamic_annotations.h b/third_party/gperftools/src/base/dynamic_annotations.h
index 4669315ced..71e205395b 100644
--- a/third_party/gperftools/src/base/dynamic_annotations.h
+++ b/third_party/gperftools/src/base/dynamic_annotations.h
@@ -501,23 +501,23 @@ void AnnotateFlushState(const char *file, int line);
   If for some reason you can't use "valgrind.h" or want to fake valgrind,
   there are two ways to make this function return non-zero:
     - Use environment variable: export RUNNING_ON_VALGRIND=1
-    - Make your tool intercept the function RunningOnValgrind() and
+    - Make your tool intercept the function GperfRunningOnValgrind() and
       change its return value.
  */
-int RunningOnValgrind(void);
+int GperfRunningOnValgrind(void);

-/* ValgrindSlowdown returns:
-    * 1.0, if (RunningOnValgrind() == 0)
-    * 50.0, if (RunningOnValgrind() != 0 && getenv("VALGRIND_SLOWDOWN") == NULL)
+/* GperfValgrindSlowdown returns:
+    * 1.0, if (GperfRunningOnValgrind() == 0)
+    * 50.0, if (GperfRunningOnValgrind() != 0 && getenv("VALGRIND_SLOWDOWN") == NULL)
     * atof(getenv("VALGRIND_SLOWDOWN")) otherwise
    This function can be used to scale timeout values:
    EXAMPLE:
    for (;;) {
      DoExpensiveBackgroundTask();
-     SleepForSeconds(5 * ValgrindSlowdown());
+     SleepForSeconds(5 * GperfValgrindSlowdown());
    }
  */
-double ValgrindSlowdown(void);
+double GperfValgrindSlowdown(void);

 #ifdef __cplusplus
 }
diff --git a/third_party/gperftools/src/base/sysinfo.cc b/third_party/gperftools/src/base/sysinfo.cc
index 36f706791a..006162c62e 100644
--- a/third_party/gperftools/src/base/sysinfo.cc
+++ b/third_party/gperftools/src/base/sysinfo.cc
@@ -58,7 +58,7 @@
 #endif
 #include "base/sysinfo.h"
 #include "base/commandlineflags.h"
-#include "base/dynamic_annotations.h"   // for RunningOnValgrind
+#include "base/dynamic_annotations.h"   // for GperfRunningOnValgrind
 #include "base/logging.h"

 #ifdef PLATFORM_WINDOWS
diff --git a/third_party/gperftools/src/base/vdso_support.cc b/third_party/gperftools/src/base/vdso_support.cc
index f88aa30312..eb3506c0ee 100644
--- a/third_party/gperftools/src/base/vdso_support.cc
+++ b/third_party/gperftools/src/base/vdso_support.cc
@@ -77,7 +77,7 @@ const void *VDSOSupport::Init() {
     // on stack, and so glibc works as if VDSO was not present.
     // But going directly to kernel via /proc/self/auxv below bypasses
     // Valgrind zapping. So we check for Valgrind separately.
-    if (RunningOnValgrind()) {
+    if (GperfRunningOnValgrind()) {
       vdso_base_ = NULL;
       return NULL;
     }
diff --git a/third_party/gperftools/src/debugallocation.cc b/third_party/gperftools/src/debugallocation.cc
index 7c438f25bc..6a217fb1fe 100644
--- a/third_party/gperftools/src/debugallocation.cc
+++ b/third_party/gperftools/src/debugallocation.cc
@@ -1158,14 +1158,14 @@ REGISTER_MODULE_INITIALIZER(debugallocation, {
   // Either we or valgrind will control memory management.  We
   // register our extension if we're the winner. Otherwise let
   // Valgrind use its own malloc (so don't register our extension).
-  if (!RunningOnValgrind()) {
+  if (!GperfRunningOnValgrind()) {
     DebugMallocImplementation *impl = new (debug_malloc_implementation_space.chars) DebugMallocImplementation();
     MallocExtension::Register(impl);
   }
 });

 REGISTER_MODULE_DESTRUCTOR(debugallocation, {
-  if (!RunningOnValgrind()) {
+  if (!GperfRunningOnValgrind()) {
     // When the program exits, check all blocks still in the free
     // queue for corruption.
     DanglingWriteChecker();
diff --git a/third_party/gperftools/src/heap-checker.cc b/third_party/gperftools/src/heap-checker.cc
index 8e71f58232..d1dd9ae91d 100755
--- a/third_party/gperftools/src/heap-checker.cc
+++ b/third_party/gperftools/src/heap-checker.cc
@@ -1967,7 +1967,7 @@ void HeapLeakChecker_InternalInitStart() {
       // turns out we do not need checking in the end; can stop profiling
       HeapLeakChecker::TurnItselfOffLocked();
       return;
-    } else if (RunningOnValgrind()) {
+    } else if (GperfRunningOnValgrind()) {
       // There is no point in trying -- we'll just fail.
       RAW_LOG(WARNING, "Can't run under Valgrind; will turn itself off");
       HeapLeakChecker::TurnItselfOffLocked();
diff --git a/third_party/gperftools/src/malloc_extension.cc b/third_party/gperftools/src/malloc_extension.cc
index e8ca31d7e4..b4b5480562 100644
--- a/third_party/gperftools/src/malloc_extension.cc
+++ b/third_party/gperftools/src/malloc_extension.cc
@@ -236,7 +236,7 @@ void MallocExtension::Register(MallocExtension* implementation) {
   // callers should be responsible for checking that they are the
   // malloc that is really being run, before calling Register.  This
   // is just here as an extra sanity check.)
-  if (!RunningOnValgrind()) {
+  if (!GperfRunningOnValgrind()) {
     current_instance = implementation;
   }
 }
diff --git a/third_party/gperftools/src/tcmalloc.cc b/third_party/gperftools/src/tcmalloc.cc
index 37c14407d9..2e0a992db7 100644
--- a/third_party/gperftools/src/tcmalloc.cc
+++ b/third_party/gperftools/src/tcmalloc.cc
@@ -117,7 +117,7 @@
 #include <gperftools/nallocx.h>
 #include "base/basictypes.h"            // for int64
 #include "base/commandlineflags.h"      // for RegisterFlagValidator, etc
-#include "base/dynamic_annotations.h"   // for RunningOnValgrind
+#include "base/dynamic_annotations.h"   // for GperfRunningOnValgrind
 #include "base/spinlock.h"              // for SpinLockHolder
 #include "central_freelist.h"  // for CentralFreeListPadded
 #include "common.h"            // for StackTrace, kPageShift, etc
@@ -1112,7 +1112,7 @@ TCMallocGuard::TCMallocGuard() {
 #ifdef TCMALLOC_USING_DEBUGALLOCATION
     // Let debugallocation register its extension.
 #else
-    if (RunningOnValgrind()) {
+    if (GperfRunningOnValgrind()) {
       // Let Valgrind uses its own malloc (so don't register our extension).
     } else {
       MallocExtension::Register(new TCMallocImplementation);
@@ -1124,7 +1124,7 @@ TCMallocGuard::TCMallocGuard() {
 TCMallocGuard::~TCMallocGuard() {
   if (--tcmallocguard_refcount == 0) {
     const char* env = NULL;
-    if (!RunningOnValgrind()) {
+    if (!GperfRunningOnValgrind()) {
       // Valgrind uses it's own malloc so we cannot do MALLOCSTATS
       env = getenv("MALLOCSTATS");
     }