* [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