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

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