Bug 259294 - [libc] add a freeres function
Summary: [libc] add a freeres function
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-20 06:10 UTC by Paul Floyd
Modified: 2022-07-01 17:55 UTC (History)
6 users (show)

See Also:


Attachments
__libc_freeres (877 bytes, patch)
2021-12-05 13:29 UTC, Paul Floyd
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2021-10-20 06:10:37 UTC
Summary
~~~~~~~

This is a request to add a freeres function to libc.


Motivation
~~~~~~~~~~

This function would be used by Valgrind (and conceivably other similar tools). The purpose is to free any "still reachable" memory on a clean exit. For instance, this would include buffers allocated for I/O.

The freeres function doesn't get called by libc and in fact never gets called for applications running in a regular environment. However, Valgrind will look to see if it is present and call it (unless a command line flag has been specified not to do so).

Current Status
~~~~~~~~~~~~~~

On Linux, GNU libc has such a function (as  does libstdc++,__gnu_cxx::__freeres ).

On FreeBSD, libc++ does not allocate any buffers whose lifetime is not managed by the runtime. The libstdc++ function __gnu_cxx::__freeres is used in the same way as it is on Linux.

For FreeBSD libc, the default suppression mechanism is used. Whilst this is a reasonable solution, some users seem to find it disturbing and want to see strictly zero leaks/memory use in the final summary.


This means that a 'printf("Hell, World!\n");' C application will have the following summary from Valgring memcheck of FreeBSD

==76307== HEAP SUMMARY:
==76307==     in use at exit: 4,096 bytes in 1 blocks
==76307==   total heap usage: 1 allocs, 0 frees, 4,096 bytes allocated
==76307== 
==76307== LEAK SUMMARY:
==76307==    definitely lost: 0 bytes in 0 blocks
==76307==    indirectly lost: 0 bytes in 0 blocks
==76307==      possibly lost: 0 bytes in 0 blocks
==76307==    still reachable: 0 bytes in 0 blocks
==76307==         suppressed: 4,096 bytes in 1 blocks

On Linux, the "in use at exit" is 0 and there is no "suppressed" line at the end.
Comment 1 Paul Floyd 2021-10-22 06:07:23 UTC
I'll see if I can work out a patch.

In the meantime, here is the canonical C example

#include <stdio.h>

int main(void)
{
   printf("Hellw, World!\n");
}

If I run

 valgrind --default-suppressions=no --leak-check=full --show-reachable=yes ./hello_world

then I get


==1497== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 1
==1497==    at 0x484C8A4: malloc (in /usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so)
==1497==    by 0x4974AA3: ??? (in /lib/libc.so.7)
==1497==    by 0x4987278: ??? (in /lib/libc.so.7)
==1497==    by 0x497B012: ??? (in /lib/libc.so.7)
==1497==    by 0x497AD89: vfprintf_l (in /lib/libc.so.7)
==1497==    by 0x4975AF3: printf (in /lib/libc.so.7)
==1497==    by 0x2018A8: main (hello_world.c:5)

And for C++ the canonical hello world

#include <iostream>

int main()
{
   std::cout << "Hello, World!\n";
}


valgrind --default-suppressions=no --leak-check=full --show-reachable=yes ./hello_world2

==1507== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 1
==1507==    at 0x484C8A4: malloc (in /usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so)
==1507==    by 0x4AB5AA3: ??? (in /lib/libc.so.7)
==1507==    by 0x4AC8278: ??? (in /lib/libc.so.7)
==1507==    by 0x4AB47D2: ??? (in /lib/libc.so.7)
==1507==    by 0x4AB50A3: fwrite (in /lib/libc.so.7)
==1507==    by 0x202C08: std::__1::basic_streambuf<char, std::__1::char_traits<char> >::sputn(char const*, long) (include/c++/v1/streambuf:229)
==1507==    by 0x202A1F: std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> > std::__1::__pad_and_output<char, std::__1::char_traits<char> >(std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> >, char const*, char const*, char const*, std::__1::ios_base&, char) (include/c++/v1/locale:1411)
==1507==    by 0x202725: std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::__put_character_sequence<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*, unsigned long) (include/c++/v1/ostream:730)
==1507==    by 0x20260B: std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::operator<< <std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*) (include/c++/v1/ostream:869)


While looking at this, it strikes me that I'm not reading the libc debuginfo. I need to investigate that.
Comment 2 Paul Floyd 2021-12-05 13:25:18 UTC
With a debug build of libc, if I run a canonical C 'printf hello world' as follows



LD_PRELOAD=/home/paulf/build/src/obj/usr/home/paulf/build/src/amd64.amd64/lib/libc/libc.so.7.full /home/paulf/scratch/valgrind/vg-in-place --default-suppressions=no --leak-check=yes --show-reachable=yes ./hello_world

then it tells me that the still reachable memory is

==89763== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 1
==89763==    at 0x484C894: malloc (vg_replace_malloc.c:385)
==89763==    by 0x49716D2: __smakebuf (lib/libc/stdio/makebuf.c:73)
==89763==    by 0x4984164: __swsetup (lib/libc/stdio/wsetup.c:82)
==89763==    by 0x4977D82: __vfprintf (lib/libc/stdio/vfprintf.c:462)
==89763==    by 0x4977AF9: vfprintf_l (lib/libc/stdio/vfprintf.c:285)
==89763==    by 0x4972703: printf (lib/libc/stdio/printf.c:57)
==89763==    by 0x2018A8: main (hello_world.c:5)

Looking at the source, calls to printf pass through

#define	prepwrite(fp) \
 	((((fp)->_flags & __SWR) == 0 || \
 	    ((fp)->_bf._base == NULL && ((fp)->_flags & __SSTR) == 0)) && \
	 __swsetup(fp)


This checks that the buffer associated with stdout (also applies to stderr) is allocated and if not allocates it.

I'll add a patch in a moment that seems to work, at least for stdout (should work for stderr, not tested)

paulf> LD_PRELOAD=/home/paulf/build/src/obj/usr/home/paulf/build/src/amd64.amd64/lib/libc/libc.so.7.full /home/paulf/scratch/valgrind/vg-in-place --default-suppressions=no --leak-check=yes --show-reachable=yes ./hello_world
==69502== Memcheck, a memory error detector
==69502== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==69502== Using Valgrind-3.19.0.GIT and LibVEX; rerun with -h for copyright info
==69502== Command: ./hello_world
==69502== 
Hellw, World!
==69502== 
==69502== HEAP SUMMARY:
==69502==     in use at exit: 0 bytes in 0 blocks
==69502==   total heap usage: 1 allocs, 1 frees, 4,096 bytes allocated
==69502== 
==69502== All heap blocks were freed -- no leaks are possible
==69502== 
==69502== For lists of detected and suppressed errors, rerun with: -s
==69502== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Comment 3 Paul Floyd 2021-12-05 13:29:50 UTC
Created attachment 229927 [details]
__libc_freeres

Initial prototype. makebuf is where the memory for stdout gets allocated, so I put freeres there. If there are other buffers that I don't know of then maybe a different/new file would be better.

This probably needs protection against being called twice.

No changes needed on the Valgrind side for this!
Comment 4 Mark Johnston freebsd_committer 2022-04-13 15:38:16 UTC
Presumably there are many other places in libc that need to be hooked.  For instance, any use of getgrent(3) will trigger allocation of a global buffer, no?
Comment 5 Paul Floyd 2022-04-13 16:04:59 UTC
(In reply to Mark Johnston from comment #4)

I guess there are probably several other things that would need freeing.

GNU libc looks like it calls about 5 functions from __libc_freeres: IO, dl, thread, and a few other more anonymous frees.

https://code.woboq.org/userspace/glibc/malloc/set-freeres.c.html

In Valgrind I just added suppressions whenever I saw anything that was a one-off allocation in libc. I haven't looked thoroughly at the libc code or written exhaustive Valgrind libc leak tests.
Comment 6 Paul Floyd 2022-07-01 16:23:40 UTC
Another function to add to the list (not tested, just from looking at the code):
setproctitle_internal has 2 static pointers that get malloc'd and never freed.
Comment 7 Brooks Davis freebsd_committer 2022-07-01 16:48:41 UTC
I think this should start with a linker set of pointers to pointers to be freed. That keeps knowledge of things to be freed where they are declared. That's what the last bit of the glibc code does, but spelled somewhat differently than we'd do it.
Comment 8 Paul Floyd 2022-07-01 17:55:41 UTC
(In reply to Paul Floyd from comment #6)

Quick confirmation:

=5018== 2,048 bytes in 1 blocks are still reachable in loss record 1 of 2
==5018==    at 0x484CBC4: malloc (vg_replace_malloc.c:397)
==5018==    by 0x490B45B: ??? (in /lib/libc.so.7)
==5018==    by 0x490B778: setproctitle (in /lib/libc.so.7)
==5018==    by 0x2018C3: main (setproctitle.c:6)
==5018== 
==5018== 2,048 bytes in 1 blocks are still reachable in loss record 2 of 2
==5018==    at 0x484CBC4: malloc (vg_replace_malloc.c:397)
==5018==    by 0x490B4A5: ??? (in /lib/libc.so.7)
==5018==    by 0x490B778: setproctitle (in /lib/libc.so.7)
==5018==    by 0x2018C3: main (setproctitle.c: