[2/3] Fix bkpt-other-inferior.exp race

Message ID 20210315234339.457551-3-pedro@palves.net
State New
Headers show
Series
  • Fix gdbserver + "maint set target-non-stop" problems
Related show

Commit Message

Pedro Alves March 15, 2021, 11:43 p.m.
gdb.server/bkpt-other-inferior.exp sometimes fails like so:

 (gdb) inferior 2
 [Switching to inferior 2 [process 368191] (<noexec>)]
 [Switching to thread 2.1 (Thread 368191.368191)]
 [remote] Sending packet: $m7ffff7fd0100,1#5b
 [remote] Packet received: 48
 [remote] Sending packet: $m7ffff7fd0100,1#5b
 [remote] Packet received: 48
 [remote] Sending packet: $m7ffff7fd0100,9#63
 [remote] Packet received: 4889e7e8e80c000049
 #0  0x00007ffff7fd0100 in ?? ()
 (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: switch to inferior
 break -q main
 Breakpoint 2 at 0x1138: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/server.c, line 21.
 (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: set breakpoint
 delete breakpoints
 Delete all breakpoints? (y or n) y
 (gdb) [remote] wait: enter
 [remote] wait: exit
 FAIL: gdb.server/bkpt-other-inferior.exp: inf 2: delete all breakpoints in delete_breakpoints (timeout)
 ERROR: breakpoints not deleted
 Remote debugging from host ::1, port 55876
 monitor exit

The problem is here:

 (gdb) [remote] wait: enter

The testcase isn't expecting any output after the prompt.  This fixes
it by removing the anchor.

Change-Id: I2fd152fd9c46b1c5e7fa678cc4d4054dac0b2bd4
---
 gdb/testsuite/gdb.server/bkpt-other-inferior.exp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.26.2

Comments

will schmidt via Gdb-patches March 17, 2021, 3:12 p.m. | #1
On 2021-03-15 7:43 p.m., Pedro Alves wrote:
> gdb.server/bkpt-other-inferior.exp sometimes fails like so:

> 

>  (gdb) inferior 2

>  [Switching to inferior 2 [process 368191] (<noexec>)]

>  [Switching to thread 2.1 (Thread 368191.368191)]

>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>  [remote] Packet received: 48

>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>  [remote] Packet received: 48

>  [remote] Sending packet: $m7ffff7fd0100,9#63

>  [remote] Packet received: 4889e7e8e80c000049

>  #0  0x00007ffff7fd0100 in ?? ()

>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: switch to inferior

>  break -q main

>  Breakpoint 2 at 0x1138: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/server.c, line 21.

>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: set breakpoint

>  delete breakpoints

>  Delete all breakpoints? (y or n) y

>  (gdb) [remote] wait: enter

>  [remote] wait: exit

>  FAIL: gdb.server/bkpt-other-inferior.exp: inf 2: delete all breakpoints in delete_breakpoints (timeout)

>  ERROR: breakpoints not deleted

>  Remote debugging from host ::1, port 55876

>  monitor exit

> 

> The problem is here:

> 

>  (gdb) [remote] wait: enter

> 

> The testcase isn't expecting any output after the prompt.  This fixes

> it by removing the anchor.


I don't really understand what happens here.  When I replicate the test
manually, I don't see this pair of enter/exit.  Also, since the test
specifically exists to ensure there won't be communcation with the
remote target when inserting the breakpoint, why should we expect the
remote target's wait to get called?

Is there a way to hack something to make the failure happen all the
time?

Simon
Pedro Alves March 19, 2021, 7:11 p.m. | #2
On 17/03/21 15:12, Simon Marchi wrote:
> 

> 

> On 2021-03-15 7:43 p.m., Pedro Alves wrote:

>> gdb.server/bkpt-other-inferior.exp sometimes fails like so:

>>

>>  (gdb) inferior 2

>>  [Switching to inferior 2 [process 368191] (<noexec>)]

>>  [Switching to thread 2.1 (Thread 368191.368191)]

>>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>>  [remote] Packet received: 48

>>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>>  [remote] Packet received: 48

>>  [remote] Sending packet: $m7ffff7fd0100,9#63

>>  [remote] Packet received: 4889e7e8e80c000049

>>  #0  0x00007ffff7fd0100 in ?? ()

>>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: switch to inferior

>>  break -q main

>>  Breakpoint 2 at 0x1138: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/server.c, line 21.

>>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: set breakpoint

>>  delete breakpoints

>>  Delete all breakpoints? (y or n) y

>>  (gdb) [remote] wait: enter

>>  [remote] wait: exit

>>  FAIL: gdb.server/bkpt-other-inferior.exp: inf 2: delete all breakpoints in delete_breakpoints (timeout)

>>  ERROR: breakpoints not deleted

>>  Remote debugging from host ::1, port 55876

>>  monitor exit

>>

>> The problem is here:

>>

>>  (gdb) [remote] wait: enter

>>

>> The testcase isn't expecting any output after the prompt.  This fixes

>> it by removing the anchor.

> 

> I don't really understand what happens here.  When I replicate the test

> manually, I don't see this pair of enter/exit.  


Sorry, I forget mentioning in the commit log that this triggers
with "maint set target-non-stop on".

If I run the testcase in a loop like this:

 $ (set -e; while true; do make check RUNTESTFLAGS="--target_board=native-extended-gdbserver-asns" TESTS="*/bkpt-other-inferior.exp"; done)

using this board file:

 $ cat native-extended-gdbserver-asns.exp 
 load_board_description "native-extended-gdbserver"

 set GDBFLAGS "${GDBFLAGS} -ex \"maint set target-non-stop on\""
 
 send_user "configuring for gdbserver local testing (extended-remote, as-ns)\n"

... then it fails after a few iterations.

Even on a passing run, I see the "[remote] wait:" in the log.  It's just
a timing issue, most times expect consumes the GDB prompt before GDB outputs
that remote log print.

> Also, since the test

> specifically exists to ensure there won't be communcation with the

> remote target when inserting the breakpoint, why should we expect the

> remote target's wait to get called?

> 


See updated commit log in the updated patch below.

From b6b70ac7266589a3eddd58ac0e7263fe8181dff3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>

Date: Sat, 13 Feb 2021 16:25:26 +0000
Subject: [PATCH] Fix bkpt-other-inferior.exp race

When testing with "maint set target-non-stop on",
gdb.server/bkpt-other-inferior.exp sometimes fails like so:

 (gdb) inferior 2
 [Switching to inferior 2 [process 368191] (<noexec>)]
 [Switching to thread 2.1 (Thread 368191.368191)]
 [remote] Sending packet: $m7ffff7fd0100,1#5b
 [remote] Packet received: 48
 [remote] Sending packet: $m7ffff7fd0100,1#5b
 [remote] Packet received: 48
 [remote] Sending packet: $m7ffff7fd0100,9#63
 [remote] Packet received: 4889e7e8e80c000049
 #0  0x00007ffff7fd0100 in ?? ()
 (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: switch to inferior
 break -q main
 Breakpoint 2 at 0x1138: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/server.c, line 21.
 (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: set breakpoint
 delete breakpoints
 Delete all breakpoints? (y or n) y
 (gdb) [remote] wait: enter
 [remote] wait: exit
 FAIL: gdb.server/bkpt-other-inferior.exp: inf 2: delete all breakpoints in delete_breakpoints (timeout)
 ERROR: breakpoints not deleted
 Remote debugging from host ::1, port 55876
 monitor exit

The problem is here:

 (gdb) [remote] wait: enter

The testcase isn't expecting any output after the prompt.

Why is that "[remote] wait" output?  What happens is that "delete
breakpoints" queries the user, and `query` disables/reenables target
async, which results in the remote target's async event handler ending
up marked:

 (top-gdb) bt
 #0  mark_async_event_handler (async_handler_ptr=0x556bffffffff) at ../../src/gdb/async-event.c:295
 #1  0x0000556bf71b711f in infrun_async (enable=1) at ../../src/gdb/infrun.c:119
 #2  0x0000556bf7471387 in target_async (enable=1) at ../../src/gdb/target.c:3684
 #3  0x0000556bf748a0bd in gdb_readline_wrapper_cleanup::~gdb_readline_wrapper_cleanup (this=0x7ffe3cf30eb0, __in_chrg=<optimized out>) at ../../src/gdb/top.c:1074
 #4  0x0000556bf74874e2 in gdb_readline_wrapper (prompt=0x556bfa17da60 "Delete all breakpoints? (y or n) ") at ../../src/gdb/top.c:1096
 #5  0x0000556bf75111c5 in defaulted_query(const char *, char, typedef __va_list_tag __va_list_tag *) (ctlstr=0x556bf7717f34 "Delete all breakpoints? ", defchar=0 '\000', args=0x7ffe3cf31020) at ../../src/gdb/utils.c:893
 #6  0x0000556bf751166f in query (ctlstr=0x556bf7717f34 "Delete all breakpoints? ") at ../../src/gdb/utils.c:985
 #7  0x0000556bf6f11404 in delete_command (arg=0x0, from_tty=1) at ../../src/gdb/breakpoint.c:13500
 ...

... which then later results in a target_wait call:

 (top-gdb) bt
 #0  remote_target::wait_ns (this=0x7ffe3cf30f80, ptid=..., status=0xde530314f0802800, options=...) at ../../src/gdb/remote.c:7937
 #1  0x0000556bf7369dcb in remote_target::wait (this=0x556bfa0b2180, ptid=..., status=0x7ffe3cf31568, options=...) at ../../src/gdb/remote.c:8173
 #2  0x0000556bf745e527 in target_wait (ptid=..., status=0x7ffe3cf31568, options=...) at ../../src/gdb/target.c:2000
 #3  0x0000556bf71be686 in do_target_wait_1 (inf=0x556bfa1573d0, ptid=..., status=0x7ffe3cf31568, options=...) at ../../src/gdb/infrun.c:3463
 #4  0x0000556bf71be88b in <lambda(inferior*)>::operator()(inferior *) const (__closure=0x7ffe3cf31320, inf=0x556bfa1573d0) at ../../src/gdb/infrun.c:3526
 #5  0x0000556bf71bebcd in do_target_wait (wait_ptid=..., ecs=0x7ffe3cf31540, options=...) at ../../src/gdb/infrun.c:3539
 #6  0x0000556bf71bf97b in fetch_inferior_event () at ../../src/gdb/infrun.c:3879
 #7  0x0000556bf71a27f8 in inferior_event_handler (event_type=INF_REG_EVENT) at ../../src/gdb/inf-loop.c:42
 #8  0x0000556bf71cc8b7 in infrun_async_inferior_event_handler (data=0x0) at ../../src/gdb/infrun.c:9220
 #9  0x0000556bf6ecb80f in check_async_event_handlers () at ../../src/gdb/async-event.c:327
 #10 0x0000556bf76b011a in gdb_do_one_event () at ../../src/gdbsupport/event-loop.cc:216
 ...

... which returns TARGET_WAITKIND_IGNORE.

Fix this by only enabling remote output around setting the breakpoint.

gdb/testsuite/ChangeLog:

	* gdb.server/bkpt-other-inferior.exp: Only enable remote output
	around setting the breakpoint.

Change-Id: I2fd152fd9c46b1c5e7fa678cc4d4054dac0b2bd4
---
 gdb/testsuite/gdb.server/bkpt-other-inferior.exp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
index 8992f190d21..5afe621de71 100644
--- a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
+++ b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
@@ -71,13 +71,13 @@ gdb_test_multiple "file" $test {
 # breakpoint on inferior 1, since it has symbols, and should not
 # result in any access to inferior 2's remote target.
 
-gdb_test_no_output "set debug remote 1"
-
 foreach inf_sel {1 2} {
     with_test_prefix "inf $inf_sel" {
 	gdb_test "inferior $inf_sel" "Switching to inferior $inf_sel.*" \
 	    "switch to inferior"
 
+	gdb_test_no_output "set debug remote 1"
+
 	set test "set breakpoint"
 	gdb_test_multiple "break -q main" $test {
 	    -re "Sending packet.*$gdb_prompt $" {
@@ -88,6 +88,8 @@ foreach inf_sel {1 2} {
 	    }
 	}
 
+	gdb_test_no_output "set debug remote 0"
+
 	delete_breakpoints
     }
 }

base-commit: 7b9f985957798ba4dacc454f22c9e426c6897cb8
-- 
2.26.2
will schmidt via Gdb-patches March 25, 2021, 5:24 p.m. | #3
On 2021-03-19 3:11 p.m., Pedro Alves wrote:
> On 17/03/21 15:12, Simon Marchi wrote:

>>

>>

>> On 2021-03-15 7:43 p.m., Pedro Alves wrote:

>>> gdb.server/bkpt-other-inferior.exp sometimes fails like so:

>>>

>>>  (gdb) inferior 2

>>>  [Switching to inferior 2 [process 368191] (<noexec>)]

>>>  [Switching to thread 2.1 (Thread 368191.368191)]

>>>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>>>  [remote] Packet received: 48

>>>  [remote] Sending packet: $m7ffff7fd0100,1#5b

>>>  [remote] Packet received: 48

>>>  [remote] Sending packet: $m7ffff7fd0100,9#63

>>>  [remote] Packet received: 4889e7e8e80c000049

>>>  #0  0x00007ffff7fd0100 in ?? ()

>>>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: switch to inferior

>>>  break -q main

>>>  Breakpoint 2 at 0x1138: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/server.c, line 21.

>>>  (gdb) PASS: gdb.server/bkpt-other-inferior.exp: inf 2: set breakpoint

>>>  delete breakpoints

>>>  Delete all breakpoints? (y or n) y

>>>  (gdb) [remote] wait: enter

>>>  [remote] wait: exit

>>>  FAIL: gdb.server/bkpt-other-inferior.exp: inf 2: delete all breakpoints in delete_breakpoints (timeout)

>>>  ERROR: breakpoints not deleted

>>>  Remote debugging from host ::1, port 55876

>>>  monitor exit

>>>

>>> The problem is here:

>>>

>>>  (gdb) [remote] wait: enter

>>>

>>> The testcase isn't expecting any output after the prompt.  This fixes

>>> it by removing the anchor.

>>

>> I don't really understand what happens here.  When I replicate the test

>> manually, I don't see this pair of enter/exit.  

> 

> Sorry, I forget mentioning in the commit log that this triggers

> with "maint set target-non-stop on".

> 

> If I run the testcase in a loop like this:

> 

>  $ (set -e; while true; do make check RUNTESTFLAGS="--target_board=native-extended-gdbserver-asns" TESTS="*/bkpt-other-inferior.exp"; done)

> 

> using this board file:

> 

>  $ cat native-extended-gdbserver-asns.exp 

>  load_board_description "native-extended-gdbserver"

> 

>  set GDBFLAGS "${GDBFLAGS} -ex \"maint set target-non-stop on\""

>  

>  send_user "configuring for gdbserver local testing (extended-remote, as-ns)\n"

> 

> ... then it fails after a few iterations.

> 

> Even on a passing run, I see the "[remote] wait:" in the log.  It's just

> a timing issue, most times expect consumes the GDB prompt before GDB outputs

> that remote log print.


Ok, I see.

I think these debug outputs are useful (that's why I added them), but
it's a shame that they interfere with the testing...

That LGTM then.

Simon

Patch

diff --git a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
index 8992f190d21..df7af35fbb7 100644
--- a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
+++ b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
@@ -71,23 +71,25 @@  gdb_test_multiple "file" $test {
 # breakpoint on inferior 1, since it has symbols, and should not
 # result in any access to inferior 2's remote target.
 
-gdb_test_no_output "set debug remote 1"
-
 foreach inf_sel {1 2} {
     with_test_prefix "inf $inf_sel" {
 	gdb_test "inferior $inf_sel" "Switching to inferior $inf_sel.*" \
 	    "switch to inferior"
 
+	gdb_test_no_output "set debug remote 1"
+
 	set test "set breakpoint"
 	gdb_test_multiple "break -q main" $test {
-	    -re "Sending packet.*$gdb_prompt $" {
+	    -re "Sending packet.*$gdb_prompt " {
 		fail $test
 	    }
-	    -re "^break -q main\r\nBreakpoint .* at .*$gdb_prompt $" {
+	    -re "^break -q main\r\nBreakpoint .* at .*$gdb_prompt " {
 		pass $test
 	    }
 	}
 
+	gdb_test_no_output "set debug remote 0"
+
 	delete_breakpoints
     }
 }