On Fri, Nov 30, 2018 at 9:07 PM Thomas Schwinge <tho...@codesourcery.com>
wrote:
> Hi!
>
> On Thu, 22 Nov 2018 11:17:24 +0200, Janne Blomqvist <
> blomqvist.ja...@gmail.com> wrote:
> > From backtrace.h for backtrace_create_state:
> >
> > Calling this function allocates resources that can not be freed.
> > There is no backtrace_free_state function. The state is used to
> > cache information that is expensive to recompute. Programs are
> > expected to call this function at most once and to save the return
> > value for all later calls to backtrace functions.
> >
> > So instead of calling backtrace_create_state every time we wish to
> > show a backtrace, do it once and store the result in a static
> > variable. libbacktrace allows multiple threads to access the state,
> > so no need to use TLS.
>
> Hmm, OK, but...
>
> > --- a/libgfortran/runtime/backtrace.c
> > +++ b/libgfortran/runtime/backtrace.c
> > @@ -146,11 +146,15 @@ full_callback (void *data, uintptr_t pc, const
> char *filename,
> > void
> > show_backtrace (bool in_signal_handler)
> > {
> > - struct backtrace_state *lbstate;
> > + /* Note that libbacktrace allows the state to be accessed from
> > + multiple threads, so we don't need to use a TLS variable for the
> > + state here. */
> > + static struct backtrace_state *lbstate;
> > struct mystate state = { 0, false, in_signal_handler };
> > -
> > - lbstate = backtrace_create_state (NULL, __gthread_active_p (),
> > - error_callback, NULL);
> > +
> > + if (!lbstate)
> > + lbstate = backtrace_create_state (NULL, __gthread_active_p (),
> > + error_callback, NULL);
>
> ... don't you still have to make sure that only one thread ever executes
> "backtrace_create_state", and writes into "lbstate" (via locking, or
> atomics, or "pthread_once", or some such)? Or is that situation (more
> than one thread entering "show_backtrace" with uninitialized "lbstate")
> not possible to happen for other reasons?
>
I thought about that, but I think it probably(?) doesn't matter.
- Two threads could race and run backtrace_create_state() in parallel, and
probably we'd waste some memory then, but that was already possible before.
- As for locking, the function can be called from a signal handler, so
pthread_mutex is out. I guess I could implement a spinlock with atomics,
but, meh, seems overkill.
- And using atomics to access lbstate, it's an aligned pointer anyway, so
while it's a race to access it, it shouldn't be possible to get a situation
with a corrupted value for the pointer, right? I could use
__atomic_load/store to access it, but that doesn't buy much in the end,
does it, since the problem is parallel initialization, and not non-atomic
access to the lbstate pointer? Or does gcc support targets with non-atomic
access to aligned pointers?
Or is there something I'm missing?
--
Janne Blomqvist
