amputate MallocHook_InitAtFirstAllocation_HeapLeakChecker logic

src/malloc_hook.cc

@@ -62,78 +62,6 @@
 using std::copy;
 
 
-// Declaration of default weak initialization function, that can be overridden
-// by linking-in a strong definition (as heap-checker.cc does).  This is
-// extern "C" so that it doesn't trigger gold's --detect-odr-violations warning,
-// which only looks at C++ symbols.
-//
-// This function is declared here as weak, and defined later, rather than a more
-// straightforward simple weak definition, as a workround for an icc compiler
-// issue ((Intel reference 290819).  This issue causes icc to resolve weak
-// symbols too early, at compile rather than link time.  By declaring it (weak)
-// here, then defining it below after its use, we can avoid the problem.
-extern "C" {
-  ATTRIBUTE_WEAK int MallocHook_InitAtFirstAllocation_HeapLeakChecker() {
-    return 0;
-  }
-}
-
-namespace {
-
-bool RemoveInitialHooksAndCallInitializers();  // below.
-
-// These hooks are installed in MallocHook as the only initial hooks.  The first
-// hook that is called will run RemoveInitialHooksAndCallInitializers (see the
-// definition below) and then redispatch to any malloc hooks installed by
-// RemoveInitialHooksAndCallInitializers.
-//
-// Note(llib): there is a possibility of a race in the event that there are
-// multiple threads running before the first allocation.  This is pretty
-// difficult to achieve, but if it is then multiple threads may concurrently do
-// allocations.  The first caller will call
-// RemoveInitialHooksAndCallInitializers via one of the initial hooks.  A
-// concurrent allocation may, depending on timing either:
-// * still have its initial malloc hook installed, run that and block on waiting
-//   for the first caller to finish its call to
-//   RemoveInitialHooksAndCallInitializers, and proceed normally.
-// * occur some time during the RemoveInitialHooksAndCallInitializers call, at
-//   which point there could be no initial hooks and the subsequent hooks that
-//   are about to be set up by RemoveInitialHooksAndCallInitializers haven't
-//   been installed yet.  I think the worst we can get is that some allocations
-//   will not get reported to some hooks set by the initializers called from
-//   RemoveInitialHooksAndCallInitializers.
-//
-// Note, RemoveInitialHooksAndCallInitializers returns false if
-// MallocHook_InitAtFirstAllocation_HeapLeakChecker was already called
-// (i.e. through mmap hooks). And true otherwise (i.e. we're first to
-// call it). In that former case (return of false), we assume that
-// heap checker already installed it's hook, so we don't re-execute
-// new hook.
-void InitialNewHook(const void* ptr, size_t size) {
-  if (RemoveInitialHooksAndCallInitializers()) {
-    MallocHook::InvokeNewHook(ptr, size);
-  }
-}
-
-// This function is called at most once by one of the above initial malloc
-// hooks.  It removes all initial hooks and initializes all other clients that
-// want to get control at the very first memory allocation.  The initializers
-// may assume that the initial malloc hooks have been removed.  The initializers
-// may set up malloc hooks and allocate memory.
-bool RemoveInitialHooksAndCallInitializers() {
-  static tcmalloc::TrivialOnce once;
-  once.RunOnce([] () {
-    RAW_CHECK(MallocHook::RemoveNewHook(&InitialNewHook), "");
-  });
-
-  // HeapLeakChecker is currently the only module that needs to get control on
-  // the first memory allocation, but one can add other modules by following the
-  // same weak/strong function pattern.
-  return (MallocHook_InitAtFirstAllocation_HeapLeakChecker() != 0);
-}
-
-}  // namespace
-
 namespace base { namespace internal {
 
 // This lock is shared between all implementations of HookList::Add & Remove.
@@ -228,7 +156,7 @@ T HookList<T>::ExchangeSingular(T value) {
 // are instantiated.
 template struct HookList<MallocHook::NewHook>;
 
-HookList<MallocHook::NewHook> new_hooks_{InitialNewHook};
+HookList<MallocHook::NewHook> new_hooks_;
 HookList<MallocHook::DeleteHook> delete_hooks_;
 
 } }  // namespace base::internal

src/mmap_hook.cc

@@ -75,11 +75,6 @@
 # define __THROW   // __THROW is just an optimization, so ok to make it ""
 #endif
 
-// Used in initial hooks to call into heap checker
-// initialization. Defined empty and weak inside malloc_hooks and
-// proper definition is in heap_checker.cc
-extern "C" int MallocHook_InitAtFirstAllocation_HeapLeakChecker();
-
 namespace tcmalloc {
 
 namespace {
@@ -130,13 +125,6 @@ public:
   }
 
   int PreInvokeAll(const MappingEvent& evt) {
-    if (!ran_initial_hooks_.load(std::memory_order_relaxed)) {
-      bool already_ran = ran_initial_hooks_.exchange(true, std::memory_order_seq_cst);
-      if (!already_ran) {
-        MallocHook_InitAtFirstAllocation_HeapLeakChecker();
-      }
-    }
-
     int stack_depth = 0;
 
     std::atomic<MappingHookDescriptor*> *place = &list_head_;
@@ -163,7 +151,6 @@ public:
 
 private:
   std::atomic<MappingHookDescriptor*> list_head_{};
-  std::atomic<bool> ran_initial_hooks_{};
 
 } mapping_hooks;
 

src/tests/mmap_hook_test.cc

@@ -53,21 +53,6 @@
 
 #include "gtest/gtest.h"
 
-static bool got_first_allocation;
-
-extern "C" int MallocHook_InitAtFirstAllocation_HeapLeakChecker() {
-#ifndef __FreeBSD__
-  // Effing, FreeBSD. Super-annoying with broken everything when it is
-  // early.
-  printf("first mmap!\n");
-#endif
-  if (got_first_allocation) {
-    abort();
-  }
-  got_first_allocation = true;
-  return 1;
-}
-
 #ifdef HAVE_MMAP
 
 #include <fcntl.h>
@@ -147,8 +132,6 @@ TEST_F(MMapHookTest, MMap) {
   have_last_evt_ = false;
   ASSERT_FALSE(HasFatalFailure());
 
-  ASSERT_TRUE(got_first_allocation);
-
 #ifdef __linux__
   void* reserve = mmap(nullptr, pagesz * 2, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
   ASSERT_NE(reserve, MAP_FAILED) << "errno: " << strerror(errno);
@@ -270,8 +253,6 @@ TEST_F(MMapHookTest, Sbrk) {
 
   void* addr = tcmalloc_hooked_sbrk(8);
 
-  ASSERT_TRUE(got_first_allocation);
-
   EXPECT_TRUE(last_evt_.is_sbrk);
   EXPECT_TRUE(!last_evt_.before_valid && !last_evt_.file_valid && last_evt_.after_valid);
   EXPECT_EQ(last_evt_.after_address, addr);