This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 25 Nov 2015 15:06:41 +0000
- Subject: Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
- Authentication-results: sourceware.org; auth=none
- References: <1444836486-25679-1-git-send-email-palves at redhat dot com> <1444836486-25679-12-git-send-email-palves at redhat dot com> <86ziz4zlof dot fsf at gmail dot com>
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