[PATCH 3/3] gdbserver/linux-low: replace direct assignment to current_thread

[Aktemur, Tankut Baris]

On Friday, December 3, 2021 6:02 PM, Pedro Alves wrote:
> On 2021-12-01 17:49, Tankut Baris Aktemur via Gdb-patches wrote:
> > Use scoped_restore_current_thread and switch_to_thread in
> > linux_process_target::wait_for_sigstop.
> > ---
> >  gdbserver/linux-low.cc | 16 ++++++----------
> >  1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> > index e79ccb5ab0e..9f74de43344 100644
> > --- a/gdbserver/linux-low.cc
> > +++ b/gdbserver/linux-low.cc
> > @@ -3814,16 +3814,11 @@ lwp_is_marked_dead (struct lwp_info *lwp)
> >  void
> >  linux_process_target::wait_for_sigstop ()
> >  {
> > -  struct thread_info *saved_thread;
> > -  ptid_t saved_tid;
> > +  thread_info *saved_thread = current_thread;
> >    int wstat;
> >    int ret;
> >
> > -  saved_thread = current_thread;
> > -  if (saved_thread != NULL)
> > -    saved_tid = saved_thread->id;
> > -  else
> > -    saved_tid = null_ptid; /* avoid bogus unused warning */
> > +  scoped_restore_current_thread restore_thread;
> >
> >    if (debug_threads)
> >      debug_printf ("wait_for_sigstop: pulling events\n");
> > @@ -3834,8 +3829,8 @@ linux_process_target::wait_for_sigstop ()
> >    ret = wait_for_event_filtered (minus_one_ptid, null_ptid, &wstat, __WALL);
> >    gdb_assert (ret == -1);
> >
> > -  if (saved_thread == NULL || mythread_alive (saved_tid))
> > -    current_thread = saved_thread;
> > +  if (saved_thread == nullptr || mythread_alive (saved_thread->id))
> 
> Hmm, what if wait_for_event_filtered runs into the saved_thread exiting?
> Can't that happen?  I suspect it might.  In that case, the saved_thread
> pointer will be stale, I think.  I.e., if we knew the thread pointer was
> going to be valid, then what's the point of calling mythread_alive in the
> first place?

I see... Below is the updated patch.

Thanks
-Baris

diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index f4b537780ca..23520c382d3 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -3826,6 +3826,8 @@ linux_process_target::wait_for_sigstop ()
   else
     saved_tid = null_ptid; /* avoid bogus unused warning */

+  scoped_restore_current_thread restore_thread;
+
   if (debug_threads)
     debug_printf ("wait_for_sigstop: pulling events\n");

@@ -3836,7 +3838,7 @@ linux_process_target::wait_for_sigstop ()
   gdb_assert (ret == -1);

   if (saved_thread == NULL || mythread_alive (saved_tid))
-    current_thread = saved_thread;
+    return;
   else
     {
       if (debug_threads)
@@ -3845,7 +3847,8 @@ linux_process_target::wait_for_sigstop ()
       /* We can't change the current inferior behind GDB's back,
         otherwise, a subsequent command may apply to the wrong
         process.  */
-      current_thread = NULL;
+      restore_thread.dont_restore ();
+      switch_to_thread (nullptr);
     }
 }

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928