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

[Luis Machado <lgustavo@codesourcery.com>]

Hi Pedro,

On 09/10/2015 07:20 AM, Pedro Alves wrote:
> Hi Luis,
>
> Sorry, I was away last week, and am still catching up.
>

No worries. I have since figured this out and addressed it in a 
different message ...

> 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.

... and it is really the case that we were forgetting to reset 
rs->ctrlc_pending_p.