mbox series

[v10,00/11] malloc hooks removal

Message ID 20210719184637.1225275-1-siddhesh@sourceware.org
Headers show
Series malloc hooks removal | expand

Message

Siddhesh Poyarekar July 19, 2021, 6:46 p.m. UTC
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
libc_malloc_debug.so, which can be preloaded for applications that need
it.  By default these hooks no longer have any effect in the library.

Further, debugging features such as MALLOC_CHECK_, mcheck() and mtrace
have been weaned away from these hooks and also moved to
libc_malloc_debug.so.  With this change, these features are only enabled
when libc_malloc_debug.so is preloaded using LD_PRELOAD.

Finally, 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.

Testing:

- x86_64 - --prefix=/usr:
  No new failures
- x86_64, i686, s390x, ppc64le, armv7l, aarch64 - Fedora config:
  No new failures
- build-many-glibcs:
  In progress

Changes from v9:
- Moved malloc_{g,s}et_state to libc_malloc_debug.so
- Moved undumping support to libc_malloc_debug.so
- Reverted reallocarray interposition

Changes from v8:
- Make hook-dependent tests conditional on GLIBC_2.23 instead of
  GLIBC_2.24
- Interpose reallocarray.

Changes from v7:
- Made mcheck tests conditional on GLIBC_2.24
- Added SHLIB_COMPAT guard around __malloc_initialize_hook usage in
  libc_malloc_debug.so
- Fixed hurd build failure.
- Add another patch to fix malloc_usable_size to mcheck

Changes from v6:
- Moved malloc-check into libc_malloc_debug.so. Tweaked malloc.c to
  allow building twice, once inside libc.so and next in
  libc_malloc_debug.so
- Included morecore.c into malloc.c
- Moved hook initialization too into libc_malloc_debug.so.
- Interposed more functions in libc_malloc_debug.so
- Versioned all symbols exported from libc_malloc_debug.so and finalized
  them so that the library cannot be linked against

Changes from v5:
- Rebased on latest master
- Fixed realloc-mcheck
- Removed residual mention of libmalloc_compathooks
- Removed LD_PRELOAD from elf/tst-setuid

Changes from v4:
- Patchset has a different approach, starting with moving out hooks
  first to restrict all major malloc.c changes to the first patch
- Renamed libmalloc_compathooks.so to libc_malloc_debug.so
- Moved all debugging features into libc_malloc_debug.so
- Made more documentation updates
- Simplified __malloc_initialized variable use
- Removed debugging tests on static variables since that is no longer
  supported

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 (11):
  Make mcheck tests conditional on GLIBC_2.23 or earlier
  Remove __after_morecore_hook
  Remove __morecore and __default_morecore
  Move malloc hooks into a compat DSO
  mcheck: Wean away from malloc hooks [BZ #23489]
  Simplify __malloc_initialized
  mtrace: Wean away from malloc hooks
  glibc.malloc.check: Wean away from malloc hooks
  Move malloc_{g,s}et_state to libc_malloc_debug
  Remove malloc hooks [BZ #23328]
  mcheck Fix malloc_usable_size [BZ #22057]

 NEWS                                          |  26 +
 Rules                                         |   9 +-
 catgets/Makefile                              |   4 +-
 elf/Makefile                                  |  15 +-
 elf/tst-leaks1-static.c                       |   1 -
 iconvdata/Makefile                            |   3 +-
 include/malloc.h                              |   6 -
 include/mcheck.h                              |   4 -
 include/stdlib.h                              |   3 -
 intl/tst-gettext.sh                           |   1 +
 libio/Makefile                                |  12 +-
 localedata/Makefile                           |   3 +-
 malloc/Makefile                               |  63 +-
 malloc/Versions                               |  47 +-
 malloc/arena.c                                |  48 +-
 malloc/hooks.c                                | 177 +----
 malloc/malloc-check.c                         |  63 +-
 malloc/malloc-debug.c                         | 671 ++++++++++++++++++
 malloc/malloc-hooks.h                         |  24 -
 malloc/malloc.c                               | 218 ++----
 malloc/malloc.h                               |  27 -
 malloc/mcheck-impl.c                          | 412 +++++++++++
 malloc/mcheck.c                               | 397 +----------
 malloc/morecore.c                             |  34 +-
 malloc/mtrace-impl.c                          | 226 ++++++
 malloc/mtrace.c                               | 313 +-------
 malloc/tst-compathooks-off.c                  | 145 ++++
 malloc/tst-compathooks-on.c                   |   2 +
 malloc/tst-malloc-usable-static-tunables.c    |   1 -
 malloc/tst-malloc-usable-static.c             |   1 -
 malloc/tst-mtrace.sh                          |   1 +
 manual/memory.texi                            | 207 +-----
 manual/tunables.texi                          |   4 +-
 misc/Makefile                                 |   6 +-
 nptl/Makefile                                 |   3 +-
 posix/Makefile                                |  48 +-
 resolv/Makefile                               |   9 +-
 shlib-versions                                |   3 +
 stdio-common/Makefile                         |  15 +-
 sysdeps/aarch64/Makefile                      |   3 +
 sysdeps/generic/libc_malloc_debug.abilist     |   0
 sysdeps/generic/localplt.data                 |   1 -
 sysdeps/mach/hurd/Makefile                    |   1 +
 sysdeps/mach/hurd/i386/libc.abilist           |   2 -
 .../mach/hurd/i386/libc_malloc_debug.abilist  |  28 +
 sysdeps/mach/hurd/i386/localplt.data          |   1 -
 sysdeps/pthread/Makefile                      |   3 +-
 sysdeps/unix/sysv/linux/aarch64/libc.abilist  |   2 -
 .../linux/aarch64/libc_malloc_debug.abilist   |  28 +
 sysdeps/unix/sysv/linux/aarch64/localplt.data |   1 -
 sysdeps/unix/sysv/linux/alpha/libc.abilist    |   2 -
 .../linux/alpha/libc_malloc_debug.abilist     |  28 +
 sysdeps/unix/sysv/linux/alpha/localplt.data   |   1 -
 .../sysv/linux/arc/libc_malloc_debug.abilist  |  26 +
 sysdeps/unix/sysv/linux/arc/localplt.data     |   1 -
 sysdeps/unix/sysv/linux/arm/be/libc.abilist   |   2 -
 .../linux/arm/be/libc_malloc_debug.abilist    |  28 +
 sysdeps/unix/sysv/linux/arm/le/libc.abilist   |   2 -
 .../linux/arm/le/libc_malloc_debug.abilist    |  28 +
 sysdeps/unix/sysv/linux/arm/localplt.data     |   1 -
 .../sysv/linux/csky/libc_malloc_debug.abilist |  26 +
 sysdeps/unix/sysv/linux/csky/localplt.data    |   1 -
 sysdeps/unix/sysv/linux/hppa/libc.abilist     |   2 -
 .../sysv/linux/hppa/libc_malloc_debug.abilist |  28 +
 sysdeps/unix/sysv/linux/hppa/localplt.data    |   1 -
 sysdeps/unix/sysv/linux/hppa/shlib-versions   |   2 +
 sysdeps/unix/sysv/linux/i386/libc.abilist     |   2 -
 .../sysv/linux/i386/libc_malloc_debug.abilist |  28 +
 sysdeps/unix/sysv/linux/i386/localplt.data    |   1 -
 sysdeps/unix/sysv/linux/ia64/libc.abilist     |   2 -
 .../sysv/linux/ia64/libc_malloc_debug.abilist |  28 +
 sysdeps/unix/sysv/linux/ia64/localplt.data    |   1 -
 sysdeps/unix/sysv/linux/ia64/shlib-versions   |   2 +
 .../sysv/linux/m68k/coldfire/libc.abilist     |   2 -
 .../m68k/coldfire/libc_malloc_debug.abilist   |  28 +
 .../sysv/linux/m68k/coldfire/localplt.data    |   1 -
 .../unix/sysv/linux/m68k/m680x0/libc.abilist  |   2 -
 .../m68k/m680x0/libc_malloc_debug.abilist     |  28 +
 .../unix/sysv/linux/m68k/m680x0/localplt.data |   1 -
 .../sysv/linux/microblaze/be/libc.abilist     |   2 -
 .../microblaze/be/libc_malloc_debug.abilist   |  28 +
 .../sysv/linux/microblaze/le/libc.abilist     |   2 -
 .../microblaze/le/libc_malloc_debug.abilist   |  28 +
 .../unix/sysv/linux/microblaze/localplt.data  |   1 -
 .../sysv/linux/mips/mips32/fpu/libc.abilist   |   2 -
 .../mips/mips32/fpu/libc_malloc_debug.abilist |  28 +
 .../sysv/linux/mips/mips32/nofpu/libc.abilist |   2 -
 .../mips32/nofpu/libc_malloc_debug.abilist    |  28 +
 .../sysv/linux/mips/mips64/n32/libc.abilist   |   2 -
 .../mips/mips64/n32/libc_malloc_debug.abilist |  28 +
 .../sysv/linux/mips/mips64/n64/libc.abilist   |   2 -
 .../mips/mips64/n64/libc_malloc_debug.abilist |  28 +
 sysdeps/unix/sysv/linux/mips/shlib-versions   |   2 +
 sysdeps/unix/sysv/linux/nios2/libc.abilist    |   2 -
 .../linux/nios2/libc_malloc_debug.abilist     |  28 +
 sysdeps/unix/sysv/linux/nios2/localplt.data   |   1 -
 .../linux/powerpc/powerpc32/fpu/libc.abilist  |   2 -
 .../powerpc32/fpu/libc_malloc_debug.abilist   |  28 +
 .../linux/powerpc/powerpc32/fpu/localplt.data |   1 -
 .../powerpc/powerpc32/nofpu/libc.abilist      |   2 -
 .../powerpc32/nofpu/libc_malloc_debug.abilist |  28 +
 .../powerpc/powerpc32/nofpu/localplt.data     |   1 -
 .../linux/powerpc/powerpc64/be/libc.abilist   |   2 -
 .../powerpc64/be/libc_malloc_debug.abilist    |  28 +
 .../linux/powerpc/powerpc64/le/libc.abilist   |   2 -
 .../powerpc64/le/libc_malloc_debug.abilist    |  28 +
 .../linux/powerpc/powerpc64/localplt.data     |   1 -
 sysdeps/unix/sysv/linux/riscv/localplt.data   |   1 -
 .../riscv/rv32/libc_malloc_debug.abilist      |  26 +
 .../riscv/rv64/libc_malloc_debug.abilist      |  26 +
 sysdeps/unix/sysv/linux/s390/localplt.data    |   1 -
 .../unix/sysv/linux/s390/s390-32/libc.abilist |   2 -
 .../s390/s390-32/libc_malloc_debug.abilist    |  28 +
 .../unix/sysv/linux/s390/s390-64/libc.abilist |   2 -
 .../s390/s390-64/libc_malloc_debug.abilist    |  28 +
 sysdeps/unix/sysv/linux/sh/be/libc.abilist    |   2 -
 .../linux/sh/be/libc_malloc_debug.abilist     |  28 +
 sysdeps/unix/sysv/linux/sh/le/libc.abilist    |   2 -
 .../linux/sh/le/libc_malloc_debug.abilist     |  28 +
 sysdeps/unix/sysv/linux/sh/localplt.data      |   1 -
 sysdeps/unix/sysv/linux/sh/shlib-versions     |   1 +
 .../sysv/linux/sparc/sparc32/libc.abilist     |   2 -
 .../sparc/sparc32/libc_malloc_debug.abilist   |  28 +
 .../sysv/linux/sparc/sparc32/localplt.data    |   1 -
 .../sysv/linux/sparc/sparc64/libc.abilist     |   2 -
 .../sparc/sparc64/libc_malloc_debug.abilist   |  28 +
 .../sysv/linux/sparc/sparc64/localplt.data    |   1 -
 .../sysv/linux/sparc/sparc64/shlib-versions   |   1 +
 .../unix/sysv/linux/x86_64/64/libc.abilist    |   2 -
 .../linux/x86_64/64/libc_malloc_debug.abilist |  28 +
 .../unix/sysv/linux/x86_64/x32/libc.abilist   |   2 -
 .../x86_64/x32/libc_malloc_debug.abilist      |  28 +
 sysdeps/x86_64/localplt.data                  |   1 -
 133 files changed, 2775 insertions(+), 1489 deletions(-)
 delete mode 100644 elf/tst-leaks1-static.c
 create mode 100644 malloc/malloc-debug.c
 delete mode 100644 malloc/malloc-hooks.h
 create mode 100644 malloc/mcheck-impl.c
 create mode 100644 malloc/mtrace-impl.c
 create mode 100644 malloc/tst-compathooks-off.c
 create mode 100644 malloc/tst-compathooks-on.c
 delete mode 100644 malloc/tst-malloc-usable-static-tunables.c
 delete mode 100644 malloc/tst-malloc-usable-static.c
 create mode 100644 sysdeps/generic/libc_malloc_debug.abilist
 create mode 100644 sysdeps/mach/hurd/i386/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/alpha/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/arc/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/arm/be/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/arm/le/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/csky/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/hppa/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/i386/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/ia64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/m68k/coldfire/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/m68k/m680x0/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/microblaze/be/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/microblaze/le/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/mips/mips32/fpu/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n32/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/nios2/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/riscv/rv32/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/riscv/rv64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/sh/be/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/sh/le/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/64/libc_malloc_debug.abilist
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/libc_malloc_debug.abilist

Comments

Siddhesh Poyarekar July 21, 2021, 9:45 a.m. UTC | #1
On 7/20/21 12:16 AM, Siddhesh Poyarekar via Libc-alpha 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
> libc_malloc_debug.so, which can be preloaded for applications that need
> it.  By default these hooks no longer have any effect in the library.
> 
> Further, debugging features such as MALLOC_CHECK_, mcheck() and mtrace
> have been weaned away from these hooks and also moved to
> libc_malloc_debug.so.  With this change, these features are only enabled
> when libc_malloc_debug.so is preloaded using LD_PRELOAD.
> 
> Finally, 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.
> 
> Testing:
> 
> - x86_64 - --prefix=/usr:
>    No new failures
> - x86_64, i686, s390x, ppc64le, armv7l, aarch64 - Fedora config:
>    No new failures
> - build-many-glibcs:
>    In progress

Finally after a few hurdles I have managed to finish the 
build-many-glibcs run and it has finished cleanly with the exception of 
alpha-linux-gnu, which failed when building compilers.  It appears to be 
due to the fix for pr27283 not being on the 2.36 branch.

Siddhesh
Stafford Horne Oct. 8, 2021, 9:47 p.m. UTC | #2
On Tue, Jul 20, 2021 at 3:48 AM Siddhesh Poyarekar via Libc-alpha
<libc-alpha@sourceware.org> 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
> libc_malloc_debug.so, which can be preloaded for applications that need
> it.  By default these hooks no longer have any effect in the library.
>
> Further, debugging features such as MALLOC_CHECK_, mcheck() and mtrace
> have been weaned away from these hooks and also moved to
> libc_malloc_debug.so.  With this change, these features are only enabled
> when libc_malloc_debug.so is preloaded using LD_PRELOAD.
>
> Finally, 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.

Hello,

I am working on an OpenRISC port and have just rebased and picked up
this series.  I
also have updated the api version to 2.35.  After this all of the
mtrace related *-mem
tests seem to be failing.

Is this expected to be completely removed after going to 2.35 and the
compatibility APIs
get compiled out?  If so, then the tests should be skipped, if not
then then something is broken
for glibcs with base version 2.35:

For example:

    cat sysdeps/unix/sysv/linux/or1k/shlib-versions
    DEFAULT                 GLIBC_2.35
    ld=ld-linux-or1k.so.1

    FAIL: posix/bug-glob2-mem
    FAIL: posix/bug-regex14-mem
    FAIL: posix/bug-regex2-mem
    FAIL: posix/bug-regex21-mem
    FAIL: posix/bug-regex31-mem
    FAIL: posix/bug-regex36-mem
    FAIL: malloc/tst-mtrace.

When I have looked into mtrace I can see a few strange things:

  In libc.so mtrace is a nop (that seems to be what you wanted for this series).
    000b5bb4 <mtrace>:
       b5bb4:       44 00 48 00     l.jr r9
       b5bb8:       15 00 00 00     l.nop 0x0

    000b5bbc <muntrace>:
       b5bbc:       44 00 48 00     l.jr r9
       b5bc0:       15 00 00 00     l.nop 0x0

Also, I am also seeing basically nothing in libc_malloc_debug.so  either:

    < shorne@antec ~/work/gnu-toolchain/build-glibc > nm
../build-glibc/malloc/libc_malloc_debug.so
    000000f4 r __abi_tag
    00004020 b completed.0
             w __cxa_finalize@GLIBC_2.35
    00000314 t deregister_tm_clones
    00000404 t __do_global_dtors_aux
    00003f0c d __do_global_dtors_aux_fini_array_entry
    00004000 d __dso_handle
    00003f10 a _DYNAMIC
    00000484 t frame_dummy
    00003f08 d __frame_dummy_init_array_entry
    0000048c r __FRAME_END__
    00000000 A GLIBC_2.35
    00004004 a _GLOBAL_OFFSET_TABLE_
             w _ITM_deregisterTMCloneTable
             w _ITM_registerTMCloneTable
    0000037c t register_tm_clones
    00004004 d __TMC_END__

-Stafford
DJ Delorie Oct. 8, 2021, 10:22 p.m. UTC | #3
Have you backported 2d2d9f2b48a9 ? The tests that use the built-in check
either won't work, or require an LD_PRELOAD to include the externalized
replacement; there were a couple patches that cleaned all that up.
Stafford Horne Oct. 8, 2021, 11:18 p.m. UTC | #4
On Sat, Oct 9, 2021 at 7:22 AM DJ Delorie <dj@redhat.com> wrote:
>
>
> Have you backported 2d2d9f2b48a9 ? The tests that use the built-in check
> either won't work, or require an LD_PRELOAD to include the externalized
> replacement; there were a couple patches that cleaned all that up.
>

Yes, I rebased september 26th.  That patch was pushed in july and it's there.

Is the issue in malloc/malloc-debug.c?  There is a big if for
GLIBC_2_0 -> GLIBC_2_34.
It seems from GLIBC_2_35 nothing is available any longer.  Is that
correct, that is what is happening.

Maybe mtrace.c should be getting linked in to malloc-debug, I don't
see that happening.

#if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_34)
/* Support only the glibc allocators.  */
extern void *__libc_malloc (size_t);
extern void __libc_free (void *);
extern void *__libc_realloc (void *, size_t);
extern void *__libc_memalign (size_t, size_t);
extern void *__libc_valloc (size_t);
extern void *__libc_pvalloc (size_t);
extern void *__libc_calloc (size_t, size_t);
...

My git history showing the patches are there:

$ git lo -n20 -- NEWS
2021-09-27 6e85a96b9a Stafford Horne   or1k: Add documentation
2021-09-24 8807e560c0 Joseph Myers     Define __STDC_IEC_60559_BFP__
and __STDC_IEC_60559_COMPLEX__
2021-09-22 b3f27d8150 Joseph Myers     Add narrowing fma functions
2021-09-19 a93d9e03a3 H.J. Lu          Extend struct r_debug to
support multiple namespaces [BZ #15971]
2021-09-10 abd383584b Joseph Myers     Add narrowing square root functions
2021-09-06 466f2be6c0 Carlos O'Donell  Add generic C.UTF-8 locale (Bug 17318)
2021-08-01 a85c93c424 Carlos O'Donell  Open master branch for glibc
2.35 development  (tag: glibc-2.34.9000)
2021-08-01 2e2c08aa4d Carlos O'Donell  Update NEWS.
2021-08-01 cfdaa29f66 Mark Harris      NEWS: Fix typos, grammar, and
missing words
2021-07-22 1e5a5866cb Siddhesh Poyarekar Remove malloc hooks [BZ #23328]
2021-07-22 0552fd2c7d Siddhesh Poyarekar Move malloc_{g,s}et_state to
libc_malloc_debug
*2021-07-22 2d2d9f2b48 Siddhesh Poyarekar Move malloc hooks into a compat DSO
2021-07-22 55a4dd3930 Siddhesh Poyarekar Remove __morecore and
__default_morecore
DJ Delorie Oct. 8, 2021, 11:24 p.m. UTC | #5
Stafford Horne <shorne@gmail.com> writes:
> Is the issue in malloc/malloc-debug.c?  There is a big if for
> GLIBC_2_0 -> GLIBC_2_34.
> It seems from GLIBC_2_35 nothing is available any longer.  Is that
> correct, that is what is happening.

Yes.  Old ports get compatibility symbols that do nothing.  New ports
get no symbols since there are no old apps that would have been linked
against them.

If yours is the first new port since we made this change, there might be
some debugging needed...
Stafford Horne Oct. 8, 2021, 11:29 p.m. UTC | #6
On Sat, Oct 9, 2021 at 8:24 AM DJ Delorie <dj@redhat.com> wrote:
>
> Stafford Horne <shorne@gmail.com> writes:
> > Is the issue in malloc/malloc-debug.c?  There is a big if for
> > GLIBC_2_0 -> GLIBC_2_34.
> > It seems from GLIBC_2_35 nothing is available any longer.  Is that
> > correct, that is what is happening.
>
> Yes.  Old ports get compatibility symbols that do nothing.  New ports
> get no symbols since there are no old apps that would have been linked
> against them.
>
> If yours is the first new port since we made this change, there might be
> some debugging needed...

I think maybe this is the issue.  The first #if is not terminated, and
it keeps the include of mtrace.c/mcheck.c from being included.  Those
includes should provide the real implementations of mtrace/muntrace to
be overloaded with the LD_PRELOAD.

I am not sure where we should end the if though.  I can test something.

#if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_34)
/* Support only the glibc allocators.  */
extern void *__libc_malloc (size_t);
extern void __libc_free (void *);
extern void *__libc_realloc (void *, size_t);
extern void *__libc_memalign (size_t, size_t);
extern void *__libc_valloc (size_t);
extern void *__libc_pvalloc (size_t);
extern void *__libc_calloc (size_t, size_t);

....

#include "mcheck.c"
#include "mtrace.c"
#include "malloc-check.c"

#if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_24)
extern void (*__malloc_initialize_hook) (void);
compat_symbol_reference (libc, __malloc_initialize_hook,
                         __malloc_initialize_hook, GLIBC_2_0);
#endif
Stafford Horne Oct. 8, 2021, 11:48 p.m. UTC | #7
On Sat, Oct 09, 2021 at 08:29:47AM +0900, Stafford Horne wrote:
> On Sat, Oct 9, 2021 at 8:24 AM DJ Delorie <dj@redhat.com> wrote:
> >
> > Stafford Horne <shorne@gmail.com> writes:
> > > Is the issue in malloc/malloc-debug.c?  There is a big if for
> > > GLIBC_2_0 -> GLIBC_2_34.
> > > It seems from GLIBC_2_35 nothing is available any longer.  Is that
> > > correct, that is what is happening.
> >
> > Yes.  Old ports get compatibility symbols that do nothing.  New ports
> > get no symbols since there are no old apps that would have been linked
> > against them.
> >
> > If yours is the first new port since we made this change, there might be
> > some debugging needed...
> 
> I think maybe this is the issue.  The first #if is not terminated, and
> it keeps the include of mtrace.c/mcheck.c from being included.  Those
> includes should provide the real implementations of mtrace/muntrace to
> be overloaded with the LD_PRELOAD.
> 
> I am not sure where we should end the if though.  I can test something.
> 

Hi

The below seems to fix the issue for me, I am not sure how correct it is.

The symbols now look correct too, I uploaded the list as below:
  nm ../build-glibc/malloc/libc_malloc_debug.so
  https://gist.github.com/stffrdhrn/2766877eef2b16e2dc3629ed4eb2086a

-Stafford

---


diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 9922ef5f25..74ea9cb283 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -23,7 +23,6 @@
 #include <unistd.h>
 #include <sys/param.h>
 
-#if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_34)
 /* Support only the glibc allocators.  */
 extern void *__libc_malloc (size_t);
 extern void __libc_free (void *);
@@ -639,6 +638,7 @@ compat_symbol (libc_malloc_debug, malloc_set_state,
malloc_set_state,
               GLIBC_2_0);
 #endif
 
+#if SHLIB_COMPAT (libc_malloc_debug, GLIBC_2_0, GLIBC_2_34)
 /* Do not allow linking against the library.  */
 compat_symbol (libc_malloc_debug, aligned_alloc, aligned_alloc, GLIBC_2_16);
 compat_symbol (libc_malloc_debug, calloc, calloc, GLIBC_2_0);
Siddhesh Poyarekar Oct. 11, 2021, 1:55 a.m. UTC | #8
On 10/9/21 03:17, Stafford Horne wrote:
> I am working on an OpenRISC port and have just rebased and picked up
> this series.  I
> also have updated the api version to 2.35.  After this all of the
> mtrace related *-mem
> tests seem to be failing.
> 
> Is this expected to be completely removed after going to 2.35 and the
> compatibility APIs
> get compiled out?  If so, then the tests should be skipped, if not
> then then something is broken
> for glibcs with base version 2.35:
> 
> For example:
> 
>      cat sysdeps/unix/sysv/linux/or1k/shlib-versions
>      DEFAULT                 GLIBC_2.35
>      ld=ld-linux-or1k.so.1
> 
>      FAIL: posix/bug-glob2-mem
>      FAIL: posix/bug-regex14-mem
>      FAIL: posix/bug-regex2-mem
>      FAIL: posix/bug-regex21-mem
>      FAIL: posix/bug-regex31-mem
>      FAIL: posix/bug-regex36-mem
>      FAIL: malloc/tst-mtrace.
> 
> When I have looked into mtrace I can see a few strange things:
> 
>    In libc.so mtrace is a nop (that seems to be what you wanted for this series).
>      000b5bb4 <mtrace>:
>         b5bb4:       44 00 48 00     l.jr r9
>         b5bb8:       15 00 00 00     l.nop 0x0
> 
>      000b5bbc <muntrace>:
>         b5bbc:       44 00 48 00     l.jr r9
>         b5bc0:       15 00 00 00     l.nop 0x0

This is expected...

> 
> Also, I am also seeing basically nothing in libc_malloc_debug.so  either:
> 
>      < shorne@antec ~/work/gnu-toolchain/build-glibc > nm
> ../build-glibc/malloc/libc_malloc_debug.so
>      000000f4 r __abi_tag
>      00004020 b completed.0
>               w __cxa_finalize@GLIBC_2.35
>      00000314 t deregister_tm_clones
>      00000404 t __do_global_dtors_aux
>      00003f0c d __do_global_dtors_aux_fini_array_entry
>      00004000 d __dso_handle
>      00003f10 a _DYNAMIC
>      00000484 t frame_dummy
>      00003f08 d __frame_dummy_init_array_entry
>      0000048c r __FRAME_END__
>      00000000 A GLIBC_2.35
>      00004004 a _GLOBAL_OFFSET_TABLE_
>               w _ITM_deregisterTMCloneTable
>               w _ITM_registerTMCloneTable
>      0000037c t register_tm_clones
>      00004004 d __TMC_END__

... but this is not.  Could you please share how you're configuring and 
building this, including versions and configure options of (presumably) 
the cross toolchains?  I'll try and figure out why the debugging 
function entry points are missing.

Thanks,
Siddhesh
Siddhesh Poyarekar Oct. 11, 2021, 2:19 a.m. UTC | #9
On 10/11/21 07:25, Siddhesh Poyarekar via Libc-alpha wrote:
> ... but this is not.  Could you please share how you're configuring and 
> building this, including versions and configure options of (presumably) 
> the cross toolchains?  I'll try and figure out why the debugging 
> function entry points are missing.

... and I've now noticed that there's an actual conversation and not 
just this email.  Reading through.

Siddhesh
Siddhesh Poyarekar Oct. 11, 2021, 2:40 a.m. UTC | #10
On 10/9/21 05:18, Stafford Horne via Libc-alpha wrote:
> Hi
> 
> The below seems to fix the issue for me, I am not sure how correct it is.
> 

I misused the compatibility symbol framework to disallow linking against 
libc_malloc_debug.so and as a result, newer ports just don't get any 
malloc debugging at all as a result.  What we need here is to make the 
symbols following the "Do not allow linking against the library." 
comment non-default.

The fix you posted simply makes the symbols in libc_malloc_debug.so at 
default versions, which isn't what we want.  I suppose we need a new 
macro similar to compat_symbol (i.e. it should wrap around 
_set_symbol_version) but not conditional on SHLIB_COMPAT.

Siddhesh