glibc.malloc.check: Wean away from malloc hooks

The malloc-check debugging feature is tightly integrated into glibc
malloc, so thanks to an idea from Florian Weimer, much of the malloc
implementation has been moved into libc_malloc_debug.so to support
malloc-check.  Due to this, glibc malloc and malloc-check can no
longer work together; they use altogether different (but identical)
structures for heap management.  This should not make a difference
though since the malloc check hook is not disabled anywhere.
malloc_set_state does, but it does so early enough that it shouldn't
cause any problems.

The malloc check tunable is now in the debug DSO and has no effect
when the DSO is not preloaded.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

41 files changed, 447 insertions, 80 deletions

diff --git a/malloc/Makefile b/malloc/Makefile
index eb5f5560bb..b89af21d19 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -133,7 +133,7 @@ test-extras = \
 libmemusage-routines = memusage
 libmemusage-inhibit-o = $(filter-out .os,$(object-suffixes))
 
-libc_malloc_debug-routines = malloc-debug
+libc_malloc_debug-routines = malloc-debug $(sysdep_malloc_debug_routines)
 libc_malloc_debug-inhibit-o = $(filter-out .os,$(object-suffixes))
 
 $(objpfx)tst-malloc-backtrace: $(shared-thread-library)
@@ -251,6 +251,7 @@ tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 \
 
 tst-mxfast-ENV = GLIBC_TUNABLES=glibc.malloc.tcache_count=0:glibc.malloc.mxfast=0
 
+CPPFLAGS-malloc-debug.c += -DUSE_TCACHE=0
 ifeq ($(experimental-malloc),yes)
 CPPFLAGS-malloc.c += -DUSE_TCACHE=1
 else
diff --git a/malloc/Versions b/malloc/Versions
index 71d933de19..cbb73d18c1 100644
--- a/malloc/Versions
+++ b/malloc/Versions
@@ -119,13 +119,25 @@ libc_malloc_debug {
     mprobe;
     mtrace;
     muntrace;
+
+    mallinfo;
+    malloc_stats;
+    malloc_trim;
+    malloc_usable_size;
+    mallopt;
   }
   GLIBC_2.2 {
     mcheck_check_all;
     mcheck_pedantic;
     posix_memalign;
   }
+  GLIBC_2.10 {
+    malloc_info;
+  }
   GLIBC_2.16 {
     aligned_alloc;
   }
+  GLIBC_2.33 {
+    mallinfo2;
+  }
 }
diff --git a/malloc/arena.c b/malloc/arena.c
index 840426f9fb..edcaa8816d 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -79,7 +79,9 @@ static __thread mstate thread_arena attribute_tls_model_ie;
    acquired after free_list_lock has been acquired.  */
 
 __libc_lock_define_initialized (static, free_list_lock);
+#if IS_IN (libc)
 static size_t narenas = 1;
+#endif
 static mstate free_list;
 
 /* list_lock prevents concurrent writes to the next member of struct
@@ -207,14 +209,6 @@ __malloc_fork_unlock_child (void)
 }
 
 #if HAVE_TUNABLES
-static void
-TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
-{
-  int32_t value = (int32_t) valp->numval;
-  if (value != 0)
-    __malloc_check_init ();
-}
-
 # define TUNABLE_CALLBACK_FNDECL(__name, __type) \
 static inline int do_ ## __name (__type value);				      \
 static void									      \
@@ -309,7 +303,7 @@ ptmalloc_init (void)
     }
 #endif
 
-#ifdef SHARED
+#if defined SHARED && IS_IN (libc)
   /* In case this libc copy is in a non-default namespace, never use
      brk.  Likewise if dlopened from statically linked program.  The
      generic sbrk implementation also enforces this, but it is not
@@ -323,7 +317,6 @@ ptmalloc_init (void)
   malloc_init_state (&main_arena);
 
 #if HAVE_TUNABLES
-  TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
   TUNABLE_GET (top_pad, size_t, TUNABLE_CALLBACK (set_top_pad));
   TUNABLE_GET (perturb, int32_t, TUNABLE_CALLBACK (set_perturb_byte));
   TUNABLE_GET (mmap_threshold, size_t, TUNABLE_CALLBACK (set_mmap_threshold));
@@ -401,8 +394,6 @@ ptmalloc_init (void)
             }
         }
     }
-  if (s && s[0] != '\0' && s[0] != '0')
-    __malloc_check_init ();
 #endif
 }
 
@@ -672,6 +663,7 @@ heap_trim (heap_info *heap, size_t pad)
 
 /* Create a new arena with initial size "size".  */
 
+#if IS_IN (libc)
 /* If REPLACED_ARENA is not NULL, detach it from this thread.  Must be
    called while free_list_lock is held.  */
 static void
@@ -947,6 +939,7 @@ arena_get_retry (mstate ar_ptr, size_t bytes)
 
   return ar_ptr;
 }
+#endif
 
 void
 __malloc_arena_thread_freeres (void)
diff --git a/malloc/hooks.c b/malloc/hooks.c
index 8e9fefe6c3..6c212fbc21 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -39,8 +39,6 @@ void *weak_variable (*__malloc_hook) (size_t, const void *) = NULL;
 void *weak_variable (*__realloc_hook) (void *, size_t, const void *) = NULL;
 void *weak_variable (*__memalign_hook) (size_t, size_t, const void *) = NULL;
 
-#include "malloc-check.c"
-
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_25)
 
 /* Support for restoring dumped heaps contained in historic Emacs
@@ -108,13 +106,6 @@ malloc_set_state (void *msptr)
      calls calloc and thus must be called only afterwards, so there
      cannot be more than one thread when we reach this point.  */
 
-  /* Disable the malloc hooks (and malloc checking).  */
-  __malloc_hook = NULL;
-  __realloc_hook = NULL;
-  __free_hook = NULL;
-  __memalign_hook = NULL;
-  using_malloc_checking = 0;
-
   /* Patch the dumped heap.  We no longer try to integrate into the
      existing heap.  Instead, we mark the existing chunks as mmapped.
      Together with the update to dumped_main_arena_start and
diff --git a/malloc/malloc-check.c b/malloc/malloc-check.c
index dcab880510..a444c7478e 100644
--- a/malloc/malloc-check.c
+++ b/malloc/malloc-check.c
@@ -17,20 +17,8 @@
    License along with the GNU C Library; see the file COPYING.LIB.  If
    not, see <https://www.gnu.org/licenses/>.  */
 
-
-/* Whether we are using malloc checking.  */
-static int using_malloc_checking;
-
-/* Activate a standard set of debugging hooks. */
-void
-__malloc_check_init (void)
-{
-  using_malloc_checking = 1;
-  __malloc_hook = malloc_check;
-  __free_hook = free_check;
-  __realloc_hook = realloc_check;
-  __memalign_hook = memalign_check;
-}
+#define __mremap mremap
+#include "malloc.c"
 
 /* When memory is tagged, the checking data is stored in the user part
    of the chunk.  We can't rely on the user not having modified the
@@ -63,14 +51,13 @@ magicbyte (const void *p)
    must reach it with this iteration, otherwise we have witnessed a memory
    corruption.  */
 static size_t
-malloc_check_get_size (mchunkptr p)
+malloc_check_get_size (void *mem)
 {
   size_t size;
   unsigned char c;
+  mchunkptr p = mem2chunk (mem);
   unsigned char magic = magicbyte (p);
 
-  assert (using_malloc_checking == 1);
-
   for (size = CHUNK_HDR_SZ + memsize (p) - 1;
        (c = *SAFE_CHAR_OFFSET (p, size)) != magic;
        size -= c)
@@ -203,7 +190,7 @@ top_check (void)
 }
 
 static void *
-malloc_check (size_t sz, const void *caller)
+malloc_check (size_t sz)
 {
   void *victim;
   size_t nb;
@@ -222,7 +209,7 @@ malloc_check (size_t sz, const void *caller)
 }
 
 static void
-free_check (void *mem, const void *caller)
+free_check (void *mem)
 {
   mchunkptr p;
 
@@ -256,7 +243,7 @@ free_check (void *mem, const void *caller)
 }
 
 static void *
-realloc_check (void *oldmem, size_t bytes, const void *caller)
+realloc_check (void *oldmem, size_t bytes)
 {
   INTERNAL_SIZE_T chnb;
   void *newmem = 0;
@@ -269,11 +256,11 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
       return NULL;
     }
   if (oldmem == 0)
-    return malloc_check (bytes, NULL);
+    return malloc_check (bytes);
 
   if (bytes == 0)
     {
-      free_check (oldmem, NULL);
+      free_check (oldmem);
       return NULL;
     }
 
@@ -348,12 +335,12 @@ invert:
 }
 
 static void *
-memalign_check (size_t alignment, size_t bytes, const void *caller)
+memalign_check (size_t alignment, size_t bytes)
 {
   void *mem;
 
   if (alignment <= MALLOC_ALIGNMENT)
-    return malloc_check (bytes, NULL);
+    return malloc_check (bytes);
 
   if (alignment < MINSIZE)
     alignment = MINSIZE;
@@ -363,14 +350,14 @@ memalign_check (size_t alignment, size_t bytes, const void *caller)
   if (alignment > SIZE_MAX / 2 + 1)
     {
       __set_errno (EINVAL);
-      return 0;
+      return NULL;
     }
 
   /* Check for overflow.  */
   if (bytes > SIZE_MAX - alignment - MINSIZE)
     {
       __set_errno (ENOMEM);
-      return 0;
+      return NULL;
     }
 
   /* Make sure alignment is power of 2.  */
@@ -388,3 +375,27 @@ memalign_check (size_t alignment, size_t bytes, const void *caller)
   __libc_lock_unlock (main_arena.mutex);
   return mem2mem_check (tag_new_usable (mem), bytes);
 }
+
+static void
+TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp)
+{
+  int32_t value = (int32_t) valp->numval;
+  if (value != 0)
+    __malloc_debug_enable (MALLOC_CHECK_HOOK);
+}
+
+static bool
+initialize_malloc_check (void)
+{
+  /* This is the copy of the malloc initializer that we pulled in along with
+     malloc-check.  This does not affect any of the libc malloc structures.  */
+  ptmalloc_init ();
+#if HAVE_TUNABLES
+  TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
+#else
+  const char *s = secure_getenv ("MALLOC_CHECK_");
+  if (s && s[0] != '\0' && s[0] != '0')
+    __malloc_debug_enable (MALLOC_CHECK_HOOK);
+#endif
+  return __is_malloc_debug_enabled (MALLOC_CHECK_HOOK);
+}
diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 9942124e02..f5290aaa6d 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -50,6 +50,7 @@ enum malloc_debug_hooks
   MALLOC_NONE_HOOK = 0,
   MALLOC_MCHECK_HOOK = 1 << 0, /* mcheck()  */
   MALLOC_MTRACE_HOOK = 1 << 1, /* mtrace()  */
+  MALLOC_CHECK_HOOK = 1 << 2,  /* MALLOC_CHECK_ or glibc.malloc.check.  */
 };
 static unsigned __malloc_debugging_hooks;
 
@@ -73,6 +74,7 @@ __malloc_debug_disable (enum malloc_debug_hooks flag)
 
 #include "mcheck.c"
 #include "mtrace.c"
+#include "malloc-check.c"
 
 extern void (*__malloc_initialize_hook) (void);
 compat_symbol_reference (libc, __malloc_initialize_hook,
@@ -105,13 +107,18 @@ generic_hook_ini (void)
   __malloc_hook = NULL;
   __realloc_hook = NULL;
   __memalign_hook = NULL;
-  /* The compiler does not know that these functions are allocators, so it will
-     not try to optimize it away.  */
-  __libc_free (__libc_malloc (0));
+
+  /* malloc check does not quite co-exist with libc malloc, so initialize
+     either on or the other.  */
+  if (!initialize_malloc_check ())
+    /* The compiler does not know that these functions are allocators, so it
+       will not try to optimize it away.  */
+    __libc_free (__libc_malloc (0));
 
   void (*hook) (void) = __malloc_initialize_hook;
   if (hook != NULL)
     (*hook)();
+
   debug_initialized = 1;
 }
 
@@ -149,10 +156,11 @@ __debug_malloc (size_t bytes)
 
   void *victim = NULL;
   size_t orig_bytes = bytes;
-  if (!__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK)
-      || !malloc_mcheck_before (&bytes, &victim))
+  if ((!__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK)
+       || !malloc_mcheck_before (&bytes, &victim)))
     {
-      victim = __libc_malloc (bytes);
+      victim = (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)
+		? malloc_check (bytes) : __libc_malloc (bytes));
     }
   if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK) && victim != NULL)
     victim = malloc_mcheck_after (victim, orig_bytes);
@@ -175,10 +183,13 @@ __debug_free (void *mem)
 
   if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK))
     mem = free_mcheck (mem);
+
+  if (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK))
+    free_check (mem);
+  else
+    __libc_free (mem);
   if (__is_malloc_debug_enabled (MALLOC_MTRACE_HOOK))
     free_mtrace (mem, RETURN_ADDRESS (0));
-
-  __libc_free (mem);
 }
 strong_alias (__debug_free, free)
 
@@ -193,10 +204,13 @@ __debug_realloc (void *oldmem, size_t bytes)
   size_t orig_bytes = bytes, oldsize = 0;
   void *victim = NULL;