public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap
@ 2024-01-15 16:35 fw at gcc dot gnu.org
2024-01-15 16:50 ` [Bug libgcc/113401] " iains at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-15 16:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
Bug ID: 113401
Summary: Memory (resource) leak in -ftrampoline-impl=heap
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: fw at gcc dot gnu.org
Target Milestone: ---
Target: x86_64-*, aarch64-*
Consider this test program:
#include <err.h>
#include <errno.h>
#include <pthread.h>
static void *volatile compiler_barrier;
void *
thread_routine (void *ignore)
{
int local_variable;
auto int *local_function (void)
{
return &local_variable;
}
compiler_barrier = local_function;
return NULL;
}
int
main (void)
{
for (int i = 0; i < 10000; ++i)
{
pthread_t thread;
int ret = pthread_create (&thread, NULL, thread_routine, NULL);
if (ret != 0)
{
errno = ret;
err (1, "pthread_create");
}
ret = pthread_join (thread, NULL);
if (ret != 0)
{
errno = ret;
err (1, "pthread_join");
}
}
}
With -ftrampoline-impl=stack, I get:
$ \time ./a.out
0.01user 0.16system 0:00.18elapsed 102%CPU (0avgtext+0avgdata 3008maxresident)k
0inputs+0outputs (0major+68minor)pagefaults 0swaps
With -ftrampoline-impl=heap:
$ \time ./a.out
0.03user 0.17system 0:00.20elapsed 101%CPU (0avgtext+0avgdata
41796maxresident)k
0inputs+0outputs (0major+10148minor)pagefaults 0swaps
The reason for the maxresident change is that __builtin_nested_func_ptr_created
and __builtin_nested_func_ptr_deleted cache the last trampoline page even if it
is completely unused. This is required for performance reasons. The fix is to
register a TLS destructor to deallocate that page, too. On glibc, that also
fixes another memory leak for -fno-exceptions compilations (the default for C)
if pthread_exit is called with an active trampoline.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
@ 2024-01-15 16:50 ` iains at gcc dot gnu.org
2024-01-15 16:54 ` fw at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: iains at gcc dot gnu.org @ 2024-01-15 16:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
Iain Sandoe <iains at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |iains at gcc dot gnu.org
--- Comment #1 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #0)
> The fix is to register a TLS destructor to
> deallocate that page, too. On glibc, that also fixes another memory leak for
> -fno-exceptions compilations (the default for C) if pthread_exit is called
> with an active trampoline.
Does this mean you have a proposed patch already? (before I start
investigation)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
2024-01-15 16:50 ` [Bug libgcc/113401] " iains at gcc dot gnu.org
@ 2024-01-15 16:54 ` fw at gcc dot gnu.org
2024-01-21 23:42 ` iains at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-15 16:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #1)
> (In reply to Florian Weimer from comment #0)
>
> > The fix is to register a TLS destructor to
> > deallocate that page, too. On glibc, that also fixes another memory leak for
> > -fno-exceptions compilations (the default for C) if pthread_exit is called
> > with an active trampoline.
>
> Does this mean you have a proposed patch already? (before I start
> investigation)
No, this was a reference to __cxa_thread_atexit, which is unfortunately in
libstdc++ (but is a thin shim around __cxa_thread_atexit_impl for current
glibc). The pthread_key_create fallback probably needs to be duplicated into
libgcc_s.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
2024-01-15 16:50 ` [Bug libgcc/113401] " iains at gcc dot gnu.org
2024-01-15 16:54 ` fw at gcc dot gnu.org
@ 2024-01-21 23:42 ` iains at gcc dot gnu.org
2024-01-22 9:57 ` fw at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: iains at gcc dot gnu.org @ 2024-01-21 23:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #3 from Iain Sandoe <iains at gcc dot gnu.org> ---
for platforms using pthreads as the underlying resource, then perhaps we can do
this without thread_atexit (which I do not see in many places) by using
pthread_cleanup_push ()
not sure if gthr.h abstracts that in any way (I do not have a patch at the
moment).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (2 preceding siblings ...)
2024-01-21 23:42 ` iains at gcc dot gnu.org
@ 2024-01-22 9:57 ` fw at gcc dot gnu.org
2024-01-22 10:11 ` iains at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-22 9:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #4 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #3)
> for platforms using pthreads as the underlying resource, then perhaps we can
> do this without thread_atexit (which I do not see in many places) by using
> pthread_cleanup_push ()
The current implementation already uses the same underlying mechanism as
pthread_cleanup_push if building with -fexceptions. It does not solve the leak
because the outermost handler deliberately does not perform a full deallocation
of the thread state.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (3 preceding siblings ...)
2024-01-22 9:57 ` fw at gcc dot gnu.org
@ 2024-01-22 10:11 ` iains at gcc dot gnu.org
2024-01-22 10:21 ` fw at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: iains at gcc dot gnu.org @ 2024-01-22 10:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #4)
> (In reply to Iain Sandoe from comment #3)
> > for platforms using pthreads as the underlying resource, then perhaps we can
> > do this without thread_atexit (which I do not see in many places) by using
> > pthread_cleanup_push ()
>
> The current implementation already uses the same underlying mechanism as
> pthread_cleanup_push if building with -fexceptions. It does not solve the
> leak because the outermost handler deliberately does not perform a full
> deallocation of the thread state.
hmm.. I'm slightly confused here.
We certainly make the __gcc_nested_func_ptr_deleted () call a cleanup attached
to scope exits and certainly the last page of trampolines is not deallocated
(as you note for the sake of avoiding churn in m-mapping).
However, in the current code the only pthread-specific stuff I see (in, say
config/i386/heap-trampoline.c) is specific to changing protections on the
mapped pages.
What I was thinking of is attaching a thread exit cleanup using
pthread_cleanup_push() for platforms with pthreads but without Libc support for
_thread_atexit - I guess I'm missing something :)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (4 preceding siblings ...)
2024-01-22 10:11 ` iains at gcc dot gnu.org
@ 2024-01-22 10:21 ` fw at gcc dot gnu.org
2024-01-22 10:27 ` iains at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-22 10:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #6 from Florian Weimer <fw at gcc dot gnu.org> ---
Sorry, pthread_cleanup_push is purely scope-based, like the existing handler.
It cannot be used to push a handler to some unscoped cleanup function list that
persists even after the current function returns. It's also implemented as a
macro, so it's not possible to emit it from builtin expansion.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (5 preceding siblings ...)
2024-01-22 10:21 ` fw at gcc dot gnu.org
@ 2024-01-22 10:27 ` iains at gcc dot gnu.org
2024-01-22 10:31 ` fw at gcc dot gnu.org
2024-01-22 10:46 ` iains at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: iains at gcc dot gnu.org @ 2024-01-22 10:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #6)
> Sorry, pthread_cleanup_push is purely scope-based, like the existing
> handler. It cannot be used to push a handler to some unscoped cleanup
> function list that persists even after the current function returns. It's
> also implemented as a macro, so it's not possible to emit it from builtin
> expansion.
Ah, then we have a documentation issue, because man pthread_cleanup_push(3)
describes running the registered functions on thread cancellation or on
thread_exit() [but not, unfortunately if the thread exits by returning - so
still not ideal].
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (6 preceding siblings ...)
2024-01-22 10:27 ` iains at gcc dot gnu.org
@ 2024-01-22 10:31 ` fw at gcc dot gnu.org
2024-01-22 10:46 ` iains at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-22 10:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #8 from Florian Weimer <fw at gcc dot gnu.org> ---
Which version of the manual page are you looking at?
https://man7.org/linux/man-pages/man3/pthread_cleanup_push.3.html seems pretty
clear about the scope-based nature (search for discussion of
break/return/goto).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug libgcc/113401] Memory (resource) leak in -ftrampoline-impl=heap
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
` (7 preceding siblings ...)
2024-01-22 10:31 ` fw at gcc dot gnu.org
@ 2024-01-22 10:46 ` iains at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: iains at gcc dot gnu.org @ 2024-01-22 10:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113401
--- Comment #9 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #8)
> Which version of the manual page are you looking at?
>
> https://man7.org/linux/man-pages/man3/pthread_cleanup_push.3.html seems
> pretty clear about the scope-based nature (search for discussion of
> break/return/goto).
yeah, got it; one needs to read the union of the sections (the page I was
reading was slightly different but the same basic info).
I suppose if we were able to create a wrapper around the thread routine and the
cleanup was NOP for cases without nested fns.
Otherwise, it looks a bit tricky for platforms without thread_atexit support.
Have to think some more.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-01-22 10:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-15 16:35 [Bug target/113401] New: Memory (resource) leak in -ftrampoline-impl=heap fw at gcc dot gnu.org
2024-01-15 16:50 ` [Bug libgcc/113401] " iains at gcc dot gnu.org
2024-01-15 16:54 ` fw at gcc dot gnu.org
2024-01-21 23:42 ` iains at gcc dot gnu.org
2024-01-22 9:57 ` fw at gcc dot gnu.org
2024-01-22 10:11 ` iains at gcc dot gnu.org
2024-01-22 10:21 ` fw at gcc dot gnu.org
2024-01-22 10:27 ` iains at gcc dot gnu.org
2024-01-22 10:31 ` fw at gcc dot gnu.org
2024-01-22 10:46 ` iains at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).