[PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
we run into attach failures.

Fix this by recognizing "ptrace: Operation not permitted" in
can_spawn_for_attach.

Tested on aarch64-linux.
---
 gdb/testsuite/gdb.base/break-interp.exp       |  4 +
 gdb/testsuite/gdb.base/dprintf-detach.exp     |  2 +-
 .../run-control-while-bg-execution.exp        | 10 ++-
 .../gdb.multi/attach-while-running.exp        |  2 +-
 .../gdb.threads/attach-into-signal.exp        |  3 +-
 .../gdb.threads/attach-slow-waitpid.exp       |  3 +-
 gdb/testsuite/gdb.threads/attach-stopped.exp  |  3 +-
 .../gdb.threads/check-libthread-db.exp        | 42 ++++-----
 gdb/testsuite/lib/gdb.exp                     | 87 ++++++++++++++++---
 9 files changed, 116 insertions(+), 40 deletions(-)

diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
index addacde552d..d7f84db4770 100644
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -318,6 +318,10 @@ proc test_attach_gdb {file pid displacement prefix} {
 }
 
 proc test_attach {file displacement {relink_args ""}} {
+    if { ![can_spawn_for_attach] } {
+	return
+    }
+
     global board_info
     global exec
 
diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
index 550d319a895..b4184d698df 100644
--- a/gdb/testsuite/gdb.base/dprintf-detach.exp
+++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
@@ -21,7 +21,7 @@
 load_lib gdbserver-support.exp
 
 # The test relies on "detach/attach".
-require !use_gdb_stub
+require can_spawn_for_attach
 
 standard_testfile
 set escapedbinfile [string_to_regexp ${binfile}]
diff --git a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
index f1cbd9351d3..a36c4ee3614 100644
--- a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
+++ b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
@@ -108,8 +108,14 @@ proc do_test { action1 action2 } {
     }
 }
 
-foreach_with_prefix action1 { kill detach add none } {
-    foreach_with_prefix action2 { start run attach } {
+set actions1 { kill detach add none }
+set actions2 { start run }
+if { [can_spawn_for_attach] } {
+    lappend actions2 attach
+}
+
+foreach_with_prefix action1 $actions1 {
+    foreach_with_prefix action2 $actions2  {
 	do_test $action1 $action2
     }
 }
diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp
index eade8b42a18..ca4fa635467 100644
--- a/gdb/testsuite/gdb.multi/attach-while-running.exp
+++ b/gdb/testsuite/gdb.multi/attach-while-running.exp
@@ -36,7 +36,7 @@
 
 standard_testfile
 
-require !use_gdb_stub
+require can_spawn_for_attach
 
 if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } {
     return
diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index 87e34070548..91da960e09a 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -17,7 +17,8 @@
 # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
index dc3e62a7b7e..28d70daad8c 100644
--- a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
+++ b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
@@ -37,7 +37,8 @@
 # during the attach phase.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/attach-stopped.exp b/gdb/testsuite/gdb.threads/attach-stopped.exp
index 78e194c992f..0421ffc3794 100644
--- a/gdb/testsuite/gdb.threads/attach-stopped.exp
+++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
@@ -18,7 +18,8 @@
 # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/check-libthread-db.exp b/gdb/testsuite/gdb.threads/check-libthread-db.exp
index 5662eeda077..6976fe6f83b 100644
--- a/gdb/testsuite/gdb.threads/check-libthread-db.exp
+++ b/gdb/testsuite/gdb.threads/check-libthread-db.exp
@@ -102,25 +102,27 @@ with_test_prefix "automated load-time check" {
     }
 
     # Automated load-time check with NPTL fully operational.
-    with_test_prefix "libpthread.so fully initialized" {
-	clean_restart ${binfile}
-
-	gdb_test_no_output "maint set check-libthread-db 1"
-	gdb_test_no_output "set debug libthread-db 1"
-
-	set test_spawn_id [spawn_wait_for_attach $binfile]
-	set testpid [spawn_id_get_pid $test_spawn_id]
-
-	gdb_test_sequence "attach $testpid" \
-	    "check debug libthread-db output" {
-		"\[\r\n\]+Running libthread_db integrity checks:"
-		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
-		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
-		"\[\r\n\]+libthread_db integrity checks passed."
-		"\[\r\n\]+[Thread debugging using libthread_db enabled]"
-	    }
-
-	gdb_exit
-	kill_wait_spawned_process $test_spawn_id
+    if { [can_spawn_for_attach] } {
+	with_test_prefix "libpthread.so fully initialized" {
+	    clean_restart ${binfile}
+
+	    gdb_test_no_output "maint set check-libthread-db 1"
+	    gdb_test_no_output "set debug libthread-db 1"
+
+	    set test_spawn_id [spawn_wait_for_attach $binfile]
+	    set testpid [spawn_id_get_pid $test_spawn_id]
+
+	    gdb_test_sequence "attach $testpid" \
+		"check debug libthread-db output" {
+		    "\[\r\n\]+Running libthread_db integrity checks:"
+		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
+		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
+		    "\[\r\n\]+libthread_db integrity checks passed."
+		    "\[\r\n\]+[Thread debugging using libthread_db enabled]"
+		}
+
+	    gdb_exit
+	    kill_wait_spawned_process $test_spawn_id
+	}
     }
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d48ea37c0cc..937add7a272 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6131,6 +6131,43 @@ proc gdb_exit { } {
     catch default_gdb_exit
 }
 
+# Helper function for can_spawn_for_attach.  Try to spawn and attach, and
+# return 0 only if we cannot attach because it's unsupported.
+
+gdb_caching_proc can_spawn_for_attach_1 {} {
+    # Assume yes.
+    set res 1
+
+    set me "can_spawn_for_attach"
+    set src { int main (void) { sleep (600); return 0; } }
+    if {![gdb_simple_compile $me $src executable]} {
+	return $res
+    }
+
+    set test_spawn_id [spawn_wait_for_attach_1 $obj]
+    file delete $obj
+
+    gdb_start
+
+    set test_pid [spawn_id_get_pid $test_spawn_id]
+    set attaching_re "Attaching to process $test_pid"
+    gdb_test_multiple "attach $test_pid" "can spawn for attach" {
+	-re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
+	    # Not permitted.
+	    set res 0
+	}
+	-re -wrap "" {
+	    # Don't know.
+	}
+    }
+
+    gdb_exit
+
+    kill_wait_spawned_process $test_spawn_id
+
+    return $res
+}
+
 # Return true if we can spawn a program on the target and attach to
 # it.
 
@@ -6151,8 +6188,25 @@ proc can_spawn_for_attach { } {
 	return 0
     }
 
-    # Assume yes.
-    return 1
+    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
+    # unpredictable which test-case will call it first, and consequently a
+    # test-case may pass in say a full test run, but fail when run
+    # individually, due to a can_spawn_for_attach call in location where a
+    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.
+    # To avoid this, we move the initial gdb_exit out of
+    # can_spawn_for_attach_1, guaranteeing that we end up in the same state
+    # regardless of whether can_spawn_for_attach_1 is called.  However, that
+    # is only necessary for the first call in a test-case, so cache the result
+    # in a global (which should be reset after each test-case) to keep track
+    # of that.
+    global cache_can_spawn_for_attach_1
+    if { [info exists cache_can_spawn_for_attach_1] } {
+	return $cache_can_spawn_for_attach_1
+    }
+    gdb_exit
+
+    set cache_can_spawn_for_attach_1 [can_spawn_for_attach_1]
+    return $cache_can_spawn_for_attach_1
 }
 
 # Centralize the failure checking of "attach" command.
@@ -6265,20 +6319,12 @@ proc spawn_id_get_pid { spawn_id } {
     return $testpid
 }
 
-# Start a set of programs running and then wait for a bit, to be sure
-# that they can be attached to.  Return a list of processes spawn IDs,
-# one element for each process spawned.  It's a test error to call
-# this when [can_spawn_for_attach] is false.
+# Helper function for spawn_wait_for_attach.  As spawn_wait_for_attach, but
+# doesn't check for can_spawn_for_attach.
 
-proc spawn_wait_for_attach { executable_list } {
+proc spawn_wait_for_attach_1 { executable_list } {
     set spawn_id_list {}
 
-    if ![can_spawn_for_attach] {
-	# The caller should have checked can_spawn_for_attach itself
-	# before getting here.
-	error "can't spawn for attach with this target/board"
-    }
-
     foreach {executable} $executable_list {
 	# Note we use Expect's spawn, not Tcl's exec, because with
 	# spawn we control when to wait for/reap the process.  That
@@ -6292,6 +6338,21 @@ proc spawn_wait_for_attach { executable_list } {
     return $spawn_id_list
 }
 
+# Start a set of programs running and then wait for a bit, to be sure
+# that they can be attached to.  Return a list of processes spawn IDs,
+# one element for each process spawned.  It's a test error to call
+# this when [can_spawn_for_attach] is false.
+
+proc spawn_wait_for_attach { executable_list } {
+    if ![can_spawn_for_attach] {
+	# The caller should have checked can_spawn_for_attach itself
+	# before getting here.
+	error "can't spawn for attach with this target/board"
+    }
+
+    return [spawn_wait_for_attach_1 $executable_list]
+}
+
 #
 # gdb_load_cmd -- load a file into the debugger.
 #		  ARGS - additional args to load command.

base-commit: ebf18671351d94185823d364b75369abc1baba31
-- 
2.35.3

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Pedro Alves]

On 2024-04-18 07:32, Tom de Vries wrote:
> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
> we run into attach failures.
> 
> Fix this by recognizing "ptrace: Operation not permitted" in
> can_spawn_for_attach.
> 
> Tested on aarch64-linux.

I really don't like relying on "attach" 's behavior to decide whether to test attach.
It's an inversion of responsibilities.

can_spawn_for_attach means "can I spawn a process, such that I will attach to it later", a
simple atomic thing.

Also, with this change, it means, "can I spawn a process and does attaching to it work?"
So it ends up misnamed.

Someone tried to add something very much like this a while ago, and I objected then:

  https://inbox.sourceware.org/gdb-patches/e5f08136-fa4d-2f21-ff83-8adf4d3a158e@palves.net/

We ended up with gdb_attach, added in a7e6a19e87f3, which handles the "ptrace: Operation not permitted"
scenarios too.  Some testcases have meanwhile been converted to use gdb_attach, but there are more,
of course.  IMO we should continue that direction.  gdb_attach is not unlike "gdb_run_cmd" for example.

See also:

  https://inbox.sourceware.org/gdb-patches/3b845985-cbd4-4996-145e-14191338b095@polymtl.ca/

Pedro Alves

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

On 4/24/24 21:40, Pedro Alves wrote:
> On 2024-04-18 07:32, Tom de Vries wrote:
>> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
>> we run into attach failures.
>>
>> Fix this by recognizing "ptrace: Operation not permitted" in
>> can_spawn_for_attach.
>>
>> Tested on aarch64-linux.
> 
> I really don't like relying on "attach" 's behavior to decide whether to test attach.
> It's an inversion of responsibilities.
> 

Hi Pedro,

I don't see it that way, I think this approach accurately handles that 
if there's no permission to attach to a spawned process, you cannot 
spawn-for-attach.

> can_spawn_for_attach means "can I spawn a process, such that I will attach to it later", a
> simple atomic thing.
>  > Also, with this change, it means, "can I spawn a process and does 
attaching to it work?"
> So it ends up misnamed.
> 

I see what you mean, though I don't see it as misnamed, but if that 
bothers you I'm glad to rename it something else.  Can_spawn_and_attach? 
You have another suggestion?

> Someone tried to add something very much like this a while ago, and I objected then:
> 
>    https://inbox.sourceware.org/gdb-patches/e5f08136-fa4d-2f21-ff83-8adf4d3a158e@palves.net/
> > We ended up with gdb_attach, added in a7e6a19e87f3, which handles the 
"ptrace: Operation not permitted"
> scenarios too.  Some testcases have meanwhile been converted to use gdb_attach, but there are more,
> of course.  IMO we should continue that direction.  gdb_attach is not unlike "gdb_run_cmd" for example.
> 
> See also:
> 
>    https://inbox.sourceware.org/gdb-patches/3b845985-cbd4-4996-145e-14191338b095@polymtl.ca/
> 

I think the gdb_attach approach is problematic for the reasons that:
- it's not versatile enough to handle all the cases where it's needed,
   and
- it's required to be used in all the attach sites.

On the contrary, the proposed patch very simply handles all the cases, 
and in only one proc which is already used in a trivial way in most 
test-cases that need it.  So it seems obvious to me that the proposed 
solution is preferable.

Thanks,
- Tom

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Pedro Alves]

On 2024-04-24 22:35, Tom de Vries wrote:
> On 4/24/24 21:40, Pedro Alves wrote:
>> On 2024-04-18 07:32, Tom de Vries wrote:
>>> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
>>> we run into attach failures.
>>>
>>> Fix this by recognizing "ptrace: Operation not permitted" in
>>> can_spawn_for_attach.
>>>
>>> Tested on aarch64-linux.
>>
>> I really don't like relying on "attach" 's behavior to decide whether to test attach.
>> It's an inversion of responsibilities.
>>
> 
> Hi Pedro,
> 
> I don't see it that way, I think this approach accurately handles that if there's no permission to attach to a spawned process, you cannot spawn-for-attach.

My inversion remark was more about of the risk of attach breaking/regressing, and attach
testcases stop being exercised.  PASS -> UNSUPPORTED/UNTESTED regressions aren't that well
noticed.  But I suppose that if the procedure is written in a way that is very lax in its attach
check, only really looking for the ptrace output case, and defaulting to return that yes,
we can attach", that is mitigated.

> 
>> can_spawn_for_attach means "can I spawn a process, such that I will attach to it later", a
>> simple atomic thing.
>>  > Also, with this change, it means, "can I spawn a process and does 
> attaching to it work?"
>> So it ends up misnamed.
>>
> 
> I see what you mean, though I don't see it as misnamed, but if that bothers you I'm glad to rename it something else.  Can_spawn_and_attach? You have another suggestion?


can_spawn_for_attach's name was originally derived from spawn_wait_for_attach.
(git 60b3033e6e2936af6fcc37cf67cade99a89940ad).  It was more about 
"can I call spawn_wait_for_attach" than "can I attach".  But given the
check for use_gdb_stub, I guess you could say that your check doesn't really
change that aspect.  It's just my expectations that need to be adjusted.

Let's just leave it as you have it.

> 
> I think the gdb_attach approach is problematic for the reasons that:
> - it's not versatile enough to handle all the cases where it's needed,
>   and
> - it's required to be used in all the attach sites.
>
> On the contrary, the proposed patch very simply handles all the cases, and in only one proc which is already used in a trivial way in most test-cases that need it.  So it seems obvious to me that the proposed solution is preferable.

One issue I was concerned about is that for extended-remote testing, this change
assumes the native-extended-gdbserver board's behavior of gdb_start spawning gdbserver
and making gdb connect to it.  I don't know whether actually-remote extended-remote
boards actually do that.  But I suppose it's a reasonable thing for them to do.

OK, I'm convinced.

Comments on the patch itself below.

On 2024-04-18 07:32, Tom de Vries wrote:
> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
> we run into attach failures.
> 
> Fix this by recognizing "ptrace: Operation not permitted" in
> can_spawn_for_attach.
> 
> Tested on aarch64-linux.
> ---
>  gdb/testsuite/gdb.base/break-interp.exp       |  4 +
>  gdb/testsuite/gdb.base/dprintf-detach.exp     |  2 +-
>  .../run-control-while-bg-execution.exp        | 10 ++-
>  .../gdb.multi/attach-while-running.exp        |  2 +-
>  .../gdb.threads/attach-into-signal.exp        |  3 +-
>  .../gdb.threads/attach-slow-waitpid.exp       |  3 +-
>  gdb/testsuite/gdb.threads/attach-stopped.exp  |  3 +-
>  .../gdb.threads/check-libthread-db.exp        | 42 ++++-----
>  gdb/testsuite/lib/gdb.exp                     | 87 ++++++++++++++++---
>  9 files changed, 116 insertions(+), 40 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
> index addacde552d..d7f84db4770 100644
> --- a/gdb/testsuite/gdb.base/break-interp.exp
> +++ b/gdb/testsuite/gdb.base/break-interp.exp
> @@ -318,6 +318,10 @@ proc test_attach_gdb {file pid displacement prefix} {
>  }
>  
>  proc test_attach {file displacement {relink_args ""}} {
> +    if { ![can_spawn_for_attach] } {
> +	return
> +    }
> +
>      global board_info
>      global exec
>  
> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
> index 550d319a895..b4184d698df 100644
> --- a/gdb/testsuite/gdb.base/dprintf-detach.exp
> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
> @@ -21,7 +21,7 @@
>  load_lib gdbserver-support.exp
>  
>  # The test relies on "detach/attach".
> -require !use_gdb_stub
> +require can_spawn_for_attach
>  
>  standard_testfile
>  set escapedbinfile [string_to_regexp ${binfile}]
> diff --git a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> index f1cbd9351d3..a36c4ee3614 100644
> --- a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> +++ b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> @@ -108,8 +108,14 @@ proc do_test { action1 action2 } {
>      }
>  }
>  
> -foreach_with_prefix action1 { kill detach add none } {
> -    foreach_with_prefix action2 { start run attach } {
> +set actions1 { kill detach add none }
> +set actions2 { start run }
> +if { [can_spawn_for_attach] } {
> +    lappend actions2 attach
> +}
> +

I suppose this isn't using the more typical form of:

foreach_with_prefix action1 { kill detach add none } {
    foreach_with_prefix action2 { start run attach } {
        if {$action2 == "attach" && ![can_spawn_for_attach]} {
           continue
        }
  	do_test $action1 $action2
    }
}

... to make sure the can_spawn_for_attach call is done early?
Can you add a comment if this is important?  Otherwise, I think
the more typical form is a lot more readable.


> +foreach_with_prefix action1 $actions1 {
> +    foreach_with_prefix action2 $actions2  {
>  	do_test $action1 $action2
>      }
>  }
> diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp
> index eade8b42a18..ca4fa635467 100644
> --- a/gdb/testsuite/gdb.multi/attach-while-running.exp
> +++ b/gdb/testsuite/gdb.multi/attach-while-running.exp
> @@ -36,7 +36,7 @@
>  
>  standard_testfile
>  
> -require !use_gdb_stub
> +require can_spawn_for_attach
>  
>  if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } {
>      return
> diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
> index 87e34070548..91da960e09a 100644
> --- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
> +++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
> @@ -17,7 +17,8 @@
>  # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
>  
>  # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>  require {!is_remote host}
>  require {istarget *-linux*}
>  
> diff --git a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> index dc3e62a7b7e..28d70daad8c 100644
> --- a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> +++ b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> @@ -37,7 +37,8 @@
>  # during the attach phase.
>  
>  # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>  require {!is_remote host}
>  require {istarget *-linux*}
>  
> diff --git a/gdb/testsuite/gdb.threads/attach-stopped.exp b/gdb/testsuite/gdb.threads/attach-stopped.exp
> index 78e194c992f..0421ffc3794 100644
> --- a/gdb/testsuite/gdb.threads/attach-stopped.exp
> +++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
> @@ -18,7 +18,8 @@
>  # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
>  
>  # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>  require {!is_remote host}
>  require {istarget *-linux*}
>  


> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index d48ea37c0cc..937add7a272 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6131,6 +6131,43 @@ proc gdb_exit { } {
>      catch default_gdb_exit
>  }
>  
> +# Helper function for can_spawn_for_attach.  Try to spawn and attach, and
> +# return 0 only if we cannot attach because it's unsupported.
> +
> +gdb_caching_proc can_spawn_for_attach_1 {} {
> +    # Assume yes.
> +    set res 1
> +
> +    set me "can_spawn_for_attach"
> +    set src { int main (void) { sleep (600); return 0; } }

Don't we need a header include for sleep?

> +    if {![gdb_simple_compile $me $src executable]} {
> +	return $res
> +    }
> +
> +    set test_spawn_id [spawn_wait_for_attach_1 $obj]
> +    file delete $obj

I think this should be a remote_file call, not a build file delete.
can_spawn_wait_for_attach returns false if the target is remote,
not if the host is remote.

> +
> +    gdb_start
> +
> +    set test_pid [spawn_id_get_pid $test_spawn_id]
> +    set attaching_re "Attaching to process $test_pid"
> +    gdb_test_multiple "attach $test_pid" "can spawn for attach" {
> +	-re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
> +	    # Not permitted.
> +	    set res 0
> +	}
> +	-re -wrap "" {
> +	    # Don't know.
> +	}
> +    }
> +
> +    gdb_exit

The comment below suggests this gdb_exit shouldn't be here.

> +
> +    kill_wait_spawned_process $test_spawn_id
> +
> +    return $res
> +}
> +
>  # Return true if we can spawn a program on the target and attach to
>  # it.
>  

> @@ -6151,8 +6188,25 @@ proc can_spawn_for_attach { } {
>  	return 0
>      }
>  
> -    # Assume yes.
> -    return 1
> +    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
> +    # unpredictable which test-case will call it first, and consequently a
> +    # test-case may pass in say a full test run, but fail when run
> +    # individually, due to a can_spawn_for_attach call in location where a
> +    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.
> +    # To avoid this, we move the initial gdb_exit out of
> +    # can_spawn_for_attach_1, guaranteeing that we end up in the same state
> +    # regardless of whether can_spawn_for_attach_1 is called.  However, that
> +    # is only necessary for the first call in a test-case, so cache the result
> +    # in a global (which should be reset after each test-case) to keep track
> +    # of that.
> +    global cache_can_spawn_for_attach_1
> +    if { [info exists cache_can_spawn_for_attach_1] } {
> +	return $cache_can_spawn_for_attach_1
> +    }
> +    gdb_exit

There is still a gdb_exit call in can_spawn_for_attach_1.
I guess you meant to remove it?

I think it'll also be good to document that can_spawn_for_attach calls gdb_exit.

> +
> +    set cache_can_spawn_for_attach_1 [can_spawn_for_attach_1]
> +    return $cache_can_spawn_for_attach_1
>  }
>  
>  # Centralize the failure checking of "attach" command.
> @@ -6265,20 +6319,12 @@ proc spawn_id_get_pid { spawn_id } {
>      return $testpid
>  }
>  
> -# Start a set of programs running and then wait for a bit, to be sure
> -# that they can be attached to.  Return a list of processes spawn IDs,
> -# one element for each process spawned.  It's a test error to call
> -# this when [can_spawn_for_attach] is false.
> +# Helper function for spawn_wait_for_attach 

... and can_spawn_for_attach_1.  Just spawn_wait_for_attach by itself
wouldn't need the helper.

> As spawn_wait_for_attach, but
> +# doesn't check for can_spawn_for_attach.
>  

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

On 4/26/24 20:50, Pedro Alves wrote:
> On 2024-04-24 22:35, Tom de Vries wrote:
>> On 4/24/24 21:40, Pedro Alves wrote:
>>> On 2024-04-18 07:32, Tom de Vries wrote:
>>>> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
>>>> we run into attach failures.
>>>>
>>>> Fix this by recognizing "ptrace: Operation not permitted" in
>>>> can_spawn_for_attach.
>>>>
>>>> Tested on aarch64-linux.
>>>
>>> I really don't like relying on "attach" 's behavior to decide whether to test attach.
>>> It's an inversion of responsibilities.
>>>
>>
>> Hi Pedro,
>>
>> I don't see it that way, I think this approach accurately handles that if there's no permission to attach to a spawned process, you cannot spawn-for-attach.
> 
> My inversion remark was more about of the risk of attach breaking/regressing, and attach
> testcases stop being exercised. 

I see.

> PASS -> UNSUPPORTED/UNTESTED regressions aren't that well
> noticed.

Agreed.

> But I suppose that if the procedure is written in a way that is very lax in its attach
> check, only really looking for the ptrace output case, and defaulting to return that yes,
> we can attach", that is mitigated.
> 

Yes, that was my intention.

>>
>>> can_spawn_for_attach means "can I spawn a process, such that I will attach to it later", a
>>> simple atomic thing.
>>>   > Also, with this change, it means, "can I spawn a process and does
>> attaching to it work?"
>>> So it ends up misnamed.
>>>
>>
>> I see what you mean, though I don't see it as misnamed, but if that bothers you I'm glad to rename it something else.  Can_spawn_and_attach? You have another suggestion?
> 
> 
> can_spawn_for_attach's name was originally derived from spawn_wait_for_attach.
> (git 60b3033e6e2936af6fcc37cf67cade99a89940ad).  It was more about
> "can I call spawn_wait_for_attach" than "can I attach".  But given the
> check for use_gdb_stub, I guess you could say that your check doesn't really
> change that aspect.  It's just my expectations that need to be adjusted.
> 
> Let's just leave it as you have it.
> 

Ack.

>>
>> I think the gdb_attach approach is problematic for the reasons that:
>> - it's not versatile enough to handle all the cases where it's needed,
>>    and
>> - it's required to be used in all the attach sites.
>>
>> On the contrary, the proposed patch very simply handles all the cases, and in only one proc which is already used in a trivial way in most test-cases that need it.  So it seems obvious to me that the proposed solution is preferable.
> 
> One issue I was concerned about is that for extended-remote testing, this change
> assumes the native-extended-gdbserver board's behavior of gdb_start spawning gdbserver
> and making gdb connect to it.  I don't know whether actually-remote extended-remote
> boards actually do that.  But I suppose it's a reasonable thing for them to do.
> 
> OK, I'm convinced.
> 

Great.

> Comments on the patch itself below.
> 
> On 2024-04-18 07:32, Tom de Vries wrote:
>> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
>> we run into attach failures.
>>
>> Fix this by recognizing "ptrace: Operation not permitted" in
>> can_spawn_for_attach.
>>
>> Tested on aarch64-linux.
>> ---
>>   gdb/testsuite/gdb.base/break-interp.exp       |  4 +
>>   gdb/testsuite/gdb.base/dprintf-detach.exp     |  2 +-
>>   .../run-control-while-bg-execution.exp        | 10 ++-
>>   .../gdb.multi/attach-while-running.exp        |  2 +-
>>   .../gdb.threads/attach-into-signal.exp        |  3 +-
>>   .../gdb.threads/attach-slow-waitpid.exp       |  3 +-
>>   gdb/testsuite/gdb.threads/attach-stopped.exp  |  3 +-
>>   .../gdb.threads/check-libthread-db.exp        | 42 ++++-----
>>   gdb/testsuite/lib/gdb.exp                     | 87 ++++++++++++++++---
>>   9 files changed, 116 insertions(+), 40 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
>> index addacde552d..d7f84db4770 100644
>> --- a/gdb/testsuite/gdb.base/break-interp.exp
>> +++ b/gdb/testsuite/gdb.base/break-interp.exp
>> @@ -318,6 +318,10 @@ proc test_attach_gdb {file pid displacement prefix} {
>>   }
>>   
>>   proc test_attach {file displacement {relink_args ""}} {
>> +    if { ![can_spawn_for_attach] } {
>> +	return
>> +    }
>> +
>>       global board_info
>>       global exec
>>   
>> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
>> index 550d319a895..b4184d698df 100644
>> --- a/gdb/testsuite/gdb.base/dprintf-detach.exp
>> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
>> @@ -21,7 +21,7 @@
>>   load_lib gdbserver-support.exp
>>   
>>   # The test relies on "detach/attach".
>> -require !use_gdb_stub
>> +require can_spawn_for_attach
>>   
>>   standard_testfile
>>   set escapedbinfile [string_to_regexp ${binfile}]
>> diff --git a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
>> index f1cbd9351d3..a36c4ee3614 100644
>> --- a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
>> +++ b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
>> @@ -108,8 +108,14 @@ proc do_test { action1 action2 } {
>>       }
>>   }
>>   
>> -foreach_with_prefix action1 { kill detach add none } {
>> -    foreach_with_prefix action2 { start run attach } {
>> +set actions1 { kill detach add none }
>> +set actions2 { start run }
>> +if { [can_spawn_for_attach] } {
>> +    lappend actions2 attach
>> +}
>> +
> 
> I suppose this isn't using the more typical form of:
> 
> foreach_with_prefix action1 { kill detach add none } {
>      foreach_with_prefix action2 { start run attach } {
>          if {$action2 == "attach" && ![can_spawn_for_attach]} {
>             continue
>          }
>    	do_test $action1 $action2
>      }
> }
> 
> ... to make sure the can_spawn_for_attach call is done early?
> Can you add a comment if this is important?

It's not important, since do_test starts with a clean_restart.

> Otherwise, I think
> the more typical form is a lot more readable.

I prefer it as I wrote it (and find that more readable), but ok, I've 
reworked it.

>> +foreach_with_prefix action1 $actions1 {
>> +    foreach_with_prefix action2 $actions2  {
>>   	do_test $action1 $action2
>>       }
>>   }
>> diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp
>> index eade8b42a18..ca4fa635467 100644
>> --- a/gdb/testsuite/gdb.multi/attach-while-running.exp
>> +++ b/gdb/testsuite/gdb.multi/attach-while-running.exp
>> @@ -36,7 +36,7 @@
>>   
>>   standard_testfile
>>   
>> -require !use_gdb_stub
>> +require can_spawn_for_attach
>>   
>>   if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } {
>>       return
>> diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
>> index 87e34070548..91da960e09a 100644
>> --- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
>> +++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
>> @@ -17,7 +17,8 @@
>>   # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
>>   
>>   # This test only works on Linux
>> -require !use_gdb_stub isnative
>> +require can_spawn_for_attach
>> +require isnative
>>   require {!is_remote host}
>>   require {istarget *-linux*}
>>   
>> diff --git a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
>> index dc3e62a7b7e..28d70daad8c 100644
>> --- a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
>> +++ b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
>> @@ -37,7 +37,8 @@
>>   # during the attach phase.
>>   
>>   # This test only works on Linux
>> -require !use_gdb_stub isnative
>> +require can_spawn_for_attach
>> +require isnative
>>   require {!is_remote host}
>>   require {istarget *-linux*}
>>   
>> diff --git a/gdb/testsuite/gdb.threads/attach-stopped.exp b/gdb/testsuite/gdb.threads/attach-stopped.exp
>> index 78e194c992f..0421ffc3794 100644
>> --- a/gdb/testsuite/gdb.threads/attach-stopped.exp
>> +++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
>> @@ -18,7 +18,8 @@
>>   # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
>>   
>>   # This test only works on Linux
>> -require !use_gdb_stub isnative
>> +require can_spawn_for_attach
>> +require isnative
>>   require {!is_remote host}
>>   require {istarget *-linux*}
>>   
> 
> 
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index d48ea37c0cc..937add7a272 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -6131,6 +6131,43 @@ proc gdb_exit { } {
>>       catch default_gdb_exit
>>   }
>>   
>> +# Helper function for can_spawn_for_attach.  Try to spawn and attach, and
>> +# return 0 only if we cannot attach because it's unsupported.
>> +
>> +gdb_caching_proc can_spawn_for_attach_1 {} {
>> +    # Assume yes.
>> +    set res 1
>> +
>> +    set me "can_spawn_for_attach"
>> +    set src { int main (void) { sleep (600); return 0; } }
> 
> Don't we need a header include for sleep?
> 

Indeed, added.

>> +    if {![gdb_simple_compile $me $src executable]} {
>> +	return $res
>> +    }
>> +
>> +    set test_spawn_id [spawn_wait_for_attach_1 $obj]
>> +    file delete $obj
> 
> I think this should be a remote_file call, not a build file delete.
> can_spawn_wait_for_attach returns false if the target is remote,
> not if the host is remote.
> 

AFAIU, with remote host, compiling mean copying sources to remote host, 
compiling on remote host and copying the artifact back to build.  This 
is all taken care of by dejagnu, and the file we're deleting is the file 
on build.

Anyway, I've changed this to use "remote_file build" to make this more 
clear.

>> +
>> +    gdb_start
>> +
>> +    set test_pid [spawn_id_get_pid $test_spawn_id]
>> +    set attaching_re "Attaching to process $test_pid"
>> +    gdb_test_multiple "attach $test_pid" "can spawn for attach" {
>> +	-re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
>> +	    # Not permitted.
>> +	    set res 0
>> +	}
>> +	-re -wrap "" {
>> +	    # Don't know.
>> +	}
>> +    }
>> +
>> +    gdb_exit
> 
> The comment below suggests this gdb_exit shouldn't be here.
> 

It does belong here.  Your comment means my comment is not good enough, 
I've updated it to make it more clear.

>> +
>> +    kill_wait_spawned_process $test_spawn_id
>> +
>> +    return $res
>> +}
>> +
>>   # Return true if we can spawn a program on the target and attach to
>>   # it.
>>   
> 
>> @@ -6151,8 +6188,25 @@ proc can_spawn_for_attach { } {
>>   	return 0
>>       }
>>   
>> -    # Assume yes.
>> -    return 1
>> +    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
>> +    # unpredictable which test-case will call it first, and consequently a
>> +    # test-case may pass in say a full test run, but fail when run
>> +    # individually, due to a can_spawn_for_attach call in location where a
>> +    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.
>> +    # To avoid this, we move the initial gdb_exit out of
>> +    # can_spawn_for_attach_1, guaranteeing that we end up in the same state
>> +    # regardless of whether can_spawn_for_attach_1 is called.  However, that
>> +    # is only necessary for the first call in a test-case, so cache the result
>> +    # in a global (which should be reset after each test-case) to keep track
>> +    # of that.
>> +    global cache_can_spawn_for_attach_1
>> +    if { [info exists cache_can_spawn_for_attach_1] } {
>> +	return $cache_can_spawn_for_attach_1
>> +    }
>> +    gdb_exit
> 
> There is still a gdb_exit call in can_spawn_for_attach_1.
> I guess you meant to remove it?
> 

There is, and it's intentional.  Again, I hope that the updated comment 
will clarify this.

> I think it'll also be good to document that can_spawn_for_attach calls gdb_exit.
> 

Done.

>> +
>> +    set cache_can_spawn_for_attach_1 [can_spawn_for_attach_1]
>> +    return $cache_can_spawn_for_attach_1
>>   }
>>   
>>   # Centralize the failure checking of "attach" command.
>> @@ -6265,20 +6319,12 @@ proc spawn_id_get_pid { spawn_id } {
>>       return $testpid
>>   }
>>   
>> -# Start a set of programs running and then wait for a bit, to be sure
>> -# that they can be attached to.  Return a list of processes spawn IDs,
>> -# one element for each process spawned.  It's a test error to call
>> -# this when [can_spawn_for_attach] is false.
>> +# Helper function for spawn_wait_for_attach
> 
> ... and can_spawn_for_attach_1.  Just spawn_wait_for_attach by itself
> wouldn't need the helper.
> 

Ack, updated.


I've submitted a v2 here ( 
https://sourceware.org/pipermail/gdb-patches/2024-May/208725.html ).

Thanks,
- Tom

>> As spawn_wait_for_attach, but
>> +# doesn't check for can_spawn_for_attach.
>>   
> 

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

On 5/17/24 05:42, Simon Marchi wrote:
> 
> 
> On 2024-05-01 04:34, Tom de Vries wrote:
>> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
>> we run into attach failures.
>>
>> Fix this by recognizing "ptrace: Operation not permitted" in
>> can_spawn_for_attach.
>>
>> Tested on aarch64-linux and x86_64-linux.
> 
> Hi Tom,
> 
> Since this patch, I see some problems in test
> gdb.testsuite/gdb-caching-proc-consistency.exp with the native-gdbserver
> board:
> 
>      ERROR: in testcase /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.testsuite/gdb-caching-proc-consistency.exp
>      ERROR:  gdbserver does not support attach 1186109 without extended-remote
>      ERROR:  tcl error code NONE
>      ERROR:  tcl error info:
>      gdbserver does not support attach 1186109 without extended-remote
>          while executing
>      "error "gdbserver does not support $command without extended-remote""
>          (procedure "gdb_test_multiple" line 51)
>          invoked from within
>      "gdb_test_multiple "attach $test_pid" "can spawn for attach" {
>              -re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
>                  # Not permitte..."
>          (procedure "gdb_real__can_spawn_for_attach_1" line 27)
>          invoked from within
>      "gdb_real__can_spawn_for_attach_1"
>          ("uplevel" body line 1)
>          invoked from within
>      "uplevel 2 [list $real_name {*}$args]"
>          invoked from within
>      "gdb_do_cache_wrap $real_name"
>          ("uplevel" body line 2)
>          invoked from within
>      "uplevel 1 $body"
>          invoked from within
>      ...

Hi Simon,

Thanks for reporting this, I've submitted a fix here ( 
https://sourceware.org/pipermail/gdb-patches/2024-May/209242.html ).

Thanks,
- Tom

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Simon Marchi]

On 2024-05-01 04:34, Tom de Vries wrote:
> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
> we run into attach failures.
> 
> Fix this by recognizing "ptrace: Operation not permitted" in
> can_spawn_for_attach.
> 
> Tested on aarch64-linux and x86_64-linux.

Hi Tom,

Since this patch, I see some problems in test
gdb.testsuite/gdb-caching-proc-consistency.exp with the native-gdbserver
board:

    ERROR: in testcase /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.testsuite/gdb-caching-proc-consistency.exp
    ERROR:  gdbserver does not support attach 1186109 without extended-remote
    ERROR:  tcl error code NONE
    ERROR:  tcl error info:
    gdbserver does not support attach 1186109 without extended-remote
        while executing                          
    "error "gdbserver does not support $command without extended-remote""
        (procedure "gdb_test_multiple" line 51)
        invoked from within
    "gdb_test_multiple "attach $test_pid" "can spawn for attach" {
            -re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
                # Not permitte..."
        (procedure "gdb_real__can_spawn_for_attach_1" line 27)
        invoked from within
    "gdb_real__can_spawn_for_attach_1"
        ("uplevel" body line 1)
        invoked from within
    "uplevel 2 [list $real_name {*}$args]"
        invoked from within
    "gdb_do_cache_wrap $real_name"
        ("uplevel" body line 2)
        invoked from within
    "uplevel 1 $body"                            
        invoked from within
    ...

Simon

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Pedro Alves]

On 2024-05-01 09:34, Tom de Vries wrote:

>  
> -    # Assume yes.
> -    return 1
> +    # The normal sequence to use for a runtime test like
> +    # can_spawn_for_attach_1 is:
> +    # - gdb_exit (don't use a running gdb, we don't know what state it is in),
> +    # - gdb_start (start a new gdb), and
> +    # - gdb_exit (cleanup).
> +    #
> +    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
> +    # unpredictable which test-case will call it first, and consequently a
> +    # test-case may pass in say a full test run, but fail when run
> +    # individually, due to a can_spawn_for_attach call in location where a

"in a location", I presume.

> +    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.

Otherwise,

 Approved-By: Pedro Alves <pedro@palves.net>

Thank you.

Re: [PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

On 5/1/24 10:34, Tom de Vries wrote:

Um, I forgot that I already send a v2, so this is v2-v2.

Thanks,
- Tom

> When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
> we run into attach failures.
> 
> Fix this by recognizing "ptrace: Operation not permitted" in
> can_spawn_for_attach.
> 
> Tested on aarch64-linux and x86_64-linux.
> ---
>   gdb/testsuite/gdb.base/break-interp.exp       |   4 +
>   gdb/testsuite/gdb.base/dprintf-detach.exp     |   2 +-
>   .../run-control-while-bg-execution.exp        |   3 +
>   .../gdb.multi/attach-while-running.exp        |   2 +-
>   .../gdb.threads/attach-into-signal.exp        |   3 +-
>   .../gdb.threads/attach-slow-waitpid.exp       |   3 +-
>   gdb/testsuite/gdb.threads/attach-stopped.exp  |   3 +-
>   .../gdb.threads/check-libthread-db.exp        |  42 +++----
>   gdb/testsuite/lib/gdb.exp                     | 112 +++++++++++++++---
>   9 files changed, 135 insertions(+), 39 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
> index addacde552d..d7f84db4770 100644
> --- a/gdb/testsuite/gdb.base/break-interp.exp
> +++ b/gdb/testsuite/gdb.base/break-interp.exp
> @@ -318,6 +318,10 @@ proc test_attach_gdb {file pid displacement prefix} {
>   }
>   
>   proc test_attach {file displacement {relink_args ""}} {
> +    if { ![can_spawn_for_attach] } {
> +	return
> +    }
> +
>       global board_info
>       global exec
>   
> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
> index 550d319a895..b4184d698df 100644
> --- a/gdb/testsuite/gdb.base/dprintf-detach.exp
> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
> @@ -21,7 +21,7 @@
>   load_lib gdbserver-support.exp
>   
>   # The test relies on "detach/attach".
> -require !use_gdb_stub
> +require can_spawn_for_attach
>   
>   standard_testfile
>   set escapedbinfile [string_to_regexp ${binfile}]
> diff --git a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> index f1cbd9351d3..380047ae854 100644
> --- a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> +++ b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
> @@ -110,6 +110,9 @@ proc do_test { action1 action2 } {
>   
>   foreach_with_prefix action1 { kill detach add none } {
>       foreach_with_prefix action2 { start run attach } {
> +	if { $action2 == "attach" && ![can_spawn_for_attach] } {
> +	   continue
> +	}
>   	do_test $action1 $action2
>       }
>   }
> diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp
> index eade8b42a18..ca4fa635467 100644
> --- a/gdb/testsuite/gdb.multi/attach-while-running.exp
> +++ b/gdb/testsuite/gdb.multi/attach-while-running.exp
> @@ -36,7 +36,7 @@
>   
>   standard_testfile
>   
> -require !use_gdb_stub
> +require can_spawn_for_attach
>   
>   if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } {
>       return
> diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
> index 87e34070548..91da960e09a 100644
> --- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
> +++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
> @@ -17,7 +17,8 @@
>   # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
>   
>   # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>   require {!is_remote host}
>   require {istarget *-linux*}
>   
> diff --git a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> index dc3e62a7b7e..28d70daad8c 100644
> --- a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> +++ b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
> @@ -37,7 +37,8 @@
>   # during the attach phase.
>   
>   # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>   require {!is_remote host}
>   require {istarget *-linux*}
>   
> diff --git a/gdb/testsuite/gdb.threads/attach-stopped.exp b/gdb/testsuite/gdb.threads/attach-stopped.exp
> index 78e194c992f..0421ffc3794 100644
> --- a/gdb/testsuite/gdb.threads/attach-stopped.exp
> +++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
> @@ -18,7 +18,8 @@
>   # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
>   
>   # This test only works on Linux
> -require !use_gdb_stub isnative
> +require can_spawn_for_attach
> +require isnative
>   require {!is_remote host}
>   require {istarget *-linux*}
>   
> diff --git a/gdb/testsuite/gdb.threads/check-libthread-db.exp b/gdb/testsuite/gdb.threads/check-libthread-db.exp
> index 5662eeda077..6976fe6f83b 100644
> --- a/gdb/testsuite/gdb.threads/check-libthread-db.exp
> +++ b/gdb/testsuite/gdb.threads/check-libthread-db.exp
> @@ -102,25 +102,27 @@ with_test_prefix "automated load-time check" {
>       }
>   
>       # Automated load-time check with NPTL fully operational.
> -    with_test_prefix "libpthread.so fully initialized" {
> -	clean_restart ${binfile}
> -
> -	gdb_test_no_output "maint set check-libthread-db 1"
> -	gdb_test_no_output "set debug libthread-db 1"
> -
> -	set test_spawn_id [spawn_wait_for_attach $binfile]
> -	set testpid [spawn_id_get_pid $test_spawn_id]
> -
> -	gdb_test_sequence "attach $testpid" \
> -	    "check debug libthread-db output" {
> -		"\[\r\n\]+Running libthread_db integrity checks:"
> -		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
> -		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
> -		"\[\r\n\]+libthread_db integrity checks passed."
> -		"\[\r\n\]+[Thread debugging using libthread_db enabled]"
> -	    }
> -
> -	gdb_exit
> -	kill_wait_spawned_process $test_spawn_id
> +    if { [can_spawn_for_attach] } {
> +	with_test_prefix "libpthread.so fully initialized" {
> +	    clean_restart ${binfile}
> +
> +	    gdb_test_no_output "maint set check-libthread-db 1"
> +	    gdb_test_no_output "set debug libthread-db 1"
> +
> +	    set test_spawn_id [spawn_wait_for_attach $binfile]
> +	    set testpid [spawn_id_get_pid $test_spawn_id]
> +
> +	    gdb_test_sequence "attach $testpid" \
> +		"check debug libthread-db output" {
> +		    "\[\r\n\]+Running libthread_db integrity checks:"
> +		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
> +		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
> +		    "\[\r\n\]+libthread_db integrity checks passed."
> +		    "\[\r\n\]+[Thread debugging using libthread_db enabled]"
> +		}
> +
> +	    gdb_exit
> +	    kill_wait_spawned_process $test_spawn_id
> +	}
>       }
>   }
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index d092258a9e5..7019f38eb15 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6155,8 +6155,54 @@ proc gdb_exit { } {
>       catch default_gdb_exit
>   }
>   
> +# Helper function for can_spawn_for_attach.  Try to spawn and attach, and
> +# return 0 only if we cannot attach because it's unsupported.
> +
> +gdb_caching_proc can_spawn_for_attach_1 {} {
> +    # Assume yes.
> +    set res 1
> +
> +    set me "can_spawn_for_attach"
> +    set src {
> +	#include <unistd.h>
> +
> +	int
> +	main (void)
> +	{
> +	    sleep (600);
> +	    return 0;
> +	}
> +    }
> +    if {![gdb_simple_compile $me $src executable]} {
> +	return $res
> +    }
> +
> +    set test_spawn_id [spawn_wait_for_attach_1 $obj]
> +    remote_file build delete $obj
> +
> +    gdb_start
> +
> +    set test_pid [spawn_id_get_pid $test_spawn_id]
> +    set attaching_re "Attaching to process $test_pid"
> +    gdb_test_multiple "attach $test_pid" "can spawn for attach" {
> +	-re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
> +	    # Not permitted.
> +	    set res 0
> +	}
> +	-re -wrap "" {
> +	    # Don't know, keep assuming yes.
> +	}
> +    }
> +
> +    gdb_exit
> +
> +    kill_wait_spawned_process $test_spawn_id
> +
> +    return $res
> +}
> +
>   # Return true if we can spawn a program on the target and attach to
> -# it.
> +# it.  Calls gdb_exit for the first call in a test-case.
>   
>   proc can_spawn_for_attach { } {
>       # We use exp_pid to get the inferior's pid, assuming that gives
> @@ -6175,8 +6221,39 @@ proc can_spawn_for_attach { } {
>   	return 0
>       }
>   
> -    # Assume yes.
> -    return 1
> +    # The normal sequence to use for a runtime test like
> +    # can_spawn_for_attach_1 is:
> +    # - gdb_exit (don't use a running gdb, we don't know what state it is in),
> +    # - gdb_start (start a new gdb), and
> +    # - gdb_exit (cleanup).
> +    #
> +    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
> +    # unpredictable which test-case will call it first, and consequently a
> +    # test-case may pass in say a full test run, but fail when run
> +    # individually, due to a can_spawn_for_attach call in location where a
> +    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.
> +    # To avoid this, we move the initial gdb_exit out of
> +    # can_spawn_for_attach_1, guaranteeing that we end up in the same state
> +    # regardless of whether can_spawn_for_attach_1 is called.  However, that
> +    # is only necessary for the first call in a test-case, so cache the result
> +    # in a global (which should be reset after each test-case) to keep track
> +    # of that.
> +    #
> +    # In summary, we distinguish between three cases:
> +    # - first call in first test-case.  Executes can_spawn_for_attach_1.
> +    #   Calls gdb_exit, gdb_start, gdb_exit.
> +    # - first call in following test-cases.  Uses cached result of
> +    #   can_spawn_for_attach_1.  Calls gdb_exit.
> +    # - rest.  Use cached result in cache_can_spawn_for_attach_1. Calls no
> +    #   gdb_start or gdb_exit.
> +    global cache_can_spawn_for_attach_1
> +    if { [info exists cache_can_spawn_for_attach_1] } {
> +	return $cache_can_spawn_for_attach_1
> +    }
> +    gdb_exit
> +
> +    set cache_can_spawn_for_attach_1 [can_spawn_for_attach_1]
> +    return $cache_can_spawn_for_attach_1
>   }
>   
>   # Centralize the failure checking of "attach" command.
> @@ -6289,20 +6366,12 @@ proc spawn_id_get_pid { spawn_id } {
>       return $testpid
>   }
>   
> -# Start a set of programs running and then wait for a bit, to be sure
> -# that they can be attached to.  Return a list of processes spawn IDs,
> -# one element for each process spawned.  It's a test error to call
> -# this when [can_spawn_for_attach] is false.
> +# Helper function for spawn_wait_for_attach and can_spawn_for_attach_1.  As
> +# spawn_wait_for_attach, but doesn't check for can_spawn_for_attach.
>   
> -proc spawn_wait_for_attach { executable_list } {
> +proc spawn_wait_for_attach_1 { executable_list } {
>       set spawn_id_list {}
>   
> -    if ![can_spawn_for_attach] {
> -	# The caller should have checked can_spawn_for_attach itself
> -	# before getting here.
> -	error "can't spawn for attach with this target/board"
> -    }
> -
>       foreach {executable} $executable_list {
>   	# Note we use Expect's spawn, not Tcl's exec, because with
>   	# spawn we control when to wait for/reap the process.  That
> @@ -6316,6 +6385,21 @@ proc spawn_wait_for_attach { executable_list } {
>       return $spawn_id_list
>   }
>   
> +# Start a set of programs running and then wait for a bit, to be sure
> +# that they can be attached to.  Return a list of processes spawn IDs,
> +# one element for each process spawned.  It's a test error to call
> +# this when [can_spawn_for_attach] is false.
> +
> +proc spawn_wait_for_attach { executable_list } {
> +    if ![can_spawn_for_attach] {
> +	# The caller should have checked can_spawn_for_attach itself
> +	# before getting here.
> +	error "can't spawn for attach with this target/board"
> +    }
> +
> +    return [spawn_wait_for_attach_1 $executable_list]
> +}
> +
>   #
>   # gdb_load_cmd -- load a file into the debugger.
>   #		  ARGS - additional args to load command.
> 
> base-commit: 7320840f1998547b4428c58d1b39ca41febad83a

[PATCH v2] [gdb/testsuite] Handle ptrace operation not permitted in can_spawn_for_attach

[Tom de Vries]

When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
we run into attach failures.

Fix this by recognizing "ptrace: Operation not permitted" in
can_spawn_for_attach.

Tested on aarch64-linux and x86_64-linux.
---
 gdb/testsuite/gdb.base/break-interp.exp       |   4 +
 gdb/testsuite/gdb.base/dprintf-detach.exp     |   2 +-
 .../run-control-while-bg-execution.exp        |   3 +
 .../gdb.multi/attach-while-running.exp        |   2 +-
 .../gdb.threads/attach-into-signal.exp        |   3 +-
 .../gdb.threads/attach-slow-waitpid.exp       |   3 +-
 gdb/testsuite/gdb.threads/attach-stopped.exp  |   3 +-
 .../gdb.threads/check-libthread-db.exp        |  42 +++----
 gdb/testsuite/lib/gdb.exp                     | 112 +++++++++++++++---
 9 files changed, 135 insertions(+), 39 deletions(-)

diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
index addacde552d..d7f84db4770 100644
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -318,6 +318,10 @@ proc test_attach_gdb {file pid displacement prefix} {
 }
 
 proc test_attach {file displacement {relink_args ""}} {
+    if { ![can_spawn_for_attach] } {
+	return
+    }
+
     global board_info
     global exec
 
diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
index 550d319a895..b4184d698df 100644
--- a/gdb/testsuite/gdb.base/dprintf-detach.exp
+++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
@@ -21,7 +21,7 @@
 load_lib gdbserver-support.exp
 
 # The test relies on "detach/attach".
-require !use_gdb_stub
+require can_spawn_for_attach
 
 standard_testfile
 set escapedbinfile [string_to_regexp ${binfile}]
diff --git a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
index f1cbd9351d3..380047ae854 100644
--- a/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
+++ b/gdb/testsuite/gdb.base/run-control-while-bg-execution.exp
@@ -110,6 +110,9 @@ proc do_test { action1 action2 } {
 
 foreach_with_prefix action1 { kill detach add none } {
     foreach_with_prefix action2 { start run attach } {
+	if { $action2 == "attach" && ![can_spawn_for_attach] } {
+	   continue
+	}
 	do_test $action1 $action2
     }
 }
diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp
index eade8b42a18..ca4fa635467 100644
--- a/gdb/testsuite/gdb.multi/attach-while-running.exp
+++ b/gdb/testsuite/gdb.multi/attach-while-running.exp
@@ -36,7 +36,7 @@
 
 standard_testfile
 
-require !use_gdb_stub
+require can_spawn_for_attach
 
 if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } {
     return
diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index 87e34070548..91da960e09a 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -17,7 +17,8 @@
 # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
index dc3e62a7b7e..28d70daad8c 100644
--- a/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
+++ b/gdb/testsuite/gdb.threads/attach-slow-waitpid.exp
@@ -37,7 +37,8 @@
 # during the attach phase.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/attach-stopped.exp b/gdb/testsuite/gdb.threads/attach-stopped.exp
index 78e194c992f..0421ffc3794 100644
--- a/gdb/testsuite/gdb.threads/attach-stopped.exp
+++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
@@ -18,7 +18,8 @@
 # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-require !use_gdb_stub isnative
+require can_spawn_for_attach
+require isnative
 require {!is_remote host}
 require {istarget *-linux*}
 
diff --git a/gdb/testsuite/gdb.threads/check-libthread-db.exp b/gdb/testsuite/gdb.threads/check-libthread-db.exp
index 5662eeda077..6976fe6f83b 100644
--- a/gdb/testsuite/gdb.threads/check-libthread-db.exp
+++ b/gdb/testsuite/gdb.threads/check-libthread-db.exp
@@ -102,25 +102,27 @@ with_test_prefix "automated load-time check" {
     }
 
     # Automated load-time check with NPTL fully operational.
-    with_test_prefix "libpthread.so fully initialized" {
-	clean_restart ${binfile}
-
-	gdb_test_no_output "maint set check-libthread-db 1"
-	gdb_test_no_output "set debug libthread-db 1"
-
-	set test_spawn_id [spawn_wait_for_attach $binfile]
-	set testpid [spawn_id_get_pid $test_spawn_id]
-
-	gdb_test_sequence "attach $testpid" \
-	    "check debug libthread-db output" {
-		"\[\r\n\]+Running libthread_db integrity checks:"
-		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
-		"\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
-		"\[\r\n\]+libthread_db integrity checks passed."
-		"\[\r\n\]+[Thread debugging using libthread_db enabled]"
-	    }
-
-	gdb_exit
-	kill_wait_spawned_process $test_spawn_id
+    if { [can_spawn_for_attach] } {
+	with_test_prefix "libpthread.so fully initialized" {
+	    clean_restart ${binfile}
+
+	    gdb_test_no_output "maint set check-libthread-db 1"
+	    gdb_test_no_output "set debug libthread-db 1"
+
+	    set test_spawn_id [spawn_wait_for_attach $binfile]
+	    set testpid [spawn_id_get_pid $test_spawn_id]
+
+	    gdb_test_sequence "attach $testpid" \
+		"check debug libthread-db output" {
+		    "\[\r\n\]+Running libthread_db integrity checks:"
+		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
+		    "\[\r\n\]+\[ \]+Got thread 0x\[1-9a-f\]\[0-9a-f\]+ => \[0-9\]+ => 0x\[1-9a-f\]\[0-9a-f\]+ ... OK"
+		    "\[\r\n\]+libthread_db integrity checks passed."
+		    "\[\r\n\]+[Thread debugging using libthread_db enabled]"
+		}
+
+	    gdb_exit
+	    kill_wait_spawned_process $test_spawn_id
+	}
     }
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d092258a9e5..7019f38eb15 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6155,8 +6155,54 @@ proc gdb_exit { } {
     catch default_gdb_exit
 }
 
+# Helper function for can_spawn_for_attach.  Try to spawn and attach, and
+# return 0 only if we cannot attach because it's unsupported.
+
+gdb_caching_proc can_spawn_for_attach_1 {} {
+    # Assume yes.
+    set res 1
+
+    set me "can_spawn_for_attach"
+    set src {
+	#include <unistd.h>
+
+	int
+	main (void)
+	{
+	    sleep (600);
+	    return 0;
+	}
+    }
+    if {![gdb_simple_compile $me $src executable]} {
+	return $res
+    }
+
+    set test_spawn_id [spawn_wait_for_attach_1 $obj]
+    remote_file build delete $obj
+
+    gdb_start
+
+    set test_pid [spawn_id_get_pid $test_spawn_id]
+    set attaching_re "Attaching to process $test_pid"
+    gdb_test_multiple "attach $test_pid" "can spawn for attach" {
+	-re -wrap "$attaching_re\r\n.*ptrace: Operation not permitted\\." {
+	    # Not permitted.
+	    set res 0
+	}
+	-re -wrap "" {
+	    # Don't know, keep assuming yes.
+	}
+    }
+
+    gdb_exit
+
+    kill_wait_spawned_process $test_spawn_id
+
+    return $res
+}
+
 # Return true if we can spawn a program on the target and attach to
-# it.
+# it.  Calls gdb_exit for the first call in a test-case.
 
 proc can_spawn_for_attach { } {
     # We use exp_pid to get the inferior's pid, assuming that gives
@@ -6175,8 +6221,39 @@ proc can_spawn_for_attach { } {
 	return 0
     }
 
-    # Assume yes.
-    return 1
+    # The normal sequence to use for a runtime test like
+    # can_spawn_for_attach_1 is:
+    # - gdb_exit (don't use a running gdb, we don't know what state it is in),
+    # - gdb_start (start a new gdb), and
+    # - gdb_exit (cleanup).
+    #
+    # By making can_spawn_for_attach_1 a gdb_caching_proc, we make it
+    # unpredictable which test-case will call it first, and consequently a
+    # test-case may pass in say a full test run, but fail when run
+    # individually, due to a can_spawn_for_attach call in location where a
+    # gdb_exit (as can_spawn_for_attach_1 does) breaks things.
+    # To avoid this, we move the initial gdb_exit out of
+    # can_spawn_for_attach_1, guaranteeing that we end up in the same state
+    # regardless of whether can_spawn_for_attach_1 is called.  However, that
+    # is only necessary for the first call in a test-case, so cache the result
+    # in a global (which should be reset after each test-case) to keep track
+    # of that.
+    #
+    # In summary, we distinguish between three cases:
+    # - first call in first test-case.  Executes can_spawn_for_attach_1.
+    #   Calls gdb_exit, gdb_start, gdb_exit.
+    # - first call in following test-cases.  Uses cached result of
+    #   can_spawn_for_attach_1.  Calls gdb_exit.
+    # - rest.  Use cached result in cache_can_spawn_for_attach_1. Calls no
+    #   gdb_start or gdb_exit.
+    global cache_can_spawn_for_attach_1
+    if { [info exists cache_can_spawn_for_attach_1] } {
+	return $cache_can_spawn_for_attach_1
+    }
+    gdb_exit
+
+    set cache_can_spawn_for_attach_1 [can_spawn_for_attach_1]
+    return $cache_can_spawn_for_attach_1
 }
 
 # Centralize the failure checking of "attach" command.
@@ -6289,20 +6366,12 @@ proc spawn_id_get_pid { spawn_id } {
     return $testpid
 }
 
-# Start a set of programs running and then wait for a bit, to be sure
-# that they can be attached to.  Return a list of processes spawn IDs,
-# one element for each process spawned.  It's a test error to call
-# this when [can_spawn_for_attach] is false.
+# Helper function for spawn_wait_for_attach and can_spawn_for_attach_1.  As
+# spawn_wait_for_attach, but doesn't check for can_spawn_for_attach.
 
-proc spawn_wait_for_attach { executable_list } {
+proc spawn_wait_for_attach_1 { executable_list } {
     set spawn_id_list {}
 
-    if ![can_spawn_for_attach] {
-	# The caller should have checked can_spawn_for_attach itself
-	# before getting here.
-	error "can't spawn for attach with this target/board"
-    }
-
     foreach {executable} $executable_list {
 	# Note we use Expect's spawn, not Tcl's exec, because with
 	# spawn we control when to wait for/reap the process.  That
@@ -6316,6 +6385,21 @@ proc spawn_wait_for_attach { executable_list } {
     return $spawn_id_list
 }
 
+# Start a set of programs running and then wait for a bit, to be sure
+# that they can be attached to.  Return a list of processes spawn IDs,
+# one element for each process spawned.  It's a test error to call
+# this when [can_spawn_for_attach] is false.
+
+proc spawn_wait_for_attach { executable_list } {
+    if ![can_spawn_for_attach] {
+	# The caller should have checked can_spawn_for_attach itself
+	# before getting here.
+	error "can't spawn for attach with this target/board"
+    }
+
+    return [spawn_wait_for_attach_1 $executable_list]
+}
+
 #
 # gdb_load_cmd -- load a file into the debugger.
 #		  ARGS - additional args to load command.

base-commit: 7320840f1998547b4428c58d1b39ca41febad83a
-- 
2.35.3