Re: [RFC PATCH] malloc_usable_size.3: Warn about _FORTIFY_SOURCE interaction

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-04-04 22:35, Alejandro Colomar wrote:
> This might be useful to you: I happen to comaintain a code base that
> uses malloc_usable_size(3).  I have no idea why it was added, and it
> seems to be used in a test, but not in the actual program, which makes
> me happy to not have to fix that :).  Maybe it is useful to you to check
> that code to see why would some heavily-optimized code base want to use
> it.  You may very well find that it was not really used for anything
> useful; there's a lot of dead code which I haven't been able to remove
> yet due to discrepancies.
> 
> Here are all the mentions I see to this API:
> 
> $ grep -rn malloc_usable_size
> src/test/nxt_malloc_test.c:54:        nxt_malloc_usable_size(p[i], s);
> src/nxt_malloc.h:37: * memory than is requested, so malloc_usable_size() allows to use all
> src/nxt_malloc.h:52: * with small cutback and then to adjust size with malloc_usable_size().
> src/nxt_malloc.h:53: * Glibc malloc_usable_size() is fast operation.
> src/nxt_malloc.h:56:#define nxt_malloc_usable_size(p, size)                                       \
> src/nxt_malloc.h:57:    size = malloc_usable_size(p)
> src/nxt_malloc.h:77: * FreeBSD 7.0 malloc_usable_size() is fast for allocations, which
> src/nxt_malloc.h:81:#define nxt_malloc_usable_size(p, size)                                       \
> src/nxt_malloc.h:82:    size = malloc_usable_size(p)
> src/nxt_malloc.h:101:#define nxt_malloc_usable_size(p, size)                                       \
> src/nxt_malloc.h:108:#define nxt_malloc_usable_size(p, size)
> src/nxt_unix.h:32:#include <malloc.h>                 /* malloc_usable_size(). */
> src/nxt_unix.h:49:#include <malloc_np.h>              /* malloc_usable_size(). */
> auto/malloc:53:# Linux malloc_usable_size().
> auto/malloc:55:nxt_feature="Linux malloc_usable_size()"
> auto/malloc:66:                      if (malloc_usable_size(p) < 4096)
> auto/malloc:75:    # FreeBSD malloc_usable_size().
> auto/malloc:77:    nxt_feature="FreeBSD malloc_usable_size()"
> auto/malloc:89:                          if (malloc_usable_size(p) < 4096)
> 
> The only ones that may be interesting to you are the single use (the
> commit that added the line says "Initial version.", so it won't help):
> 
> <https://github.com/nginx/unit/blob/c54331fa3d9597ba6bc85e7b7242981f00ed25c2/src/test/nxt_malloc_test.c#L54>
> 
> and the header where we define a wrapper macro, which contains several
> comments about assumptions made about different libc implementations:
> 
> <https://github.com/nginx/unit/blob/c54331fa3d9597ba6bc85e7b7242981f00ed25c2/src/nxt_malloc.h#L35>
> 
> I hope that tells you something.  It doesn't tell me anything, but I'm
> not used to fiddling with allocators.  :)

I'm sorry it doesn't, I can't tell from a quick peek what that test is 
trying to do :/

>> This amendment that DJ wrote is probably the most precise description of
>> the current malloc_usage_size situation:
>>
>>     The value returned by malloc_usable_size() may be greater than the
>>     requested size of the allocation because of various internal
>>     implementation details, none of which the programmer should rely on.
>>     This function is intended to only be used for diagnostics and
>>     statistics; writing to the excess memory without first calling
>>     realloc() to resize the allocation is not supported.  The returned
>>     value is only valid at the time of the call; any other call to a
>>     malloc family API may invalidate it.

I should probably add that I'm trying to come up with something that 
will provide the functionality most people depend on malloc_usable_size 
for but with more defined semantics that doesn't simply leak internals 
like malloc_usable_size does.  However it's too early for me to promise 
anything and whatever solution that comes up will likely be ABI 
incompatible anyway.  So the above is the most precise description of 
the situation and whatever happens, we'll only end up adding to it, not 
replacing it.

Thanks,
Sid