Re: qemu-system debugging broken ([PATCH 03/18] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG)

[Pedro Alves <palves@redhat.com>]

Hi Luis,

Sorry, I was away last week, and am still catching up.

On 08/27/2015 11:19 PM, Luis Machado wrote:
> Just a heads-up. It looks like this particular commit...
> 
> commit 567420d10895611e03d5ee65e6b24c16a69a6e99
> Author: Pedro Alves <palves@redhat.com>
> Date:   Fri Aug 7 17:23:56 2015 +0100
> 
>      remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and 
> TARGET_WNOHANG
> 
>      Even though "target remote" supports target-async, the all-stop
>      target_wait implementation ignores TARGET_WNOHANG.  If the core
>      happens to poll for events and we've already read the stop reply out
>      of the serial/socket, remote_wait_as hangs forever instead of
>      returning an indication that there are no events to process.  This
>      can't happen currently, but later changes will trigger this.
> 
>      gdb/ChangeLog:
>      2015-08-07  Pedro Alves  <palves@redhat.com>
> 
>          * remote.c (remote_wait_as): If not waiting for a stop reply,
>          return TARGET_WAITKIND_NO_RESUMED.  If TARGET_WNOHANG is
>          requested, don't block waiting forever.
> 
> ... broke simulator-based debugging. The following is what i get when 
> trying to run the gdb.base/break binary on qemu-system for arm eabi:
> 
> _ftext () at arm-vector.S:25
> 25              ldr pc, [pc, #24] @ reset
> (gdb) load
> Loading section .text, size 0xc01c lma 0x0
> Loading section .eh_frame, size 0x48 lma 0xc01c
> Loading section .ARM.exidx, size 0x8 lma 0xc064
> Loading section .rodata, size 0x398 lma 0xc070
> Loading section .data, size 0x8e0 lma 0xc408
> Start address 0x40, load size 52452
> Transfer rate: 17074 KB/sec, 1748 bytes/write.
> (gdb) c
> Continuing.
> infrun: clear_proceed_status_thread (Thread 1)
> infrun: proceed (addr=0xffffffff, signal=GDB_SIGNAL_DEFAULT)
> infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current 
> thread [Thread 1] at 0x40
> infrun: infrun_async(1)
> infrun: prepare_to_wait
> infrun: target_wait (-1.0.0, status) =
> infrun:   -1.0.0 [Thread 0],
> infrun:   status->kind = ignore
> infrun: TARGET_WAITKIND_IGNORE
> infrun: prepare_to_wait
> infrun: target_wait (-1.0.0, status) =
> infrun:   -1.0.0 [Thread 0],
> infrun:   status->kind = ignore
> infrun: TARGET_WAITKIND_IGNORE
> infrun: prepare_to_wait
> infrun: target_wait (-1.0.0, status) =
> infrun:   -1.0.0 [Thread 0],
> infrun:   status->kind = no-resumed
> infrun: TARGET_WAITKIND_NO_RESUMED
> infrun: stop_waiting
> infrun: clear_step_over_info
> No unwaited-for children left.
> infrun: infrun_async(0)
> (gdb) c
> Continuing.
> Cannot execute this command while the selected thread is running.
> (gdb)
> Continuing.
> Cannot execute this command while the selected thread is running.
> 
> Upon further inspection, it looks like the setting of status->kind to 
> TARGET_WAITKIND_NO_RESUMED in remote_wait_as causes GDB to stop waiting 
> for events and marks the active thread as running, returning the GDB 
> prompt and printing the old familiar message.
> 
> Without setting TARGET_WAITKIND_NO_RESUMED, things worked fine and the 
> binary runs to completion, like so:

...

> It sounds like we  shouldn't drop to the prompt while we wait for
> something to happen in all-stop mode either.

Yes, but if the target was resumed, how come rs->waiting_for_stop_reply
was false?

      if (!rs->waiting_for_stop_reply)
	{
	  status->kind = TARGET_WAITKIND_NO_RESUMED;
	  return minus_one_ptid;
	}

I suspect that what happens is that qemu sends an F packet, and
we miss setting waiting_for_stop_reply true back, like we do
in the other cases:

gdb/remote:remote_wait_as ()
{
...
  /* We got something.  */
  rs->waiting_for_stop_reply = 0;
...
    case 'F':		/* File-I/O request.  */
      remote_fileio_request (buf, rs->ctrlc_pending_p);
      rs->ctrlc_pending_p = 0;
      break;
...

Looks like it'd be simpler to instead only clear
waiting_for_stop_reply in the stop reply cases, instead of
re-setting it in some cases, forgetting others.

Thanks,
Pedro Alves