diff mbox

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

Message ID 1517222676-467-1-git-send-email-omair.javaid@linaro.org
State New, archived
Headers

Commit Message

Omair Javaid Jan. 29, 2018, 10:44 a.m. UTC 
  This behaviour was observed with OpenOCD GDB stub where gdb was failing to stop when interrupted after issuing run command that does not preceede a continue.

Bug report can be found here: https://bugs.launchpad.net/gcc-arm-embedded/+bug/1594341

Here are the steps to reproduce:
1) arm-none-eabi-gdb file-to-debug.elf
2) target remote :3333
At this stage gdb would have connected and halted successfully.
3) run
Issue ctrl + C to interrupt running program and GDB wont be able to stop.

The reason narrowed down to be a case where gdb was unable to clear stop_soon to NO_STOP_QUIETLY;
As some gdb stubs dont report a PID in stop reply the inferior_ptid stays null. A call to remote_current_thread may assign a magic PID in that case.
Based on inferior_ptid function inferior_clear_proceed_status updates inferior->control.stop_soon = NO_STOP_QUIETLY;
If extended_remote_create_inferior calls inferior_clear_proceed_status before a magic PID is assigned to current inferior then it will fail to set inferior->control.stop_soon = NO_STOP_QUIETLY;

This patch adjusts call to inferior_clear_proceed_status such that a PID is assigned to inferior before we try to update inferior->control.stop_soon.

gdb/ChangeLog:
2018-01-29  Omair Javaid  <omair.javaid@linaro.org>

	* remote.c: (extended_remote_create_inferior): Adjust call to
	inferior_clear_proceed_status.
---
 gdb/remote.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Yao Qi Jan. 31, 2018, 4:41 p.m. UTC | #1 
Omair Javaid <omair.javaid@linaro.org> writes:

> Here are the steps to reproduce:
> 1) arm-none-eabi-gdb file-to-debug.elf
> 2) target remote :3333

"target extended-remote :3333".

> At this stage gdb would have connected and halted successfully.
> 3) run
> Issue ctrl + C to interrupt running program and GDB wont be able to stop.

Could you show the log of "set debug remote 1", especially after
"ctrl-c" is typed.  I want to understand what is going on over these
packets between GDB and stub, and reproduce it on GDBserver by disabling
some packets.  This is what I get,

(gdb) c
Continuing.
Sending packet: $Z0,7ffff7dea940,1#e0...Packet received: OK
Sending packet: $QPassSignals:#f3...Packet received: OK
Sending packet: $Hc0#db...Packet received: OK
Sending packet: $s#73...Packet received: T0506:c0dcffffff7f0000;07:c0dcffffff7f0000;10:7f05400000000000;
Sending packet: $Z0,40057a,1#74...Packet received: OK
Sending packet: $QPassSignals:e;10;14;17;1a;1b;1c;21;24;25;2c;4c;97;#0a...Packet received: OK
Sending packet: $Hcpa410.0#6f...Packet received: E01
Sending packet: $c#63...^Cremote_pass_ctrlc called
remote_interrupt called
Packet received: T0206:ffffffff00000000;07:d8daffffff7f0000;10:e05dadf7ff7f0000;
Sending packet: $qL1200000000000000000#50...Packet received: 

Program received signal SIGINT, Interrupt.
Sending packet: $z0,40057a,1#94...Packet received: OK

In the stop reply from my gdbserver, there is no "thread id" either.  My
stop reply has expedite registers while OpenOCD doesn't.  I don't think
this matters here.
Omair Javaid Jan. 31, 2018, 5 p.m. UTC | #2 
On 31 January 2018 at 21:41, Yao Qi <qiyaoltc@gmail.com> wrote:

> Omair Javaid <omair.javaid@linaro.org> writes:
>
> > Here are the steps to reproduce:
> > 1) arm-none-eabi-gdb file-to-debug.elf
> > 2) target remote :3333
>
> "target extended-remote :3333".
>
> > At this stage gdb would have connected and halted successfully.
> > 3) run
> > Issue ctrl + C to interrupt running program and GDB wont be able to stop.
>
> Could you show the log of "set debug remote 1", especially after
> "ctrl-c" is typed.  I want to understand what is going on over these
> packets between GDB and stub, and reproduce it on GDBserver by disabling
> some packets.  This is what I get,
>

Heres the gdb log with run (no other command b/w connection and run):
https://paste.ubuntu.com/26496015/

Heres the gdb log with continue + run (after we issue a continue/interrupt
cycle where inferior PID is set and inferior->control.stop_soon =
NO_STOP_QUIETLY;) https://paste.ubuntu.com/26496048/


>
> (gdb) c
> Continuing.
> Sending packet: $Z0,7ffff7dea940,1#e0...Packet received: OK
> Sending packet: $QPassSignals:#f3...Packet received: OK
> Sending packet: $Hc0#db...Packet received: OK
> Sending packet: $s#73...Packet received: T0506:c0dcffffff7f0000;07:
> c0dcffffff7f0000;10:7f05400000000000;
> Sending packet: $Z0,40057a,1#74...Packet received: OK
> Sending packet: $QPassSignals:e;10;14;17;1a;1b;1c;21;24;25;2c;4c;97;#0a...Packet
> received: OK
> Sending packet: $Hcpa410.0#6f...Packet received: E01
> Sending packet: $c#63...^Cremote_pass_ctrlc called
> remote_interrupt called
> Packet received: T0206:ffffffff00000000;07:d8daffffff7f0000;10:
> e05dadf7ff7f0000;
> Sending packet: $qL1200000000000000000#50...Packet received:
>
> Program received signal SIGINT, Interrupt.
> Sending packet: $z0,40057a,1#94...Packet received: OK
>
> In the stop reply from my gdbserver, there is no "thread id" either.  My
> stop reply has expedite registers while OpenOCD doesn't.  I don't think
> this matters here.
>

The situation with OpenOCD happens because we ran and set inferior pid to
null hoping to update it. Later in extended_remote_create_inferior called
buy run command thread pid gets assigned but we are checking
inferior->control.stop_soon
just before we update the pid and thus it remains to STOP_QUIETLY for
lifetime of the inferior as this flag is only altered by attach command
handlers or during call of clear_proceed_status


>
> --
> Yao (齐尧)
>
Yao Qi Feb. 1, 2018, 12:48 p.m. UTC | #3 
Omair Javaid <omair.javaid@linaro.org> writes:

> Heres the gdb log with run (no other command b/w connection and run):
> https://paste.ubuntu.com/26496015/
>

One thing suspicious to me is an empty reply to qXfer:threads:read

w $qXfer:threads:read::0,fff#03
r $l<?xml version="1.0"?>\n<threads>\n</threads>\n#02

OpenOCD claims support "qXfer:threads:read+;", but such reply above may
confuse GDB.  In OpenOCD source, I find OpenOCD supports qXfer:threads
unconditionally, but I think it should be changed to support it when
"target->rtos != NULL".  This may don't matter.

> Heres the gdb log with continue + run (after we issue a
> continue/interrupt cycle where inferior PID is set and
> inferior->control.stop_soon = NO_STOP_QUIETLY;)
> https://paste.ubuntu.com/26496048/
>

>
> The situation with OpenOCD happens because we ran and set inferior pid
> to null hoping to update it. Later in extended_remote_create_inferior
> called buy run command thread pid gets assigned but we are checking
> inferior->control.stop_soon just before we update the pid and thus it
> remains to STOP_QUIETLY for lifetime of the inferior as this flag is
> only altered by attach command handlers or during call of
> clear_proceed_status 

I still don't understand why inferior->control.stop_soon remains
"STOP_QUIETLY", inferior::control::stop_soon is initialized to
NO_STOP_QUIETLY.  I even don't see where it is changed to STOP_QUIETLY.

I can understand that GDB check inferior_ptid (try to set
inferior->control.stop_soon) first [1], and then update inferior_ptid
later [2],

  if (!have_inferiors ())
    {
      /* Clean up from the last time we ran, before we mark the target
	 running again.  This will mark breakpoints uninserted, and
	 get_offsets may insert breakpoints.  */
      init_thread_list ();
      init_wait_for_inferior ();  <-- [1]
    }

  /* vRun's success return is a stop reply.  */
  stop_reply = run_worked ? rs->buf : NULL;
  add_current_inferior_and_thread (stop_reply);  <-- [2]

However, current_inferior_ isn't changed at all, and
current_inferior_->control.stop_soon is not changed either
(NO_STOP_QUIETLY).  Could you show me where inf->control.stop_soon is
changed from NO_STOP_QUIETLY?
Omair Javaid Feb. 1, 2018, 3:55 p.m. UTC | #4 
On 1 February 2018 at 17:48, Yao Qi <qiyaoltc@gmail.com> wrote:

> Omair Javaid <omair.javaid@linaro.org> writes:
>
> > Heres the gdb log with run (no other command b/w connection and run):
> > https://paste.ubuntu.com/26496015/
> >
>
> One thing suspicious to me is an empty reply to qXfer:threads:read
>
> w $qXfer:threads:read::0,fff#03
> r $l<?xml version="1.0"?>\n<threads>\n</threads>\n#02
>
> OpenOCD claims support "qXfer:threads:read+;", but such reply above may
> confuse GDB.  In OpenOCD source, I find OpenOCD supports qXfer:threads
> unconditionally, but I think it should be changed to support it when
> "target->rtos != NULL".  This may don't matter.
>
> > Heres the gdb log with continue + run (after we issue a
> > continue/interrupt cycle where inferior PID is set and
> > inferior->control.stop_soon = NO_STOP_QUIETLY;)
> > https://paste.ubuntu.com/26496048/
> >
>
> >
> > The situation with OpenOCD happens because we ran and set inferior pid
> > to null hoping to update it. Later in extended_remote_create_inferior
> > called buy run command thread pid gets assigned but we are checking
> > inferior->control.stop_soon just before we update the pid and thus it
> > remains to STOP_QUIETLY for lifetime of the inferior as this flag is
> > only altered by attach command handlers or during call of
> > clear_proceed_status
>
> I still don't understand why inferior->control.stop_soon remains
> "STOP_QUIETLY", inferior::control::stop_soon is initialized to
> NO_STOP_QUIETLY.  I even don't see where it is changed to STOP_QUIETLY.
>
> I can understand that GDB check inferior_ptid (try to set
> inferior->control.stop_soon) first [1], and then update inferior_ptid
> later [2],
>
>   if (!have_inferiors ())
>     {
>       /* Clean up from the last time we ran, before we mark the target
>          running again.  This will mark breakpoints uninserted, and
>          get_offsets may insert breakpoints.  */
>       init_thread_list ();
>       init_wait_for_inferior ();  <-- [1]
>     }
>
>   /* vRun's success return is a stop reply.  */
>   stop_reply = run_worked ? rs->buf : NULL;
>   add_current_inferior_and_thread (stop_reply);  <-- [2]
>
> However, current_inferior_ isn't changed at all, and
> current_inferior_->control.stop_soon is not changed either
> (NO_STOP_QUIETLY).  Could you show me where inf->control.stop_soon is
> changed from NO_STOP_QUIETLY?
>
>
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;


--
> Yao (齐尧)
>
diff mbox

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 5ac84df..3387e1c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9688,13 +9688,16 @@  Remote replied unexpectedly while setting startup-with-shell: %s"),
 	 running again.  This will mark breakpoints uninserted, and
 	 get_offsets may insert breakpoints.  */
       init_thread_list ();
-      init_wait_for_inferior ();
     }
 
   /* vRun's success return is a stop reply.  */
   stop_reply = run_worked ? rs->buf : NULL;
   add_current_inferior_and_thread (stop_reply);
 
+  /* We have called add_current_inferior_and_thread above,
+  call init_wait_for_inferior before new inferior begins.  */
+  init_wait_for_inferior ();
+
   /* Get updated offsets, if the stub uses qOffsets.  */
   get_offsets ();
 }