Subject: bug#76970: 31.0.50; master emacs crash with stack overflow

From: Aaron Zeng <[email-masked]>
Date: Tue, 24 Jun 2025 14:50:17 -0400

On Tue, Jun 24, 2025 at 1:46 AM Pip Cet <[email-masked]> wrote:
>
> The original fix was incomplete, please try applying this one, too:
>
> diff --git a/src/eval.c b/src/eval.c
> index 46705dc4543..20782639990 100644
> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -159,7 +159,11 @@ set_backtrace_debug_on_exit (union specbinding *pdl, bool doe)
>
>  bool
>  backtrace_p (union specbinding *pdl)
> -{ return specpdl ? pdl >= specpdl : false; }
> +{
> +  if (current_thread && specpdl && pdl)
> +    return pdl >= specpdl;
> +  return false;
> +}
>
>  static bool
>  backtrace_thread_p (struct thread_state *tstate, union specbinding *pdl)
>
> > This is from Emacs compiled at revision 991d3ad80a37a1cf8951d2607eb5f7544f968e93.
> >
> > It seems it is possible for current_thread to be set to NULL by the dying thread,
> > during backtrace_top.
>
> It is possible, in theory, for that to happen, and this additional race
> condition also should be fixed, but it's not what happened here.  What
> happened here was that current_thread became NULL, we checked it in
> backtrace_top, returned a NULL pointer for the pdl, passed that NULL
> pointer to backtrace_p, dereferenced current_thread again (by evaluating
> specpdl) and caused a segfault.

Doesn't this new code have the same issue, though?  Each time specpdl appears
in the code, it's dereferencing current_thread anew.

Maybe I am being overly paranoid, but it seems like the only correct
way to avoid
this race condition altogether is to read current_thread only once
during the course
of the signal handler, and to avoid using the macros which may read it a second
time, instead dereferencing a local copy of the pointer once it has
been verified
to be not-NULL.