[PATCH] Improve attach on Windows (was: Re: [PATCH v2 00/47] Windows non-stop mode)

[Pedro Alves <pedro@palves.net>]

Hi!

On 2025-06-05 18:57, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> I've pushed the series to the users/palves/windows-non-stop-v2 branch
> Pedro> on sourceware.org, for your convenience.
> 
> I merged this into the local tree and ran the internal AdaCore test
> suite on Windows 2016.

Thank you very much for doing this.  Much appreciated.

> 
> There were three "failures".
> 
> One of them is actually an improvement, where the test works around a
> current issue.
> 

One step forward, two steps back, I guess.

Tackling this incrementally.  I've looked at the first one:

> One of them seems to be a problem with the test.  This test "attach"es
> to a running process without a "file", and the test doesn't seem to
> match the new output:
> 
>     (gdb) attach 1228
>     Attaching to process 1228
>     [Switching to Thread 1228.0x580]
>     0x00007ffe3c756714 in ntdll!ZwDelayExecution ()
>        from C:\Windows\SYSTEM32\ntdll.dll
> 

I guess your test wasn't expecting that the current frame is printed?

See the patch below, on top of current master.  You are now seeing the current
frame being printed, unlike what I claim in the commit log of the patch below,
because with target-non-stop on, the Windows backend is a target_wait_no_wait==false
target, like Linux.

But even with the non-stop series, with "maint set target-non-stop off",
(or with Windows older than Windows 10,) then target_wait_no_wait==true, and
GDB doesn't print the frame.

IMO, this is just a preexisting bug that the non-stop series happens to hide.

Let me know what you think of the patch.

And yes, the fact that run control communicates with normal_stop via
globals is ugly and fragile to say the least.

From 0cb83571c91ba42b3de519b034509092a526dc65 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 11 Jun 2025 22:05:23 +0100
Subject: [PATCH] Improve attach on Windows

Unlike most targets, on Windows, when you attach, GDB doesn't print
the current stack frame.  Vis:

On GNU/Linux:

 attach 3340347
 Attaching to program: /home/pedro/gdb/build/gdb/testsuite/outputs/gdb.base/attach/attach, process 3340347
 Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...
 Reading symbols from /usr/lib/debug/.build-id/d5/197096f709801829b118af1b7cf6631efa2dcd.debug...
 Reading symbols from /lib64/ld-linux-x86-64.so.2...
 Reading symbols from /usr/lib/debug/.build-id/9c/b53985768bb99f138f48655f7b8bf7e420d13d.debug...
 [Thread debugging using libthread_db enabled]
 Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
 0x00005b3bf29be174 in main () at /home/pedro/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/attach.c:19
 19        while (! should_exit)
 (gdb) PASS: gdb.base/attach.exp: do_attach_tests: attach1, after setting file

On Cygwin:

 (gdb) attach 6692
 Attaching to program: /home/alves/gdb/build-cygwin-testsuite/outputs/gdb.base/attach/attach, process 6692
 [New Thread 6692.0x2e60]
 [New Thread 6692.0x2e9c]
 [New Thread 6692.0xd6c]
 [New Thread 6692.0x137c]
 [New Thread 6692.0x1270]
 (gdb) FAIL: gdb.base/attach.exp: do_attach_tests: attach1, after setting file

On Linux, GDB prints the frame because after the target_attach, GDB
goes back to the event loop, to wait for an initial stop event.  The
stop event arrives, and we process it, which sets the stop_print_frame
global, and then we get to normal_stop, which prints the frame iff
stop_print_frame is set, which it is.

Windows OTOH, is a target_attach_no_wait target, so after
target_attach, there is no going back to event loop.  In
infcmd.c:attach_command, we go straight to attach_post_wait which
takes us to normal_stop.  But this time, nothing set
stop_print_frame to true, so no frame is printed.  Actually, if the
global happened to be true due to an earlier event from debugging a
previous inferior, then we will print the frame.

This patch makes GDB's behavior consistent, by making sure the globals
normal_stop looks at are in a good state in the target_attach_no_wait
path.

With that alone, GDB now prints the frame:

 (gdb) attach 2915
 Attaching to program: /usr/bin/sleep.exe, process 2832
 [New Thread 2832.0x2a68]
 [New Thread 2832.0xb1c]
 [New Thread 2832.0x8ac]
 [Switching to Thread 2832.0x8ac]
 0x00007ffec51d4a71 in ntdll!DbgBreakPoint () from C:/Windows/SYSTEM32/ntdll.dll

This is still not ideal, IMHO, as the current thread is the thread
that Windows injects to attach:

 (gdb) info threads
   Id   Target Id                  Frame
   1    Thread 2832.0x2100 "sleep" 0x00007ffec51d18d7 in ntdll!ZwWaitForMultipleObjects () from C:/Windows/SYSTEM32/ntdll.dll
   2    Thread 2832.0x2a68 "sig"   0x00007ffec51d0e47 in ntdll!ZwReadFile () from C:/Windows/SYSTEM32/ntdll.dll
   3    Thread 2832.0xb1c          0x00007ffec51d49d7 in ntdll!ZwWaitForWorkViaWorkerFactory () from C:/Windows/SYSTEM32/ntdll.dll
 * 4    Thread 2832.0x8ac          0x00007ffec51d4a71 in ntdll!DbgBreakPoint () from C:/Windows/SYSTEM32/ntdll.dll

Automatically switching to main thread is IMHO more useful.  That
results in very similar output than what we see on Linux:

 attach 5164
 Attaching to program: /home/alves/gdb/build-cygwin-testsuite/outputs/gdb.base/attach/attach, process 5164
 [New Thread 5164.0x87c]
 [New Thread 5164.0x28f0]
 [New Thread 5164.0x376c]
 [New Thread 5164.0x2db4]
 [New Thread 5164.0xce4]
 main () at /home/alves/gdb/src/gdb/testsuite/gdb.base/attach.c:19
 19        while (! should_exit)
 (gdb)

If we do this, then we can simplify gdb.base/attach.exp a bit by
removing a couple Cygwin special cases.

The patch does all that, which results in the following
gdb.base/attach.exp progressions:

 -FAIL: gdb.base/attach.exp: do_attach_tests: attach1, after setting file
 -FAIL: gdb.base/attach.exp: do_attach_tests: attach2, with no file
 -FAIL: gdb.base/attach.exp: do_attach_tests: load file manually, after attach2 (re-read) (got interactive prompt)
 -FAIL: gdb.base/attach.exp: do_attach_tests: attach when process' a.out not in cwd
 -FAIL: gdb.base/attach.exp: do_attach_failure_tests: first attach
 +PASS: gdb.base/attach.exp: do_attach_tests: attach1, after setting file
 +PASS: gdb.base/attach.exp: do_attach_tests: attach2, with no file
 +PASS: gdb.base/attach.exp: do_attach_tests: attach when process' a.out not in cwd
 +PASS: gdb.base/attach.exp: do_attach_failure_tests: first attach

Change-Id: I359bdb25660c9a4d5d873e8771cfd1cd2a54c97b
---
 gdb/infcmd.c                      |  5 ++++-
 gdb/infrun.c                      | 13 +++++++++++++
 gdb/infrun.h                      |  3 +++
 gdb/testsuite/gdb.base/attach.exp | 26 ++++++--------------------
 gdb/windows-nat.c                 |  7 +++++++
 5 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index e9b58ce5521..cbf2373193c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2722,7 +2722,10 @@ attach_command (const char *args, int from_tty)
       return;
     }
   else
-    attach_post_wait (from_tty, mode);
+    {
+      set_normal_stop_state_just_attached ();
+      attach_post_wait (from_tty, mode);
+    }
 
   disable_commit_resumed.reset_and_commit ();
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2e02642c52a..41ab3342ee3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -407,6 +407,19 @@ static process_stratum_target *target_last_proc_target;
 static ptid_t target_last_wait_ptid;
 static struct target_waitstatus target_last_waitstatus;
 
+/* See infrun.h.  */
+
+void
+set_normal_stop_state_just_attached ()
+{
+  stop_print_frame = true;
+  stopped_by_random_signal = 0;
+
+  target_waitstatus status;
+  status.set_ignore ();
+  set_last_target_status (nullptr, minus_one_ptid, status);
+}
+
 void init_thread_stepping_state (struct thread_info *tss);
 
 static const char follow_fork_mode_child[] = "child";
diff --git a/gdb/infrun.h b/gdb/infrun.h
index b9b64aca45a..b707314fba6 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -418,5 +418,8 @@ struct scoped_enable_commit_resumed
   bool m_prev_enable_commit_resumed;
 };
 
+/* Set up state for normal_stop after we just attached, on
+   target_attach_no_wait targets.  */
+extern void set_normal_stop_state_just_attached ();
 
 #endif /* GDB_INFRUN_H */
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 0d1550a0541..484fa776ae4 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -159,16 +159,9 @@ proc_with_prefix do_attach_failure_tests {} {
     
     # Verify that we can't double attach to the process.
 
-    set test "first attach"
-    gdb_test_multiple "attach $testpid" "$test" {
-	-re "Attaching to program.*`?$escapedbinfile'?, process $testpid.*main.*at .*$srcfile:.*$gdb_prompt $" {
-	    pass "$test"
-	}
-	-re "Attaching to program.*`?$escapedbinfile\.exe'?, process $testpid.*\[Switching to thread $testpid\..*\].*$gdb_prompt $" {
-	    # Response expected on Cygwin.
-	    pass "$test"
-	}
-    }
+    gdb_test "attach $testpid" \
+	"Attaching to program.*`?${escapedbinfile}(\.exe)?'?, process $testpid.*main.*at .*$srcfile:.*" \
+	"first attach"
 
     gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
     gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
@@ -252,16 +245,9 @@ proc_with_prefix do_attach_tests {} {
 	}
     }
 
-    set test "attach1, after setting file"
-    gdb_test_multiple "attach $testpid" "$test" {
-	-re "Attaching to program.*`?$escapedbinfile'?, process $testpid.*main.*at .*$srcfile:.*$gdb_prompt $" {
-	    pass "$test"
-	}
-	-re "Attaching to program.*`?$escapedbinfile\.exe'?, process $testpid.*\[Switching to thread $testpid\..*\].*$gdb_prompt $" {
-	    # Response expected on Cygwin
-	    pass "$test"
-	}
-    }
+    gdb_test "attach $testpid" \
+	"Attaching to program.*`?${escapedbinfile}(\.exe)?'?, process $testpid.*main.*at .*$srcfile:.*" \
+	"attach1, after setting file"
 
     # Verify that we can "see" the variable "should_exit" in the
     # program, and that it is zero.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 939c5813aa7..0477731d40c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1980,6 +1980,13 @@ windows_nat_target::attach (const char *args, int from_tty)
 #endif
 
   do_initial_windows_stuff (pid, 1);
+
+  /* The thread that reports the initial breakpoint, and thus ends up
+     as the selected thread when we get here, was injected into the
+     inferior by DebugActiveProcess.  Switch to the main thread, which
+     is normally more useful to the user than the injected thread.  */
+  switch_to_thread (first_thread_of_inferior (current_inferior ()));
+
   target_terminal::ours ();
 }
 

base-commit: eb6c9310ee4d6cbde509d251fafb54ae45f5a5bf
-- 
2.49.0