Re: [PATCH 8/8] Special-case wildcard requests in ravenscar-thread.c

[Joel Brobecker <brobecker at adacore dot com>]

> > ravenscar-thread.c intercepts resume and wait target requests and
> > replaces the requested ptid with the ptid of the underlying CPU.
> > However, this is incorrect when a request is made with a wildcard
> > ptid.
> 
> Can you clarify a bit more what went wrong?
> IIRC, the base target always has one thread/cpu only, anyway?  What
> difference does the patch make in practice?

This happens when debugging application on a multi-CPU target,
using QEMU, where some tasks are allocated to one CPU, and others
are allocated to the other.

When stopping, QEMU tells us about one thread per CPU, so we start
with one possible base_ptid per CPU.  Previously, sending a resume
order for one thread actually resumed execution on all the CPUs.
But during an upgrade, which behavior was changed so that sending
a resume order for one thread only wakes the corresponding CPU up.

At the user level, we noticed the issue because we had a test were
we insert a breakpoint one some code which is only run from, say,
CPU #2, whereas we unfortunately resumed the execution after having
stopped somewhere in CPU #1. As a result, we sent an order to resume
CPU #1, which starves CPU #2 forever, because the code in CPU #1
waits for some of the Ada tasks allocated to CPU #2 (and we never
reach our breakpoint either).

> The reason I'm wondering is because the patch makes me wonder about
> a step request with no sched-lock, which is the default (*).
> In that case, you'll have:
> 
>  - the thread to step is in inferior_ptid
>  - ptid == minus_one_ptid (indicating everything resumes / no schedlock)
> 
> So seems like after this patch the lower layer might get a request to step
> an unknown inferior_ptid?  Might not happen by luck/accident.
> I'd suspect that you'd want to do instead:
> 
>   inferior_ptid = m_base_ptid;
>   /* If we see a wildcard resume, we simply pass that on.  Otherwise,
>      arrange to resume the base ptid.  */
>   if (ptid != minus_one_ptid)
>     ptid = m_base_ptid;
>   beneath ()->resume (ptid, step, siggnal);
> 
> I.e., always flip inferior_ptid.
> 
> (*) since ravenscar-thread.c doesn't know how to interact with
>     the ravenscar runtime to specify which threads are resumable,
>     "schedlock on" most certainly doesn't work properly.  E.g.,
>       "task 1 stops; set scheduler-locking on; task 2; step"
>     seemingly will step task 1 instead of 2, AFAICT.

That's correct (unless if you are in the particular situation where
you have one task per CPU).

-- 
Joel