Fix for GDB failing to interrupt after run when no PID issued by stub

Message ID 86o9l2jhuf.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Feb. 6, 2018, 2:47 p.m. UTC
  Omair Javaid <omair.javaid@linaro.org> writes:

> Here is the sequence of change to inf->control.stop_soon
>
> gdb) tar ext :3333
>
> infrun.c clear_proceed_status () line: 2903
> (inferior->control.stop_soon = NO_STOP_QUIETLY;)
>
> infrun.c start_remote () line:3223 (inferior->control.stop_soon =
> STOP_QUIETLY_REMOTE;)
>
> gdb) run
>
> infcmd.c:575 calls run_command_1 call () which sets PID to null 
>
> Further more if we do a continue after connection a call to
> clear_proceed_status with a valid PID will set
> inferior->control.stop_soon = NO_STOP_QUIETLY;

Hi Omair,
I manged to reproduce it with GDBserver, 1.exe is a simple program calls
sleep,

$ ./gdbserver/gdbserver  --multi :1234 ./1.exe
$ ./gdb ./1.exe
(gdb) target extended-remote :1234
Remote debugging using :1234
...
0x00007ffff7ddb2d0 in ?? () from target:/lib64/ld-linux-x86-64.so.2
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /scratch/yao/gdb/build-git/x86_64/gdb/1.exe 
Reading /lib64/ld-linux-x86-64.so.2 from remote target...
Reading /lib64/ld-linux-x86-64.so.2 from remote target...
Reading /lib64/ld-2.19.so from remote target...
Reading /lib64/.debug/ld-2.19.so from remote target...
Reading /lib/x86_64-linux-gnu/libc.so.6 from remote target...
Reading /lib/x86_64-linux-gnu/libc-2.19.so from remote target...
Reading /lib/x86_64-linux-gnu/.debug/libc-2.19.so from remote target...
^C

This is consistent to the bug reported
https://bugs.launchpad.net/gcc-arm-embedded/+bug/1594341

I set watchpoint on inferior->control.stop_soon and inferior_ptid, so I
find that inferior_ptid is reset in generic_mourn_inferior,

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Hardware watchpoint 3: inferior_ptid

Old value = {m_pid = 643, m_lwp = 643, m_tid = 0}
New value = {m_pid = 0, m_lwp = 643, m_tid = 0}
0x0000000000c83ef6 in generic_mourn_inferior () at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:3299
3299      inferior_ptid = null_ptid;
top?bt 10
#0  0x0000000000c83ef6 in generic_mourn_inferior () at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:3299
#1  0x0000000000b9d1f2 in remote_mourn (target=0x1839d40 <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:9431
#2  0x0000000000c621f4 in delegate_mourn_inferior (self=0x1839d40 <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/git/gdb/target-delegates.c:1382
#3  0x0000000000c809b6 in target_mourn_inferior (ptid=...) at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:2397
#4  0x0000000000b9cd45 in remote_kill (ops=0x1839d40 <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:9302
#5  0x0000000000c5fd4e in delegate_kill (self=0x1839d40 <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/git/gdb/target-delegates.c:1015
#6  0x0000000000c7a905 in target_kill () at /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:423
#7  0x0000000000a21025 in kill_if_already_running (from_tty=1) at /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:522
#8  0x0000000000a211cd in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:575
#9  0x0000000000a2183f in run_command (args=0x0, from_tty=1) at /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:688

however, this leads me thinking that why don't we reset inferior too?
Note that generic_mourn_inferior calls exit_inferior_1, where various
fields of inferior are "reset".  The patch below fixes the problem with
gdbserver, does it work with you on OpenOCD?
  

Comments

Omair Javaid Feb. 8, 2018, 10:08 a.m. UTC | #1
On 6 February 2018 at 19:47, Yao Qi <qiyaoltc@gmail.com> wrote:

> Omair Javaid <omair.javaid@linaro.org> writes:
>
> > Here is the sequence of change to inf->control.stop_soon
> >
> > gdb) tar ext :3333
> >
> > infrun.c clear_proceed_status () line: 2903
> > (inferior->control.stop_soon = NO_STOP_QUIETLY;)
> >
> > infrun.c start_remote () line:3223 (inferior->control.stop_soon =
> > STOP_QUIETLY_REMOTE;)
> >
> > gdb) run
> >
> > infcmd.c:575 calls run_command_1 call () which sets PID to null
> >
> > Further more if we do a continue after connection a call to
> > clear_proceed_status with a valid PID will set
> > inferior->control.stop_soon = NO_STOP_QUIETLY;
>
> Hi Omair,
> I manged to reproduce it with GDBserver, 1.exe is a simple program calls
> sleep,
>
> $ ./gdbserver/gdbserver  --multi :1234 ./1.exe
> $ ./gdb ./1.exe
> (gdb) target extended-remote :1234
> Remote debugging using :1234
> ...
> 0x00007ffff7ddb2d0 in ?? () from target:/lib64/ld-linux-x86-64.so.2
> (gdb) r
> The program being debugged has been started already.
> Start it from the beginning? (y or n) y
> Starting program: /scratch/yao/gdb/build-git/x86_64/gdb/1.exe
> Reading /lib64/ld-linux-x86-64.so.2 from remote target...
> Reading /lib64/ld-linux-x86-64.so.2 from remote target...
> Reading /lib64/ld-2.19.so from remote target...
> Reading /lib64/.debug/ld-2.19.so from remote target...
> Reading /lib/x86_64-linux-gnu/libc.so.6 from remote target...
> Reading /lib/x86_64-linux-gnu/libc-2.19.so from remote target...
> Reading /lib/x86_64-linux-gnu/.debug/libc-2.19.so from remote target...
> ^C
>
> This is consistent to the bug reported
> https://bugs.launchpad.net/gcc-arm-embedded/+bug/1594341
>
> I set watchpoint on inferior->control.stop_soon and inferior_ptid, so I
> find that inferior_ptid is reset in generic_mourn_inferior,
>
> (gdb) r
> The program being debugged has been started already.
> Start it from the beginning? (y or n) y
> Hardware watchpoint 3: inferior_ptid
>
> Old value = {m_pid = 643, m_lwp = 643, m_tid = 0}
> New value = {m_pid = 0, m_lwp = 643, m_tid = 0}
> 0x0000000000c83ef6 in generic_mourn_inferior () at
> /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:3299
> 3299      inferior_ptid = null_ptid;
> top?bt 10
> #0  0x0000000000c83ef6 in generic_mourn_inferior () at
> /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:3299
> #1  0x0000000000b9d1f2 in remote_mourn (target=0x1839d40
> <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/
> git/gdb/remote.c:9431
> #2  0x0000000000c621f4 in delegate_mourn_inferior (self=0x1839d40
> <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/
> git/gdb/target-delegates.c:1382
> #3  0x0000000000c809b6 in target_mourn_inferior (ptid=...) at
> /home/yao/SourceCode/gnu/gdb/git/gdb/target.c:2397
> #4  0x0000000000b9cd45 in remote_kill (ops=0x1839d40
> <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/
> git/gdb/remote.c:9302
> #5  0x0000000000c5fd4e in delegate_kill (self=0x1839d40
> <extended_remote_ops>) at /home/yao/SourceCode/gnu/gdb/
> git/gdb/target-delegates.c:1015
> #6  0x0000000000c7a905 in target_kill () at /home/yao/SourceCode/gnu/gdb/
> git/gdb/target.c:423
> #7  0x0000000000a21025 in kill_if_already_running (from_tty=1) at
> /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:522
> #8  0x0000000000a211cd in run_command_1 (args=0x0, from_tty=1,
> run_how=RUN_NORMAL) at /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:575
> #9  0x0000000000a2183f in run_command (args=0x0, from_tty=1) at
> /home/yao/SourceCode/gnu/gdb/git/gdb/infcmd.c:688
>
> however, this leads me thinking that why don't we reset inferior too?
> Note that generic_mourn_inferior calls exit_inferior_1, where various
> fields of inferior are "reset".  The patch below fixes the problem with
> gdbserver, does it work with you on OpenOCD?
>
> --
> Yao (齐尧)
> From 854cca001e56dd40f3e5e5118d1e25ffa06a643b Mon Sep 17 00:00:00 2001
> From: Yao Qi <yao.qi@linaro.org>
> Date: Tue, 6 Feb 2018 14:45:08 +0000
> Subject: [PATCH] Reset inferior::control on inferior exit
>
> When we kill an inferior, the inferior is not deleted.  What is more, it
> is reused when the new process is created, so we need to reset inferior's
> state when it exits.
>

Hi Yao,

Your patch fixes the issue with openOCD stub. I was a little reluctant to
make a generic change with regards to reseting inferior control structure.

If there isnt a need to preserve inferior control structure in any case
then your change is the right solution.

gdb:
>
> 2018-02-06  Yao Qi  <yao.qi@linaro.org>
>
>         * inferior.c (exit_inferior_1): Reset inf->control.
>
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 38b7369..880f25d 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -224,6 +224,8 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
>      }
>
>    inf->pending_detach = 0;
> +  /* Reset it.  */
> +  inf->control = {NO_STOP_QUIETLY};
>  }
>
>  void
>
  
Yao Qi Feb. 15, 2018, 3:06 p.m. UTC | #2
On Thu, Feb 8, 2018 at 10:08 AM, Omair Javaid <omair.javaid@linaro.org> wrote:
> Hi Yao,
>
> Your patch fixes the issue with openOCD stub. I was a little reluctant to
> make a generic change with regards to reseting inferior control structure.
>
> If there isnt a need to preserve inferior control structure in any case then
> your change is the right solution.
>

I opened PR 22849 for this issue.  Pushed patch to both master and 8.1
branch.
  

Patch

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 38b7369..880f25d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -224,6 +224,8 @@  exit_inferior_1 (struct inferior *inftoex, int silent)
     }
 
   inf->pending_detach = 0;
+  /* Reset it.  */
+  inf->control = {NO_STOP_QUIETLY};
 }
 
 void