[PATCH v4 00/10] Remove malloc hooks

[Carlos O'Donell]

On 7/2/21 7:38 AM, Siddhesh Poyarekar wrote:
> This patchset removes the malloc hooks __malloc_hook, __free_hook,
> __realloc_hook and __memalign_hook from the API and leaves compatibility
> symbols so that existing applications can continue to link to them.  The
> reading and execution of the hooks has been moved to a DSO
> libmalloc_compathooks.so, which can be preloaded for applications that
> need it.  By default these hooks no longer have any effect in the
> library.

OK. Good.
 
> Further, the __morecore, __morecore_after_hook and __default_morecore
> hooks have also been moved to compat symbols and removed from the API.
> Existing applications will continue to link to them but they won't have
> any effect on malloc behaviour.

OK. Good.

> To enable this, the MALLOC_CHECK_, mcheck() and mtrace() hooks have been
> weaned away from hooks.  In the process, some mcheck() failures have
> been fixed and the overall behaviour is now simpler to follow.  Finally,
> tr_break and mallwatch symbols have been deprecated and are not used
> anywhere.  Users are advised to use gdb watchpoints and conditional
> breakpoints to debug malloc internals since they ought to provide
> equivalent functionality.

OK.

Thanks for working through this.

This is looking better but we need to discuss the mcheck status and
dropping some of those interfaces, perhaps just deprecating libmcheck.a
and moving the functionality into libc_malloc_debug.so. See further review.

I think we're almost done with a v4 review if we agree on direction.

> Changes from v3:
> - Remove source file dependencies
> - Commit mcheck tests
> 
> Changes from v2:
> - Move hooks dependencies to malloc.o{,sS}
> 
> Changes from v1:
> 
> - Added makefile dependencies for the new hooks files
> - Fixed memset call in calloc debugging hooks
> - Added the tr_break deprecation patch and mcheck test patch to this
>   series
> 
> Siddhesh Poyarekar (10):
>   Drop source dependencies on hooks.c and arena.c
>   mtrace: Deprecate mallwatch and tr_break
>   Move glibc.malloc.check implementation into its own file
>   malloc: Move malloc hook references to hooks.c
>   glibc.malloc.check: Wean away from malloc hooks
>   mcheck: Wean away from malloc hooks
>   mtrace: Wean away from malloc hooks
>   Remove malloc hooks
>   Remove __after_morecore_hook
>   Remove __morecore and __default_morecore
> 
>  Makeconfig                                    |   2 +-
>  NEWS                                          |  18 +
>  include/malloc.h                              |  11 +-
>  include/stdlib.h                              |   3 -
>  malloc/Makefile                               |  23 +-
>  malloc/Versions                               |   3 +
>  malloc/arena.c                                |  26 +-
>  malloc/hooks.c                                | 545 +++++++-----------
>  malloc/malloc-check.c                         | 376 ++++++++++++
>  malloc/malloc-compathooks.c                   | 166 ++++++
>  malloc/malloc-internal.h                      |   6 +
>  malloc/malloc.c                               | 235 ++++----
>  malloc/malloc.h                               |  27 -
>  malloc/mcheck-hooks.c                         | 411 +++++++++++++
>  malloc/mcheck-init.c                          |  14 +-
>  malloc/mcheck.c                               | 369 +-----------
>  malloc/morecore.c                             |  15 +-
>  malloc/mtrace-hooks.c                         | 137 +++++
>  malloc/mtrace.c                               | 276 +--------
>  manual/memory.texi                            | 191 +-----
>  sysdeps/mach/hurd/i386/libc.abilist           |   1 +
>  sysdeps/unix/sysv/linux/aarch64/libc.abilist  |   1 +
>  sysdeps/unix/sysv/linux/alpha/libc.abilist    |   1 +
>  sysdeps/unix/sysv/linux/arc/libc.abilist      |   1 +
>  sysdeps/unix/sysv/linux/arm/be/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/arm/le/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/csky/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/hppa/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/i386/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/ia64/libc.abilist     |   1 +
>  .../sysv/linux/m68k/coldfire/libc.abilist     |   1 +
>  .../unix/sysv/linux/m68k/m680x0/libc.abilist  |   1 +
>  .../sysv/linux/microblaze/be/libc.abilist     |   1 +
>  .../sysv/linux/microblaze/le/libc.abilist     |   1 +
>  .../sysv/linux/mips/mips32/fpu/libc.abilist   |   1 +
>  .../sysv/linux/mips/mips32/nofpu/libc.abilist |   1 +
>  .../sysv/linux/mips/mips64/n32/libc.abilist   |   1 +
>  .../sysv/linux/mips/mips64/n64/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/nios2/libc.abilist    |   1 +
>  .../linux/powerpc/powerpc32/fpu/libc.abilist  |   1 +
>  .../powerpc/powerpc32/nofpu/libc.abilist      |   1 +
>  .../linux/powerpc/powerpc64/be/libc.abilist   |   1 +
>  .../linux/powerpc/powerpc64/le/libc.abilist   |   1 +
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |   1 +
>  .../unix/sysv/linux/riscv/rv64/libc.abilist   |   1 +
>  .../unix/sysv/linux/s390/s390-32/libc.abilist |   1 +
>  .../unix/sysv/linux/s390/s390-64/libc.abilist |   1 +
>  sysdeps/unix/sysv/linux/sh/be/libc.abilist    |   1 +
>  sysdeps/unix/sysv/linux/sh/le/libc.abilist    |   1 +
>  .../sysv/linux/sparc/sparc32/libc.abilist     |   1 +
>  .../sysv/linux/sparc/sparc64/libc.abilist     |   1 +
>  .../unix/sysv/linux/x86_64/64/libc.abilist    |   1 +
>  .../unix/sysv/linux/x86_64/x32/libc.abilist   |   1 +
>  53 files changed, 1547 insertions(+), 1340 deletions(-)
>  create mode 100644 malloc/malloc-check.c
>  create mode 100644 malloc/malloc-compathooks.c
>  create mode 100644 malloc/mcheck-hooks.c
>  create mode 100644 malloc/mtrace-hooks.c
> 


-- 
Cheers,
Carlos.