Re: [PATCH 11/18] gdbserver: fix killed-outside.exp

Pedro Alves
2015-11-25T15:06:46Z

Hi Yao,

It's taking me a bit to go through your comments.
Thanks for the review, and sorry about that.

On 10/27/2015 09:48 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 1c447c2..af9ca22 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -1535,12 +1535,6 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>> if (!lp->status_pending_p)
>> return 0;
>>
>> - /* If we got a `vCont;t', but we haven't reported a stop yet, do
>> - report any status pending the LWP may have. */
>> - if (thread->last_resume_kind == resume_stop
>> - && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
>> - return 0;
>> -
>
> Shouldn't we return 1 instead of 0 here? This is consistent with the
> comments above.

I may have misunderstood what you mean, but if we remove this code,
that's what we get -- the "return 1;" at the end is reached.

>
>> if (thread->last_resume_kind != resume_stop
>> && (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
>> || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
>> @@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>> return 1;
>> }
>>
>> +/* Returns true if LWP is resumed from the client's perspective. */
>> +
>> +static int
>> +lwp_resumed (struct lwp_info *lwp)
>> +{
>> + struct thread_info *thread = get_lwp_thread (lwp);
>> +
>> + if (thread->last_resume_kind != resume_stop)
>> + return 1;
>> +
>> + /* Did we get a `vCont;t', but we haven't reported a stop yet? */
>> + if (thread->last_resume_kind == resume_stop
>> + && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
>> + return 1;
>> +
>
> I feel "reported" isn't precise. Is "got" better? The code means
> GDBserver has already got vCont;t and sent SIGSTOP, but hasn't *got*
> stop event out of event loop. After GDBserver got this event, it then
> reports the event back to GDB if needed.

Hmm, reported to gdb is really what I mean. How about this:

/* Did gdb send us a `vCont;t', but we haven't reported the
corresponding stop to gdb yet? If so, the thread is still
resumed/running from gdb's perspective. */
if (thread->last_resume_kind == resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
return 1;

Thanks,
Pedro Alves