[PATCH, libfortran] PR 88137 Initialize backtrace state once

[Thomas Schwinge]

Hi!

On Thu, 22 Nov 2018 11:17:24 +0200, Janne Blomqvist <blomqvist.janne@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?


Grüße
 Thomas