[Bug gdb/28471] Internal error on Assertion `pid != 0' on AIX

["simon.marchi at polymtl dot ca" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=28471

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #1 from Simon Marchi <simon.marchi at polymtl dot ca> ---
Hi,

Not too familiar with AIX, but I'll try to help.  The AIX target suffers from
bit-rot, as you found out, so needs a bit of updating.

The inferior_ptid is used as "the current thread" value.  A lot of functions in
GDB work on a ptid or thread, but don't take a ptid or thread as argument. 
Instead, they read inferior_ptid.

But as explained in the comment you quoted, target_wait doesn't work that way. 
It is used to pull an event out of the target, it doesn't operate on a specific
thread.  Clearing inferior_ptid prior to calling target_wait (instead of
leaving it set to some random value) ensures the target doesn't rely on it,
like some targets historically did.

So already, the first line of aix_thread_target::wait is suspicious:

  scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);

This saves the value of inferior_ptid and restores it at the end of the scope. 
But we know there is nothing to restore, the entry value of inferior_ptid is
meaningless.

What I understand from the current code (it would need to be confirmed by
somebody who knows AIX) is that we are dealing with something like userspace
threads.  The process layer (rs6000-nat) just knows about a single process /
thread, with ptid (pid=1234, lwp=0, tid=0).  The aix-thread target sits on top
and exposes the threads that are managed by some userspace threading library. 
Threads managed by aix-thread would therefore have ptids (pid=1234, lwp=0,
tid=1), (pid=1234, lwp=0, tid=2), and so forth.

The ptid argument passed to target_wait acts as a filter, it asks the target
"get me events only for that ptid".  So let's say the core of GDB passes
ptid=(pid=1234, lwp=0, tid=2), the aix-thread modifies that ptid to clear the
tid field (function pid_to_prc) before passing the filter to the target below,
because the tid field doesn't make sense to the target below and would confuse
it.  I think that is correct, but the subsequent line:

  inferior_ptid = ptid_t (inferior_ptid.pid ());

doesn't make sense anymore, and should be removed.

Then, here:

  if (ptid.pid () == -1)
    return ptid_t (-1);

Means: if the target didn't produce an interesting event, return early.  This
looks correct, but could be made nicer as:

  if (ptid == minus_one_ptid)
    return minus_one_ptid;

Now, if the target below did return an interesting even, we go in pd_activate
or pd_update.  These functions rely on inferior_ptid, but they shouldn't that
needs to be fixed.  Let's assume that the target below returned an event for
ptid (pid=1234, lwp=0, tid=0), I suppose that pd_activate and pd_update will do
some bookeeping and will fill in that tid field, so that
aix_thread_target::wait returns a ptid for a specific tid.  These functions
used to rely on inferior_ptid, I suppose they can just be changed to accept the
ptid (the one returned by the target below) as argument.  And do the thing that
makes sense, like:

    /* If this target is not active, return the ptid as returned by the target
below.  */
    if (!pd_active)
      return ptid_passed_as_argument;  // used to say inferior_ptid

One question I would have for an AIX expert is: is this threading model /
library still used today, or is it just some ancient stuff?  Userspace threads
are rare these days, so it's possible that this aix-thread target isn't
actually useful, and just gets in the way, and that threads are managed and
exposed by the kernel, and therefore managed by the rs6000-nat target  If so,
maybe aix-thread.c could be removed.

And one last thing: since rs6000-nat.c is AIX-specific, it should be named as
such, rs6000-aix-nat.c.  If you plan on touching that area, could you please
make this change?

-- 
You are receiving this mail because:
You are on the CC list for the bug.