[PATCH 4/8] mcheck: Wean away from malloc hooks
Siddhesh Poyarekar
siddhesh@sourceware.org
Mon Jun 28 06:22:33 GMT 2021
On 6/25/21 4:21 AM, DJ Delorie via Libc-alpha wrote:
>
> Siddhesh Poyarekar <siddhesh@sourceware.org> writes:
>> Initialization using -lmcheck now depends on a new exported variable
>> __libc_lmcheck which is a const flag that mcheck-init can override.
>
> I wonder if we should bite the bullet and add an official malloc_ioctl()
> or something. Oh wait, we have mallopt() already. Can we hijack this
> instead of adding an ABI?
>
> mallopt (MALLOC_INTERNAL_GET_VER, 0) returns malloc_version + 1000
> (or returns 0 or 1, for "not supported")
>
> mallopt (MALLOC_INTERNAL_SET_DEBUG, debug_flags)
>
> This could be done in a .ctor/.init; I wonder if that would be early
> enough.
It won't be; it is likely that a ctor preceding the DSO that was linked
with -lmcheck could call malloc and hence result in ptmalloc_init being
called before the ctor mallopt is called.
I reckon it has to be an ELF hack. An exported symbol like
__libc_lmcheck seems like the cleanest way to do it.
We could add an ELF note, but typically notes don't impact execution so
that seems like a bad idea.
We could add a specially named segment (e.g. .glibc.lmcheck) with no
contents (or a single nop word) that ld.so reads to decide if mcheck
should be enabled. However that's not too different from __libc_lmcheck
in terms of ABI impact (we don't add a symbol version but its absence
will have the same effect) with the added complexity in the dynamic linker.
>> - if (__glibc_unlikely (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK)))
>> + if (__glibc_unlikely (__malloc_debugging_hooks))
>> {
>> - *victimp = malloc_check (bytes);
>> - if (*victimp != NULL)
>> - memset (*victimp, 0, bytes);
>> - return true;
>> + if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK)
>> + && malloc_mcheck_before (bytesp, victimp))
>> + return true;
>> + if (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK))
>> + {
>> + *victimp = malloc_check (*bytesp);
>> + return true;
>
> memset missing? no, moved to below... A comment would help here.
Added comment.
>> +static __always_inline void *
>> +_calloc_debug_after (void *mem, size_t bytes, const void *address)
>> +{
>> + if (__glibc_unlikely (__malloc_debugging_hooks) && mem != NULL)
>> + {
>> + if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK))
>> + mem = malloc_mcheck_after (mem, bytes);
>> + memset (mem, 0, bytes);
>> + }
>> + return mem;
>> +}
>
> So we memset if *any* hook is called, ok.
>
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index 60753446a1..5ea12d1d3b 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -3192,12 +3192,13 @@ __libc_malloc (size_t bytes)
>> {
>> mstate ar_ptr;
>> void *victim;
>> + size_t orig_bytes = bytes;
>
> Do we need to initialize victim to NULL here?
All paths initialize victim, but I've initialized it anyway and left it
to the compiler to optimize it away.
Thanks,
Siddhesh
More information about the Libc-alpha
mailing list