Message ID | 20210719184637.1225275-1-siddhesh@sourceware.org |
---|---|
Headers | show |
Series | malloc hooks removal | expand |
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
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
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.
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
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...
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
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);
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
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
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