This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] waiting_for_stop_reply around remote_fileio_request
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Thu, 04 Feb 2016 15:11:11 +0000
- Subject: Re: [PATCH] waiting_for_stop_reply around remote_fileio_request
- Authentication-results: sourceware.org; auth=none
- References: <1454064546-4419-1-git-send-email-yao dot qi at linaro dot org> <56B35836 dot 9000909 at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
> LGTM, with nits below.
>
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index d5701e3..f396a8f 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -6994,8 +6994,16 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
>> status->value.sig = GDB_SIGNAL_0;
>> break;
>> case 'F': /* File-I/O request. */
>> + /* GDB may access the inferior memory while handling the File-I/O
>> + request, but we don't want it GDB accessing memory while waiting
>
> Either "want it", or "want GDB", instead of "want it GDB".
>
>
>> remote_fileio_request (buf, rs->ctrlc_pending_p);
>> rs->ctrlc_pending_p = 0;
>> + /* GDB handled the File-I/O request, but the target is running
>> + again. Keep waiting for events. */
>
> s/but/and/.
Fixed. Patch below is pushed in.
--
Yao (éå)
From e42e5352d1d1e8a262178f606a5df3d0d988f78a Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 4 Feb 2016 15:09:09 +0000
Subject: [PATCH] waiting_for_stop_reply around remote_fileio_request
Hi,
I see this error when GDB connects with qemu,
(gdb) n
....
Sending packet: $vCont;c#a8...Ack
Packet received: Ffstat,00000001,f6fff038
Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.
looks we don't set rs->waiting_for_stop_reply to zero
before handle fileio request,
#10 0x00000000005edb64 in target_write (len=64, offset=4143968312, buf=0x7fffffffd570 "\375\377\377\377", annex=0x0, object=TARGET_OBJECT_MEMORY,
ops=<optimised out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:1922
#11 target_write_memory (memaddr=memaddr@entry=4143968312, myaddr=myaddr@entry=0x7fffffffd6a0 "", len=len@entry=64)
at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:1500
#12 0x00000000004b2b41 in remote_fileio_func_fstat (buf=0x127b258 "") at /home/yao/SourceCode/gnu/gdb/git/gdb/remote-fileio.c:1037
#13 0x00000000004b1878 in do_remote_fileio_request (uiout=<optimised out>, buf_arg=buf_arg@entry=0x127b240)
at /home/yao/SourceCode/gnu/gdb/git/gdb/remote-fileio.c:1204
#14 0x00000000005b8c7c in catch_exceptions_with_msg (func_uiout=<optimised out>, func=func@entry=0x4b1800 <do_remote_fileio_request>,
func_args=func_args@entry=0x127b240, gdberrmsg=gdberrmsg@entry=0x0, mask=mask@entry=RETURN_MASK_ALL)
at /home/yao/SourceCode/gnu/gdb/git/gdb/exceptions.c:187
#15 0x00000000005b8dea in catch_exceptions (uiout=<optimised out>, func=func@entry=0x4b1800 <do_remote_fileio_request>, func_args=func_args@entry=0x127b240,
mask=mask@entry=RETURN_MASK_ALL) at /home/yao/SourceCode/gnu/gdb/git/gdb/exceptions.c:167
#16 0x00000000004b2fff in remote_fileio_request (buf=0x127b240 "Xf6fff038,0:", ctrlc_pending_p=0) at /home/yao/SourceCode/gnu/gdb/git/gdb/remote-fileio.c:1255
#17 0x0000000000496f12 in remote_wait_as (ptid=..., status=0x7fffffffdb20, options=1) at /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:6997
however, we did set rs->waiting_for_stop_reply to zero before Luis's
patch https://sourceware.org/ml/gdb-patches/2015-10/msg00336.html
In fact, Luis's patch v1
https://sourceware.org/ml/gdb-patches/2015-08/msg00809.html is about
setting rs->waiting_for_stop_reply back to one after
remote_fileio_request, which is correct. However during the review, the
patch is changed and ends up with "not setting rs->waiting_for_stop_reply
to zero".
I manually test GDB, but I don't have a way to run regression tests.
gdb:
2016-02-04 Yao Qi <yao.qi@linaro.org>
* remote.c (remote_wait_as): Set rs->waiting_for_stop_reply to
0 before handling 'F' and set it back afterwards.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 829a48c..a2b0d39 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-04 Yao Qi <yao.qi@linaro.org>
+
+ * remote.c (remote_wait_as): Set rs->waiting_for_stop_reply to
+ 0 before handling 'F' and set it back afterwards.
+
2016-02-02 Simon Marchi <simon.marchi@ericsson.com>
* ui-out.c (MAX_UI_OUT_LEVELS): Remove.
diff --git a/gdb/remote.c b/gdb/remote.c
index 8831b50..b1af8aa 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6984,8 +6984,16 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
status->value.sig = GDB_SIGNAL_0;
break;
case 'F': /* File-I/O request. */
+ /* GDB may access the inferior memory while handling the File-I/O
+ request, but we don't want GDB accessing memory while waiting
+ for a stop reply. See the comments in putpkt_binary. Set
+ waiting_for_stop_reply to 0 temporarily. */
+ rs->waiting_for_stop_reply = 0;
remote_fileio_request (buf, rs->ctrlc_pending_p);
rs->ctrlc_pending_p = 0;
+ /* GDB handled the File-I/O request, and the target is running
+ again. Keep waiting for events. */
+ rs->waiting_for_stop_reply = 1;
break;
case 'N': case 'T': case 'S': case 'X': case 'W':
{