Re: [PATCH] manual: Document replacing malloc [BZ #20424]

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 03/05/17 10:08, Florian Weimer wrote:
> On 04/24/2017 07:17 PM, Szabolcs Nagy wrote:
>> On 20/04/17 11:53, Florian Weimer wrote:
>>> +@node Replacing malloc
>>> +@subsection Replacing @code{malloc}
>>> +
>>> +@cindex @code{malloc} replacement
>>> +@cindex @code{LD_PRELOAD} and @code{malloc}
>>> +@cindex alternative @code{malloc} implementations
>>> +@cindex customizing @code{malloc}
>>> +@cindex interposing @code{malloc}
>>> +@cindex preempting @code{malloc}
>>> +@cindex replacing @code{malloc}
>>> +@Theglibc{} supports replacing the built-in @code{malloc} implementation
>>> +with a different allocator with the same interface.  For dynamically
>>> +linked programs, this happens through ELF symbol interposition, either
>>> +using shared object dependencies or @code{LD_PRELOAD}.  For static
>>> +linking, the @code{malloc} replacement library must be linked in before
>>> +linking against @code{libc.a} (explicitly or implicitly).
>>> +
>>
>> this documentation does not mention known caveats, e.g.
> 
> There are many more, like following ABI requirements regarding alignment and pointer bit patterns.
> 
>> - when wrapping calloc via dlsym, dlsym may call calloc, the
>>    user has to deal with it,
>>
>> - similarly any interface that internally may use malloc (in
>>    the future) better not be used in the malloc implementation.
>>
>> - malloc may be called when locks are held (e.g. some stdio
>>    lock during scanf) so synchronizing with anything that might
>>    also hold the same lock in the malloc implementation may
>>    deadlock (a more interesting example is probably the dl_load_lock
>>    while allocating dynamic tls in some cases so a user provided
>>    malloc should not use dtls)
> 
> It should be able to use initial-exec TLS, though.
> 
> Is it really worthwhile to go into such details?  The malloc/fork interaction is something mostly dependent on
> malloc implementation details.
> 
> I can add a general warning to the documentation that implementing malloc is not easy, but I'm not sure how
> helpful it would be.
> 

i don't want to block this patch, it is better to
specify the list of symbols to interpose than not
saying anything on the matter.

> The symbol list is mainly there because jemalloc forgot to override __libc_memalign for older glibcs, and we
> don't want a repeat of that.
> 

i remember other cases of malloc interposition bugs
mostly related to glibc internals calling malloc in
unexpected situations, so it is worth specifying
the internal interface contract in more detail at
some point.

> Thanks,
> Florian