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

[Pedro Alves <pedro@palves.net>]

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