diff mbox series

malloc: Fix malloc debug for 2.35 onwards

Message ID 20211113004047.1980486-1-shorne@gmail.com
State Committed
Headers show
Series malloc: Fix malloc debug for 2.35 onwards | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Stafford Horne Nov. 13, 2021, 12:40 a.m. UTC
The change 1e5a5866cb ("Remove malloc hooks [BZ #23328]") has broken
ports that are using GLIBC_2_35, like the new OpenRISC port I am working
on.

The libc_malloc_debug.so library used to bring in the debug
infrastructure is currently essentially empty for GLIBC_2_35 ports like
mine causing mtrace tests to fail:

    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.

The issue seems to be with the ifdefs in malloc/malloc-debug.c. The
ifdefs are currently essentially exluding all symbols for ports > 2.35.
In short:

    if (SHLIB_COMPAT 2.0 -> 2.34)
      debug api
      inlcude .c files
      implement apis

      Generate debug compat api
    endif

This fix changes this to:

    debug api
    inlcude .c files
    implement apis

    if (SHLIB_COMPAT 2.0 -> 2.34)
      Generate debug compat api
    endif

Fixes: 1e5a5866cb ("Remove malloc hooks [BZ #23328]")
Cc: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 malloc/malloc-debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Siddhesh Poyarekar Nov. 15, 2021, 2:34 a.m. UTC | #1
On 11/13/21 06:10, Stafford Horne wrote:
> The change 1e5a5866cb ("Remove malloc hooks [BZ #23328]") has broken
> ports that are using GLIBC_2_35, like the new OpenRISC port I am working
> on.
> 
> The libc_malloc_debug.so library used to bring in the debug
> infrastructure is currently essentially empty for GLIBC_2_35 ports like
> mine causing mtrace tests to fail:
> 
>      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.
> 
> The issue seems to be with the ifdefs in malloc/malloc-debug.c. The
> ifdefs are currently essentially exluding all symbols for ports > 2.35.
> In short:
> 
>      if (SHLIB_COMPAT 2.0 -> 2.34)
>        debug api
>        inlcude .c files
>        implement apis
> 
>        Generate debug compat api
>      endif
> 
> This fix changes this to:
> 
>      debug api
>      inlcude .c files
>      implement apis
> 
>      if (SHLIB_COMPAT 2.0 -> 2.34)
>        Generate debug compat api
>      endif

What symbols does this change generate?  Could you please share the 
output of objdump -T malloc/libc_malloc_debug.so from your build?

Thanks,
Siddhesh
Siddhesh Poyarekar Nov. 15, 2021, 2:40 a.m. UTC | #2
[sorry, hit send too soon]

On 11/15/21 08:04, Siddhesh Poyarekar wrote:
> What symbols does this change generate?  Could you please share the 
> output of objdump -T malloc/libc_malloc_debug.so from your build?
> 

Basically the idea of SHLIB_COMPAT usage here is twofold: first, to 
ensure that the symbol versions in libc_malloc_debug.so are at the exact 
same version as those in libc.so and second, to prevent linking against 
libc_malloc_debug.so.

In that sense, I think SHLIB_COMPAT usage is a hack.  The correct way 
would be to make a new macro like SHLIB_COMPAT, which creates 
non-default symbol version at the latest version of the libc symbol; 
i.e. for your port it would be malloc@GLIBC_2.35 and for x86_64 it would 
be malloc@GLIBC_2.2.5.

Siddhesh
Stafford Horne Nov. 15, 2021, 9:04 p.m. UTC | #3
On Mon, Nov 15, 2021 at 08:04:25AM +0530, Siddhesh Poyarekar wrote:
> On 11/13/21 06:10, Stafford Horne wrote:
> > The change 1e5a5866cb ("Remove malloc hooks [BZ #23328]") has broken
> > ports that are using GLIBC_2_35, like the new OpenRISC port I am working
> > on.
> > 
> > The libc_malloc_debug.so library used to bring in the debug
> > infrastructure is currently essentially empty for GLIBC_2_35 ports like
> > mine causing mtrace tests to fail:
> > 
> >      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.
> > 
> > The issue seems to be with the ifdefs in malloc/malloc-debug.c. The
> > ifdefs are currently essentially exluding all symbols for ports > 2.35.
> > In short:
> > 
> >      if (SHLIB_COMPAT 2.0 -> 2.34)
> >        debug api
> >        inlcude .c files
> >        implement apis
> > 
> >        Generate debug compat api
> >      endif
> > 
> > This fix changes this to:
> > 
> >      debug api
> >      inlcude .c files
> >      implement apis
> > 
> >      if (SHLIB_COMPAT 2.0 -> 2.34)
> >        Generate debug compat api
> >      endif
> 
> What symbols does this change generate?  Could you please share the output
> of objdump -T malloc/libc_malloc_debug.so from your build?

Here it is:

../build-glibc/malloc/libc_malloc_debug.so:     file format elf32-or1k

DYNAMIC SYMBOL TABLE:
00001114 l    d  .text	00000000              .text
0000a7c8 l    d  .rodata	00000000              .rodata
0000b7c0 l    d  .eh_frame	00000000              .eh_frame
0000df00 l    d  .tbss	00000000              .tbss
0000e000 l    d  .data	00000000              .data
0000e574 l    d  .bss	00000000              .bss
00000000      DF *UND*	00000000  GLIBC_2.35  fclose
00000000      DF *UND*	00000000  GLIBC_2.35  __libc_free
00000000      DF *UND*	00000000  GLIBC_PRIVATE __lll_lock_wake_private
00000000  w   D  *UND*	00000000  Base        _ITM_deregisterTMCloneTable
00000000      D  *UND*	00000000  GLIBC_PRIVATE errno
00000000      DF *UND*	00000000  GLIBC_2.35  __libc_memalign
00000000      DF *UND*	00000000  GLIBC_PRIVATE __libc_fatal
00000000      DO *UND*	00000000  GLIBC_2.35  _libc_intl_domainname
00000000      DF *UND*	00000000  GLIBC_PRIVATE __mmap
00000000      DF *UND*	00000000  GLIBC_2.35  __libc_malloc
00000000      DF *UND*	00000000  GLIBC_PRIVATE __munmap
00000000      DF *UND*	00000000  GLIBC_2.35  __assert_fail
00000000      DO *UND*	00000000  GLIBC_PRIVATE __libc_enable_secure
00000000      DF *UND*	00000000  GLIBC_2.35  dlsym
00000000      DF *UND*	00000000  GLIBC_2.35  mremap
00000000      DF *UND*	00000000  GLIBC_PRIVATE __open_nocancel
00000000      DF *UND*	00000000  GLIBC_2.35  setvbuf
00000000      DF *UND*	00000000  GLIBC_2.35  sprintf
00000000      DF *UND*	00000000  GLIBC_2.35  __sbrk
00000000  w   DF *UND*	00000000  GLIBC_2.35  __cxa_finalize
00000000      DF *UND*	00000000  GLIBC_2.35  __dcgettext
00000000      DF *UND*	00000000  GLIBC_PRIVATE __mprotect
00000000      DO *UND*	00000000  GLIBC_PRIVATE _rtld_global_ro
00000000      DF *UND*	00000000  GLIBC_2.35  fopen
00000000      DF *UND*	00000000  GLIBC_2.35  funlockfile
00000000      DF *UND*	00000000  GLIBC_2.35  sysconf
00000000      DF *UND*	00000000  GLIBC_2.35  flockfile
00000000      DF *UND*	00000000  GLIBC_PRIVATE __read_nocancel
00000000      DO *UND*	00000000  GLIBC_2.35  stderr
00000000      DF *UND*	00000000  GLIBC_2.35  memset
00000000      DF *UND*	00000000  GLIBC_2.35  __libc_realloc
00000000      DF *UND*	00000000  GLIBC_2.35  fprintf
00000000      DF *UND*	00000000  GLIBC_2.35  secure_getenv
00000000      DF *UND*	00000000  GLIBC_2.35  __libc_freeres
00000000      DF *UND*	00000000  GLIBC_PRIVATE __madvise
00000000      DF *UND*	00000000  GLIBC_2.35  strlen
00000000      DF *UND*	00000000  GLIBC_2.35  memcpy
00000000      DF *UND*	00000000  GLIBC_2.35  dladdr
00000000      DF *UND*	00000000  GLIBC_PRIVATE __tunable_get_val
00000000      DF *UND*	00000000  GLIBC_2.35  __cxa_atexit
00000000  w   D  *UND*	00000000  Base        _ITM_registerTMCloneTable
00000000      DF *UND*	00000000  GLIBC_PRIVATE __lll_lock_wait_private
00000000      DF *UND*	00000000  GLIBC_PRIVATE __close_nocancel
00000000      DF *UND*	00000000  GLIBC_2.35  fwrite
0000e498 g    DO .data	00000004  GLIBC_2.35  __realloc_hook
0000e7c8 g    DO .bss	00000004  GLIBC_2.35  __free_hook
00000000 g    DO *ABS*	00000000  GLIBC_2.35  GLIBC_2.35
00009814 g    DF .text	00000024  GLIBC_2.35  mcheck_pedantic
00009c2c g    DF .text	00000104  GLIBC_2.35  pvalloc
000095fc g    DF .text	0000011c  GLIBC_2.35  free
000099c8 g    DF .text	000001a4  GLIBC_2.35  realloc
0000a470 g    DF .text	00000138  GLIBC_2.35  mallinfo2
00009d30 g    DF .text	000000a8  GLIBC_2.35  valloc
00004450 g    DF .text	00000024  GLIBC_2.35  mcheck_check_all
00004474 g    DF .text	00000020  GLIBC_2.35  mprobe
0000e494 g    DO .data	00000004  GLIBC_2.35  __memalign_hook
00009bb4 g    DF .text	00000030  GLIBC_2.35  aligned_alloc
0000e49c g    DO .data	00000004  GLIBC_2.35  __malloc_hook
0000a1b4 g    DF .text	000000f8  GLIBC_2.35  malloc_info
000044b4 g    DF .text	00000020  GLIBC_2.35  muntrace
0000a138 g    DF .text	0000007c  GLIBC_2.35  malloc_usable_size
00009dd8 g    DF .text	000000bc  GLIBC_2.35  posix_memalign
000097f0 g    DF .text	00000024  GLIBC_2.35  mcheck
00004494 g    DF .text	00000020  GLIBC_2.35  mtrace
0000a3a4 g    DF .text	000000cc  GLIBC_2.35  malloc_stats
00009e94 g    DF .text	000002a4  GLIBC_2.35  calloc
0000a6e0 g    DF .text	000000e8  GLIBC_2.35  malloc_trim
00009bb4 g    DF .text	00000030  GLIBC_2.35  memalign
0000a5a8 g    DF .text	00000138  GLIBC_2.35  mallinfo
000091a0 g    DF .text	00000180  GLIBC_2.35  malloc
0000a2ac g    DF .text	000000f8  GLIBC_2.35  mallopt
Stafford Horne Nov. 15, 2021, 9:07 p.m. UTC | #4
On Mon, Nov 15, 2021 at 08:10:42AM +0530, Siddhesh Poyarekar wrote:
> [sorry, hit send too soon]
> 
> On 11/15/21 08:04, Siddhesh Poyarekar wrote:
> > What symbols does this change generate?  Could you please share the
> > output of objdump -T malloc/libc_malloc_debug.so from your build?
> > 
> 
> Basically the idea of SHLIB_COMPAT usage here is twofold: first, to ensure
> that the symbol versions in libc_malloc_debug.so are at the exact same
> version as those in libc.so and second, to prevent linking against
> libc_malloc_debug.so.
> 
> In that sense, I think SHLIB_COMPAT usage is a hack.  The correct way would
> be to make a new macro like SHLIB_COMPAT, which creates non-default symbol
> version at the latest version of the libc symbol; i.e. for your port it
> would be malloc@GLIBC_2.35 and for x86_64 it would be malloc@GLIBC_2.2.5.

I am not sure, after my patch the SHLIB_COMPAT usage is just there for compat
symbols.  As you mention using SHLIB_COMPAT to ensure libc_malloc_debug.so and
libc.so have the same version seems strange, I would think that would be
controlled by shlib-version or done out of the box.

-Stafford
Siddhesh Poyarekar Nov. 15, 2021, 10:47 p.m. UTC | #5
On 11/16/21 02:37, Stafford Horne wrote:
> I am not sure, after my patch the SHLIB_COMPAT usage is just there for compat
> symbols.  As you mention using SHLIB_COMPAT to ensure libc_malloc_debug.so and
> libc.so have the same version seems strange, I would think that would be
> controlled by shlib-version or done out of the box.

Right, so it mostly works, i.e. you have symbols with version names:

...
> 00009bb4 g    DF .text	00000030  GLIBC_2.35  memalign
> 0000a5a8 g    DF .text	00000138  GLIBC_2.35  mallinfo
> 000091a0 g    DF .text	00000180  GLIBC_2.35  malloc
> 0000a2ac g    DF .text	000000f8  GLIBC_2.35  mallopt
...

but they're default versions, which is not desirable since we don't want 
this library to be linkable.  So you will need a new macro that always 
creates the non-default symbol at the version that it was introduced.

Siddhesh
Stafford Horne Nov. 16, 2021, 5:12 a.m. UTC | #6
On Tue, Nov 16, 2021 at 04:17:10AM +0530, Siddhesh Poyarekar wrote:
> On 11/16/21 02:37, Stafford Horne wrote:
> > I am not sure, after my patch the SHLIB_COMPAT usage is just there for compat
> > symbols.  As you mention using SHLIB_COMPAT to ensure libc_malloc_debug.so and
> > libc.so have the same version seems strange, I would think that would be
> > controlled by shlib-version or done out of the box.
> 
> Right, so it mostly works, i.e. you have symbols with version names:
> 
> ...
> > 00009bb4 g    DF .text	00000030  GLIBC_2.35  memalign
> > 0000a5a8 g    DF .text	00000138  GLIBC_2.35  mallinfo
> > 000091a0 g    DF .text	00000180  GLIBC_2.35  malloc
> > 0000a2ac g    DF .text	000000f8  GLIBC_2.35  mallopt
> ...
> 
> but they're default versions, which is not desirable since we don't want
> this library to be linkable.  So you will need a new macro that always
> creates the non-default symbol at the version that it was introduced.

I think I get what you mean.  Is this something you are planning to sort out?
As is without this patch the mem tests are failing on my port, and any future
ports.  For existing ports it is working because of the HACK as you say.

-Stafford
Siddhesh Poyarekar Nov. 16, 2021, 5:15 a.m. UTC | #7
On 11/16/21 10:42, Stafford Horne wrote:
> On Tue, Nov 16, 2021 at 04:17:10AM +0530, Siddhesh Poyarekar wrote:
>> On 11/16/21 02:37, Stafford Horne wrote:
>>> I am not sure, after my patch the SHLIB_COMPAT usage is just there for compat
>>> symbols.  As you mention using SHLIB_COMPAT to ensure libc_malloc_debug.so and
>>> libc.so have the same version seems strange, I would think that would be
>>> controlled by shlib-version or done out of the box.
>>
>> Right, so it mostly works, i.e. you have symbols with version names:
>>
>> ...
>>> 00009bb4 g    DF .text	00000030  GLIBC_2.35  memalign
>>> 0000a5a8 g    DF .text	00000138  GLIBC_2.35  mallinfo
>>> 000091a0 g    DF .text	00000180  GLIBC_2.35  malloc
>>> 0000a2ac g    DF .text	000000f8  GLIBC_2.35  mallopt
>> ...
>>
>> but they're default versions, which is not desirable since we don't want
>> this library to be linkable.  So you will need a new macro that always
>> creates the non-default symbol at the version that it was introduced.
> 
> I think I get what you mean.  Is this something you are planning to sort out?
> As is without this patch the mem tests are failing on my port, and any future
> ports.  For existing ports it is working because of the HACK as you say.

I was hoping you'd do that ;)  If you're unable to, I'll try to get to 
it later in the week.

Siddhesh
Stafford Horne Nov. 16, 2021, 5:29 a.m. UTC | #8
On Tue, Nov 16, 2021 at 10:45:17AM +0530, Siddhesh Poyarekar wrote:
> On 11/16/21 10:42, Stafford Horne wrote:
> > On Tue, Nov 16, 2021 at 04:17:10AM +0530, Siddhesh Poyarekar wrote:
> > > On 11/16/21 02:37, Stafford Horne wrote:
> > > > I am not sure, after my patch the SHLIB_COMPAT usage is just there for compat
> > > > symbols.  As you mention using SHLIB_COMPAT to ensure libc_malloc_debug.so and
> > > > libc.so have the same version seems strange, I would think that would be
> > > > controlled by shlib-version or done out of the box.
> > > 
> > > Right, so it mostly works, i.e. you have symbols with version names:
> > > 
> > > ...
> > > > 00009bb4 g    DF .text	00000030  GLIBC_2.35  memalign
> > > > 0000a5a8 g    DF .text	00000138  GLIBC_2.35  mallinfo
> > > > 000091a0 g    DF .text	00000180  GLIBC_2.35  malloc
> > > > 0000a2ac g    DF .text	000000f8  GLIBC_2.35  mallopt
> > > ...
> > > 
> > > but they're default versions, which is not desirable since we don't want
> > > this library to be linkable.  So you will need a new macro that always
> > > creates the non-default symbol at the version that it was introduced.
> > 
> > I think I get what you mean.  Is this something you are planning to sort out?
> > As is without this patch the mem tests are failing on my port, and any future
> > ports.  For existing ports it is working because of the HACK as you say.
> 
> I was hoping you'd do that ;)  If you're unable to, I'll try to get to it
> later in the week.

Sure :), Let me have a look at it.

Thanks,

-Stafford
diff mbox series

Patch

diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 3d7e6d44fd..6f2b6a2d23 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -24,7 +24,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 *);
@@ -640,6 +639,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);