[Bug malloc/15592] New: mtrace.c tr_break() is not called from malloc hooks

[Bug malloc/15592] New: mtrace.c tr_break() is not called from malloc hooks

[fcollyer at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15592

            Bug ID: 15592
           Summary: mtrace.c tr_break() is not called from malloc hooks
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: malloc
          Assignee: unassigned at sourceware dot org
          Reporter: fcollyer at gmail dot com

Looking at some dumps generated on a CentOS 5.6, it seems as if gcc is
optimizing away some calls to tr_break().

Everything is in place (according to the file instructions), but tr_break() is
not firing after setting its breakpoint.

Small gdb disasm from glibc-2.5x:

Dump of assembler code for function tr_freehook:
0x0066ffc0 <tr_freehook+0>:     push   %ebp
0x0066ffc1 <tr_freehook+1>:     mov    %esp,%ebp
0x0066ffc3 <tr_freehook+3>:     sub    $0x18,%esp
0x0066ffc6 <tr_freehook+6>:     mov    0x8(%ebp),%eax
0x0066ffc9 <tr_freehook+9>:     mov    %ebx,-0xc(%ebp)
0x0066ffcc <tr_freehook+12>:    call   0x616ce0 <__i686.get_pc_thunk.bx>
0x0066ffd1 <tr_freehook+17>:    add    $0xd3023,%ebx
0x0066ffd7 <tr_freehook+23>:    mov    %esi,-0x8(%ebp)
0x0066ffda <tr_freehook+26>:    test   %eax,%eax
0x0066ffdc <tr_freehook+28>:    mov    %edi,-0x4(%ebp)
0x0066ffdf <tr_freehook+31>:    je     0x6700a4 <tr_freehook+228>
0x0066ffe5 <tr_freehook+37>:    xor    %edi,%edi
0x0066ffe7 <tr_freehook+39>:    mov    $0x1,%esi
0x0066ffec <tr_freehook+44>:    mov    %edi,%eax
0x0066ffee <tr_freehook+46>:    mov    %esi,%ecx
0x0066fff0 <tr_freehook+48>:    cmpl   $0x0,%gs:0xc
0x0066fff8 <tr_freehook+56>:    je     0x66fffb <tr_freehook+59>
0x0066fffa <tr_freehook+58>:    lock cmpxchg %ecx,0x1648(%ebx)
0x00670002 <tr_freehook+66>:    jne    0x67040c <_L_lock_464>
0x00670008 <tr_freehook+72>:    mov    0xc(%ebp),%eax
0x0067000b <tr_freehook+75>:    call   0x66fe70 <tr_where>
0x00670010 <tr_freehook+80>:    mov    0x8(%ebp),%eax
0x00670013 <tr_freehook+83>:    mov    %eax,0x8(%esp)
0x00670017 <tr_freehook+87>:    lea    -0x1b2a9(%ebx),%eax
0x0067001d <tr_freehook+93>:    mov    %eax,0x4(%esp)
0x00670021 <tr_freehook+97>:    mov    0x1640(%ebx),%eax
0x00670027 <tr_freehook+103>:   mov    %eax,(%esp)
0x0067002a <tr_freehook+106>:   call   0x646e20 <fprintf>
0x0067002f <tr_freehook+111>:   cmpl   $0x0,%gs:0xc
0x00670037 <tr_freehook+119>:   je     0x67003a <tr_freehook+122>
0x00670039 <tr_freehook+121>:   lock subl $0x1,0x1648(%ebx)
0x00670041 <tr_freehook+129>:   jne    0x67041c <_L_unlock_481>
---Type <return> to continue, or q <return> to quit---
0x00670047 <tr_freehook+135>:   mov    %edi,%eax
0x00670049 <tr_freehook+137>:   mov    %esi,%ecx
0x0067004b <tr_freehook+139>:   cmpl   $0x0,%gs:0xc
0x00670053 <tr_freehook+147>:   je     0x670056 <tr_freehook+150>
0x00670055 <tr_freehook+149>:   lock cmpxchg %ecx,0x1648(%ebx)
0x0067005d <tr_freehook+157>:   jne    0x67042c <_L_lock_490>
0x00670063 <tr_freehook+163>:   mov    0x164c(%ebx),%eax
0x00670069 <tr_freehook+169>:   mov    -0x38(%ebx),%esi
0x0067006f <tr_freehook+175>:   test   %eax,%eax
0x00670071 <tr_freehook+177>:   mov    %eax,(%esi)
0x00670073 <tr_freehook+179>:   je     0x6700b1 <tr_freehook+241>
0x00670075 <tr_freehook+181>:   mov    0xc(%ebp),%edx
0x00670078 <tr_freehook+184>:   mov    %edx,0x4(%esp)
0x0067007c <tr_freehook+188>:   mov    0x8(%ebp),%edx
0x0067007f <tr_freehook+191>:   mov    %edx,(%esp)
0x00670082 <tr_freehook+194>:   call   *%eax
0x00670084 <tr_freehook+196>:   lea    -0xd3034(%ebx),%eax
0x0067008a <tr_freehook+202>:   mov    %eax,(%esi)
0x0067008c <tr_freehook+204>:   cmpl   $0x0,%gs:0xc
0x00670094 <tr_freehook+212>:   je     0x670097 <tr_freehook+215>
0x00670096 <tr_freehook+214>:   lock subl $0x1,0x1648(%ebx)
0x0067009e <tr_freehook+222>:   jne    0x67043c <_L_unlock_517>
0x006700a4 <tr_freehook+228>:   mov    -0xc(%ebp),%ebx
0x006700a7 <tr_freehook+231>:   mov    -0x8(%ebp),%esi
0x006700aa <tr_freehook+234>:   mov    -0x4(%ebp),%edi
0x006700ad <tr_freehook+237>:   mov    %ebp,%esp
0x006700af <tr_freehook+239>:   pop    %ebp
0x006700b0 <tr_freehook+240>:   ret
0x006700b1 <tr_freehook+241>:   mov    0x8(%ebp),%eax
0x006700b4 <tr_freehook+244>:   mov    %eax,(%esp)
0x006700b7 <tr_freehook+247>:   call   0x66a990 <free>
0x006700bc <tr_freehook+252>:   jmp    0x670084 <tr_freehook+196>
End of assembler dump.

This can be mapped to the corresponding source-code:

http://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/mtrace.c;h=1a9522b09de37f96fb9e4ed807f3cc1dedaca3fb;hb=88cc61e84e8e75e6e91b1a2e51147aeb63712ff8

146   tr_where (caller);
147   /* Be sure to print it first.  */
148   fprintf (mallstream, "- %p\n", ptr);
149   __libc_lock_unlock (lock);
150   if (ptr == mallwatch)
151     tr_break ();
152   __libc_lock_lock (lock);

Inlined gcc tls lock code around tr_break() seems to be ok.
tr_break() related code seems to be missing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug malloc/15592] mtrace.c tr_break() is not called from malloc hooks

[neleai at seznam dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=15592

Ondrej Bilka <neleai at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |neleai at seznam dot cz

--- Comment #1 from Ondrej Bilka <neleai at seznam dot cz> ---
Created attachment 7233
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7233&action=edit
patch

Does marking tr_break noinline help? 
What about second trick of adding side effect as a __asm__ in attached patch
does?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug malloc/15592] mtrace.c tr_break() is not called from malloc hooks

[fcollyer at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15592

--- Comment #2 from Felipe Collyer <fcollyer at gmail dot com> ---
(In reply to Ondrej Bilka from comment #1)
> Created attachment 7233 [details]
> patch
> 
> Does marking tr_break noinline help? 
> What about second trick of adding side effect as a __asm__ in attached patch
> does?

The 2 techniques employed together may work. I don't have the resources at hand
to check (let me know if any final dso analysis helps).

As further notice, _dl_debug_state() follows similar tr_break() usage pattern.
But its address is taken (r_debug struct), defeating optimizations (some, I
believe).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug malloc/15592] mtrace.c tr_break() is not called from malloc hooks

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15592

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug malloc/15592] mtrace.c tr_break() is not called from malloc hooks

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=15592

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |WONTFIX
             Status|NEW                         |RESOLVED
                 CC|                            |siddhesh at sourceware dot org

--- Comment #3 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
tr_break has been removed in 2.34.

commit 00d28960c5388a582a0485e07629b553c32dde49
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Sat Jul 3 00:47:34 2021 +0530

    mtrace: Deprecate mallwatch and tr_break

    The variable and function pair appear to provide a way for users to
    set conditional breakpoints in mtrace when a specific address is
    returned by the allocator.  This can be achieved by using conditional
    breakpoints in gdb so it is redundant.  There is no documentation of
    this interface in the manual either, so it appears to have been a hack
    that got added to debug malloc.  Deprecate these symbols and do not
    call tr_break anymore.

    Reviewed-by: DJ Delorie <dj@redhat.com>
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.