Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io_threads

Stefan Metzmacher
2021-05-05T22:21:28Z

Hi Simon,

> When you attach to PIDVAL (assuming that PIDVAL is a thread-group
> leader), GDB attaches to all the threads of that thread group. The
> inferior_ptid global variable is "the thread we are currently working
> with", and changes whenever GDB wants to deal with a different thread.
>
> After attaching to all threads, GDB wants to know more about that
> process' architecture (that read_description call mentioned in [1]).
> The way this is implemented varies from arch to arch, but as you found
> out, for x86-64 it consists of peeking into a thread's CS/DS to
> determine whether the process is x86-64, x32 or i386. The thread used
> for this - the inferior_ptid value - just happens to be the latest
> thread we switched inferior_ptid to (presumably because we iterated on
> the thread list to do something and that was the last thread in the
> list). And up to now, this was working under the assumption that
> picking any thread of the process would yield the same values for that
> purpose. I don't think it was intentionally done this way, but it
> worked and didn't cause any trouble until now.
>
> With io threads, that assumption doesn't hold anymore.

Yes, in 5.12

> From what I understand, your v1 patch made it so that the kernel puts the CS/DS
> values GDB expects in the io threads (even though they are never
> actually used otherwise). I don't understand how your v2 patch
> addresses the problem though.

v1 did clear everything and tried to keep some selected registers:

'memset(childregs, 0, sizeof(struct pt_regs));'
childregs->cs = currenttrgs->cs;
childregs->ss = currenttrgs->ss;
childregs->ds = currenttrgs->ds;
childregs->es = currenttrgs->es;

v2 copies everything and only clears 3 selected registers (I added the last two for
the PF_IO_WORKER case:

*childregs = *current_pt_regs();
childregs->ax = 0;
...
childregs->ip = 0;
childregs->sp = 0;

So regarding childregs->cs and childregs->ds the effect is the same.

(Also note that on x86_64 ds in handled by savesegment(ds, p->thread.ds);
already instead so the effective problem as only childregs->cs which
is cleared in 5.12, but will be kept with both of my patches.

> I don't think it would be a problem on the GDB-side to make sure to
> fetch these values from a "standard" thread. Most likely the thread
> group leader, like Tom has proposed in [1].

Ok.

Is it clear now?
metze