Re: [PATCH] Return correct thread for cached event

[Hannes Domani <ssbssa@yahoo.de>]

 Am Mittwoch, 2. September 2026 um 20:10:06 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:

> Commit 2e1aacf15a9 ("Windows gdb+gdbserver: Make current_event
> per-thread state") introduced a regression.  This was detected by the
> AdaCore internal test suite in a somewhat unusual configuration: when
> using "attach" with a 32-bit Windows process, an extra stop would be
> generated, like:
> 
>     (gdb) break break_me
>     Breakpoint 1 at 0x1211a04: file pck.adb, line 18.
>     (gdb) continue
>     Continuing.
> 
>     Thread 4 received signal SIGINT, Interrupt.
>     [Switching to thread 4 (Thread 6652)]
>     0x77034210 in ntdll!RtlUserThreadStart () from C:/Windows/SysWOW64/ntdll.dll
> 
> Here, we expect to stop in break_me, but instead stop in some Windows
> DLL.
> 
> I tracked this down to this hunk in the aforementioned commit:
> 
> -      return debug_event_ptid (&windows_process.current_event);
> +      return ptid_t (windows_process.process_id,
> +            windows_process.main_thread_id, 0);
> 
> What happens here is that the "cached" stop ends up being reported in
> the main thread, rather than whatever thread actually caused this
> stop.
> 
> This patch fixes the problem by arranging to also cache the thread
> ptid.
> 
> I am not sure whether the call to switch_to_thread here is really
> needed; but since other returns seem to switch the thread, I thought
> this one ought to as well.
> 
> I tested this using the AdaCore internal test suite.
> 
> As this is a regression, when it lands I also plan to apply it to the
> gdb 18 branch.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34583
> ---
> gdbserver/win32-low.cc | 5 +++--
> gdbserver/win32-low.h  | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> index 13c14a7c69f..55e4b2e2fe3 100644
> --- a/gdbserver/win32-low.cc
> +++ b/gdbserver/win32-low.cc
> @@ -337,6 +337,7 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
>       || status.kind () == TARGET_WAITKIND_STOPPED)
>     {
>       windows_process.cached_status = status;
> +      windows_process.cached_ptid = current_thread->id;
>       break;
>     }
> 
> @@ -1145,8 +1146,8 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
>     fails).  Report it now.  */
>       *ourstatus = windows_process.cached_status;
>       windows_process.cached_status.set_ignore ();
> -      return ptid_t (windows_process.process_id,
> -            windows_process.main_thread_id, 0);
> +      switch_to_thread (find_thread_ptid (windows_process.cached_ptid));
> +      return windows_process.cached_ptid;
>     }
> 
>   while (1)
> diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
> index 439adb84bc2..f4f02851734 100644
> --- a/gdbserver/win32-low.h
> +++ b/gdbserver/win32-low.h
> @@ -193,6 +193,8 @@ struct gdbserver_windows_process : public windows_nat::windows_process_info
>       win32_wait should return it next, instead of fetching the next
>       debug event off the win32 API.  */
>   struct target_waitstatus cached_status;
> +  /* The ptid corresponding to the above status.  */
> +  ptid_t cached_ptid;
> 
>   /* True if current_process_handle needs to be closed.  */
>   bool open_process_used = false;
> 
> base-commit: 49c379f8c2f8ff888b621b4a070dc1976b942577
> -- 
> 2.55.0

This fix works for me, and the change itself LGTM as well.
With this windows_process_info::main_thread_id can be removed I think.

Tested-By: Hannes Domani <ssbssa@yahoo.de>


Hannes