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: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Thu, 26 Nov 2015 16:51:28 +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> <5655CE81 dot 3090603 at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
>>> - /* 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.
>
What I meant is to change "return 0" to "return 1", like,
/* 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;
+ return 1;
that is the same as removing them.
>>
>>> 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;
Oh, sounds it is about thread's state from GDB's perspective. Patch is
good to me.
By the way, after GDBserver reported the stop GDB, does the thread
changed to
(thread->last_resume_kind == resume_stop
&& thread->last_status.kind != TARGET_WAITKIND_IGNORE)
it is used in proceed_one_lwp.
--
Yao (éå)