[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-cvs
Subject:    [binutils-gdb] Windows: Fix run/attach hang after bad run/attach
From:       Pedro Alves via Gdb-cvs <gdb-cvs () sourceware ! org>
Date:       2024-04-26 20:23:55
Message-ID: 20240426202355.223DE3858CD1 () sourceware ! org
[Download RAW message or body]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e0139e5b033d6ad5b2d880d6ee7a10bf6914886c


commit e0139e5b033d6ad5b2d880d6ee7a10bf6914886c
Author: Pedro Alves <pedro@palves.net>
Date:   Fri Jun 2 22:29:02 2023 +0100

    Windows: Fix run/attach hang after bad run/attach
    
    On Cygwin, gdb.base/attach.exp exposes that an "attach" after a
    previously failed "attach" hangs:
    
     (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: attach to \
digits-starting nonsense is prohibited  attach 0
     Can't attach to process 0 (error 2: The system cannot find the file specified.)
     (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: attach to nonexistent \
process is prohibited  attach 10644
     FAIL: gdb.base/attach.exp: do_attach_failure_tests: first attach (timeout)
    
    The problem is that windows_nat_target::attach always returns success
    even if the attach fails.  When we return success, the helper thread
    begins waiting for events (which will never come), and thus the next
    attach deadlocks on the do_synchronously call within
    windows_nat_target::attach.
    
    "run" has the same problem, which is exposed by the new
    gdb.base/run-fail-twice.exp testcase added in a following patch:
    
     (gdb) run
     Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
     Error creating process .../gdb.base/run-fail-twice/run-fail-twice.nox, (error 6: \
The handle is invalid.)  (gdb) PASS: gdb.base/run-fail-twice.exp: test: bad run 1
     run
     Starting program: .../gdb.base/run-fail-twice/run-fail-twice.nox
     FAIL: gdb.base/run-fail-twice.exp: test: bad run 2 (timeout)
    
    The problem here is the same, except that this time it is
    windows_nat_target::create_inferior that returns the incorrect result.
    
    This commit fixes both the "attach" and "run" paths, and the latter
    both the Cygwin and MinGW paths.  The tests mentioned above now pass
    on Cygwin.  Confirmed the fixes manually for MinGW GDB.
    
    Change-Id: I15ec9fa279aff269d4982b00f4ea7c25ae917239
    Approved-By: Tom Tromey <tom@tromey.com>

Diff:
---
 gdb/windows-nat.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 325c4d11ad8..3b3239ab938 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2059,7 +2059,7 @@ windows_nat_target::attach (const char *args, int from_tty)
       if (!ok)
 	err = (unsigned) GetLastError ();
 
-      return true;
+      return ok;
     });
 
   if (err.has_value ())
@@ -2784,12 +2784,15 @@ windows_nat_target::create_inferior (const char *exec_file,
   windows_init_thread_list ();
   do_synchronously ([&] ()
     {
-      if (!create_process (nullptr, args, flags, w32_env,
-			   inferior_cwd != nullptr ? infcwd : nullptr,
-			   disable_randomization,
-			   &si, &pi))
+      BOOL ok = create_process (nullptr, args, flags, w32_env,
+				inferior_cwd != nullptr ? infcwd : nullptr,
+				disable_randomization,
+				&si, &pi);
+
+      if (!ok)
 	ret = (unsigned) GetLastError ();
-      return true;
+
+      return ok;
     });
 
   if (w32_env)
@@ -2910,16 +2913,18 @@ windows_nat_target::create_inferior (const char *exec_file,
   windows_init_thread_list ();
   do_synchronously ([&] ()
     {
-      if (!create_process (nullptr, /* image */
-			   args,	/* command line */
-			   flags,	/* start flags */
-			   w32env,	/* environment */
-			   inferior_cwd, /* current directory */
-			   disable_randomization,
-			   &si,
-			   &pi))
+      BOOL ok = create_process (nullptr, /* image */
+				args,	/* command line */
+				flags,	/* start flags */
+				w32env,	/* environment */
+				inferior_cwd, /* current directory */
+				disable_randomization,
+				&si,
+				&pi);
+      if (!ok)
 	ret = (unsigned) GetLastError ();
-      return true;
+
+      return ok;
     });
   if (tty != INVALID_HANDLE_VALUE)
     CloseHandle (tty);


[prev in list] [next in list] [prev in thread] [next in thread]