[07/12] Fix "attach" failure handling with GDBserver

diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.c b/gdb/testsuite/gdb.base/attach-fail-twice.c
new file mode 100644
index 00000000000..d045e35e4b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-fail-twice.c
@@ -0,0 +1,27 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+int
+main ()
+{
+  for (int i = 0; i < 60; i++)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/attach-fail-twice.exp b/gdb/testsuite/gdb.base/attach-fail-twice.exp
new file mode 100644
index 00000000000..59020463c67
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-fail-twice.exp
@@ -0,0 +1,94 @@ 
+# Copyright 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test doing a "attach" that fails, and then another "attach".
+
+require can_spawn_for_attach
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+set test_spawn_id [spawn_wait_for_attach $binfile]
+set testpid [spawn_id_get_pid $test_spawn_id]
+
+# Test an attach that fails.
+
+proc test_bad_attach {test} {
+    global testpid gdb_prompt
+
+    set boguspid 0
+    if { [istarget "*-*-*bsd*"] } {
+	# In FreeBSD 5.0, PID 0 is used for "swapper".  Use -1 instead
+	# (which should have the desired effect on any version of
+	# FreeBSD, and probably other *BSD's too).
+	set boguspid -1
+    }
+    gdb_test_multiple "attach $boguspid" $test {
+	-re "Attaching to.*, process $boguspid.*No such process.*$gdb_prompt $" {
+	    # Response expected on ptrace-based systems (i.e. GNU/Linux).
+	    pass "$test"
+	}
+	-re "Attaching to.*, process $boguspid.*denied.*$gdb_prompt $" {
+	    pass "$gdb_test_name"
+	}
+	-re "Attaching to.*, process $boguspid.*not permitted.*$gdb_prompt $" {
+	    pass "$gdb_test_name"
+	}
+	-re "Attaching to.*, process .*couldn't open /proc file.*$gdb_prompt $" {
+	    # Response expected from /proc-based systems.
+	    pass "$gdb_test_name"
+	}
+	-re "Can't attach to process..*$gdb_prompt $" {
+	    # Response expected on Windows.
+	    pass "$gdb_test_name"
+	}
+	-re "Attaching to.*, process $boguspid.*failed.*$gdb_prompt $" {
+	    # Response expected on the extended-remote target.
+	    pass "$gdb_test_name"
+	}
+    }
+}
+
+# Test an attach that succeeds.
+
+proc test_good_attach {test} {
+    gdb_test "attach $::testpid" \
+	"Attaching to program.*, process $::testpid.*" \
+	"$test"
+
+    set thread_count [get_valueof "" "\$_inferior_thread_count" -1]
+    gdb_assert {$thread_count > 0} \
+	"attached"
+}
+
+proc_with_prefix test {} {
+    clean_restart $::binfile
+
+    # GDB used to have a bug on Windows where failing to attach once
+    # made a subsequent "attach" or "run" hang.  So it's important for
+    # this regression test that we try to attach more than once.
+
+    test_bad_attach "bad attach 1"
+    test_bad_attach "bad attach 2"
+
+    # For good measure, test that we can attach to something after
+    # failing to attach previously.
+    test_good_attach "good attach"
+}
+
+test
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 7907441054b..84b2d27f3eb 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -173,10 +173,6 @@  proc_with_prefix do_attach_failure_tests {} {
     gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
     gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
 
-    # Probe this before the failing attach: the failed attach against GDBserver
-    # currently leaves the extended-remote target in a bad state.
-    set do_kfail [target_is_gdbserver]
-
     set test "fail to attach again"
     gdb_test_multiple "attach $testpid" "$test" {
 	-re "Attaching to process $testpid.*warning: process .* is already traced by process .*$gdb_prompt $" {
@@ -193,14 +189,12 @@  proc_with_prefix do_attach_failure_tests {} {
     gdb_test_no_output "set confirm off"
     gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1"
 
-    if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" }
     gdb_test "kill" "killed.*" "exit after attach failures"
 
     # This can probably be replaced with a call to runto or runto_main once
     # the kfail is removed.
     gdb_breakpoint "main"
     gdb_run_cmd
-    if { $do_kfail } { setup_kfail "gdb/19558" "*-*-*" }
     gdb_test_multiple "" "stop at main" {
 	-wrap -re "Breakpoint $::decimal, main .*" {
 	    pass $gdb_test_name
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index b170da44f6a..2633df08ddb 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -3294,29 +3294,36 @@  static void
 handle_v_attach (char *own_buf)
 {
   client_state &cs = get_client_state ();
-  int pid;
 
-  pid = strtol (own_buf + 8, NULL, 16);
-  if (pid != 0 && attach_inferior (pid) == 0)
-    {
-      /* Don't report shared library events after attaching, even if
-	 some libraries are preloaded.  GDB will always poll the
-	 library list.  Avoids the "stopped by shared library event"
-	 notice on the GDB side.  */
-      current_process ()->dlls_changed = false;
+  int pid = strtol (own_buf + 8, NULL, 16);
 
-      if (non_stop)
+  try
+    {
+      if (attach_inferior (pid) == 0)
 	{
-	  /* In non-stop, we don't send a resume reply.  Stop events
-	     will follow up using the normal notification
-	     mechanism.  */
-	  write_ok (own_buf);
+	  /* Don't report shared library events after attaching, even if
+	     some libraries are preloaded.  GDB will always poll the
+	     library list.  Avoids the "stopped by shared library event"
+	     notice on the GDB side.  */
+	  current_process ()->dlls_changed = false;
+
+	  if (non_stop)
+	    {
+	      /* In non-stop, we don't send a resume reply.  Stop events
+		 will follow up using the normal notification
+		 mechanism.  */
+	      write_ok (own_buf);
+	    }
+	  else
+	    prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status);
 	}
       else
-	prepare_resume_reply (own_buf, cs.last_ptid, cs.last_status);
+	write_enn (own_buf);
+    }
+  catch (const gdb_exception_error &exception)
+    {
+      sprintf (own_buf, "E.%s", exception.what ());
     }
-  else
-    write_enn (own_buf);
 }
 
 /* Decode an argument from the vRun packet buffer.  PTR points to the