[PATCH 1/2] gdb/testsuite: gdb.base/attach.exp: fix support check in test_command_line_attach_run

[PATCH 1/2] gdb/testsuite: gdb.base/attach.exp: fix support check in test_command_line_attach_run

[Simon Marchi]

When running this test with the native-extended-gdbserver, we get:

    main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
    19	  while (! should_exit)
    The program being debugged has been started already.
    Start it from the beginning? (y or n) PASS: gdb.base/attach.exp: cmdline attach run: run to prompt
    y
    Don't know how to run.  Try "help target".
    (gdb) FAIL: gdb.base/attach.exp: cmdline attach run: run to main

This test tests using both "-p <pid>" and "-ex start" on the command line,
making sure that we first attach and then run.

Normally, after that "y", we should see the program running again.
However, a particuliarity of the native-extended-gdbserver is that it
uses "set auto-connect-native-target off" on the command line.  The full
GDB command line is:

    ./gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory \
          -iex set height 0 -iex set width 0 -ex set auto-connect-native-target off \
	  -ex set sysroot -quiet -iex set height 0 -iex set width 0 --pid=536609 -ex start

The attach succeeds.  I guess it is done before "set
auto-connect-native-target off", or it somehow bypasses it.  When the
"start" is executed, the native target is unpushed, while killing the
existing process, but not re-pushed, due to "set
auto-connect-native-target off".  So we get that "Don't know how to run"
message.

Really, I think it's a case of the test doing things incompatible with
the board, I think it should just be skipped.  And as we can see with
the current code, there were some attempts at doing this, just using the
wrong checks:

 - isnative: this is a dejagnu proc which checks if the target board has
   the same triplet as the build machine.  In the case of
   native-extended-gdbserver, it does.
 - is_remote target: this checks whether the target board is remote, as
   in executing on a different machin.  native-extended-gdbserver is not
   remote.

Since the --pid option specifically attaches to a process using the
native target, change the test to use gdb_is_target_native instead.

gdb/testsuite/ChangeLog:

	* gdb.base/attach.exp (test_command_line_attach_run): Use
	gdb_is_target_native to check if test is supported.

Change-Id: I762e127f39623889999dc9ed2185540a0951bfb0
---
 gdb/testsuite/gdb.base/attach.exp | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index b0ded430bb27..8b78dcc51811 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -472,14 +472,19 @@ proc_with_prefix test_command_line_attach_run {} {
     global gdb_prompt
     global binfile
 
-    # Skip test if we cannot attach on the command line and use the run command.
-    # ??? Unclear what condition to use to return here when using gdbserver.
-    # ??? None of the below works.
-    #    ![isnative] || [target_is_gdbserver]
-    #    ![isnative] || [use_gdb_stub]
-    if { ![isnative] || [is_remote target] } then {
+    # The --pid option is used to attach to a process using the native target.
+    # Start GDB and run to main just to see what the execution target is, skip
+    # if it's not the native target.
+    clean_restart $binfile
+
+    if { ![runto_main] } {
+	fail "could not run to main"
+	return
+    }
+
+    if { ![gdb_is_target_native] } {
 	unsupported "commandline attach run test"
-	return 0
+	return
     }
 
     set test_spawn_id [spawn_wait_for_attach $binfile]
-- 
2.32.0

[PATCH 2/2] gdb/testsuite: gdb.base/attach.exp: expose bug when testing with native-extended-gdbserver

[Simon Marchi]

In gdb.base/attach.exp, proc do_attach_failure_tests, we attach to a
process.  When then try to attach to the same process in another
inferior, expecting it to fail.  We then come back to the first inferior
and try to kill it, to clean up the test.  When using the
native-extended-gdbserver board, this "kill" test passes, even though it
didn't actually work:

    add-inferior
    [New inferior 2]
    Added inferior 2 on connection 1 (extended-remote localhost:2347)
    (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: add empty inferior 2
    inferior 2
    [Switching to inferior 2 [<null>] (<noexec>)]
    (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 2
    attach 817032
    Attaching to process 817032
    Attaching to process 817032 failed
    (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: fail to attach again
    inferior 1
    [Switching to inferior 1 [process 817032] (/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/attach/attach)]
    [Switching to thread 1.1 (Thread 817032.817032)]
    #0  main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
    19	  while (! should_exit)
    (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: switch to inferior 1
    kill
    Kill the program being debugged? (y or n) y
    Remote connection closed  <==== That's unexpected
    (gdb) PASS: gdb.base/attach.exp: do_attach_failure_tests: exit after attach failures

When the second attach fails, gdbserver seems to break the connection
(it hangs up on the existing remote target) and start listening again
for incoming connections.  This is documented in PR 19558 [1].

Make the expected output regexp for the kill command tighter (it
currently accepts anything).  Use "set confirm off" so we don't have to
deal with the confirmation.  And to be really sure the extended-remote
target still works, try to run the inferior again after killing.  The
now tests are kfail'ed when the target is gdbserver.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=19558

gdb/testsuite/ChangeLog:

	* gdb.base/attach.exp (do_attach_failure_tests): Make kill
	regexp tighter, run inferior after killing it.  Kfail when
	target is gdbserver.

Change-Id: I99c5cd3968ce2ec962ace35b016f842a243b7a0d
---
 gdb/testsuite/gdb.base/attach.exp | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 8b78dcc51811..d6c592275590 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -179,6 +179,10 @@ 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 $" {
@@ -190,13 +194,24 @@ proc_with_prefix do_attach_failure_tests {} {
 	}
     }
 
+    # To ensure the target is still alive and working after this, try to run
+    # inferior 1.
+    gdb_test_no_output "set confirm off"
     gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1"
-    set test "exit after attach failures"
-    gdb_test "kill" \
-	"" \
-	"$test" \
-	"Kill the program being debugged.*y or n. $" \
-	"y"
+
+    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
+	}
+    }
 
     # Another "don't leave a process around"
     kill_wait_spawned_process $test_spawn_id
-- 
2.32.0

Re: [PATCH 1/2] gdb/testsuite: gdb.base/attach.exp: fix support check in test_command_line_attach_run

[Simon Marchi]

I pushed these two patches.

Simon

On 2021-06-10 10:47 a.m., Simon Marchi wrote:
> When running this test with the native-extended-gdbserver, we get:
> 
>     main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/attach.c:19
>     19	  while (! should_exit)
>     The program being debugged has been started already.
>     Start it from the beginning? (y or n) PASS: gdb.base/attach.exp: cmdline attach run: run to prompt
>     y
>     Don't know how to run.  Try "help target".
>     (gdb) FAIL: gdb.base/attach.exp: cmdline attach run: run to main
> 
> This test tests using both "-p <pid>" and "-ex start" on the command line,
> making sure that we first attach and then run.
> 
> Normally, after that "y", we should see the program running again.
> However, a particuliarity of the native-extended-gdbserver is that it
> uses "set auto-connect-native-target off" on the command line.  The full
> GDB command line is:
> 
>     ./gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory \
>           -iex set height 0 -iex set width 0 -ex set auto-connect-native-target off \
> 	  -ex set sysroot -quiet -iex set height 0 -iex set width 0 --pid=536609 -ex start
> 
> The attach succeeds.  I guess it is done before "set
> auto-connect-native-target off", or it somehow bypasses it.  When the
> "start" is executed, the native target is unpushed, while killing the
> existing process, but not re-pushed, due to "set
> auto-connect-native-target off".  So we get that "Don't know how to run"
> message.
> 
> Really, I think it's a case of the test doing things incompatible with
> the board, I think it should just be skipped.  And as we can see with
> the current code, there were some attempts at doing this, just using the
> wrong checks:
> 
>  - isnative: this is a dejagnu proc which checks if the target board has
>    the same triplet as the build machine.  In the case of
>    native-extended-gdbserver, it does.
>  - is_remote target: this checks whether the target board is remote, as
>    in executing on a different machin.  native-extended-gdbserver is not
>    remote.
> 
> Since the --pid option specifically attaches to a process using the
> native target, change the test to use gdb_is_target_native instead.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/attach.exp (test_command_line_attach_run): Use
> 	gdb_is_target_native to check if test is supported.
> 
> Change-Id: I762e127f39623889999dc9ed2185540a0951bfb0
> ---
>  gdb/testsuite/gdb.base/attach.exp | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
> index b0ded430bb27..8b78dcc51811 100644
> --- a/gdb/testsuite/gdb.base/attach.exp
> +++ b/gdb/testsuite/gdb.base/attach.exp
> @@ -472,14 +472,19 @@ proc_with_prefix test_command_line_attach_run {} {
>      global gdb_prompt
>      global binfile
>  
> -    # Skip test if we cannot attach on the command line and use the run command.
> -    # ??? Unclear what condition to use to return here when using gdbserver.
> -    # ??? None of the below works.
> -    #    ![isnative] || [target_is_gdbserver]
> -    #    ![isnative] || [use_gdb_stub]
> -    if { ![isnative] || [is_remote target] } then {
> +    # The --pid option is used to attach to a process using the native target.
> +    # Start GDB and run to main just to see what the execution target is, skip
> +    # if it's not the native target.
> +    clean_restart $binfile
> +
> +    if { ![runto_main] } {
> +	fail "could not run to main"
> +	return
> +    }
> +
> +    if { ![gdb_is_target_native] } {
>  	unsupported "commandline attach run test"
> -	return 0
> +	return
>      }
>  
>      set test_spawn_id [spawn_wait_for_attach $binfile]
>