gdb: relax assertion in target_mourn_inferior
authorSimon Marchi <simon.marchi@polymtl.ca>
Thu, 25 Feb 2021 20:52:29 +0000 (15:52 -0500)
committerSimon Marchi <simon.marchi@polymtl.ca>
Thu, 25 Feb 2021 20:52:29 +0000 (15:52 -0500)
commitdffdd8b51f3fb086faf750592709baa8faa66fd1
treeca3765eaf653d2c392f9d8ee8dde42c72ca0e334
parent64d38fdd9956aefd1f0fbeb7e1d2774e71c0a9b8
gdb: relax assertion in target_mourn_inferior

As reported in PR 26861, when killing an inferior on macOS, we hit the
assert:

    ../../gdb-10.1/gdb/target.c:2149: internal-error: void target_mourn_inferior(ptid_t): Assertion `ptid == inferior_ptid' failed.

This is because darwin_nat_target::kill passes a pid-only ptid to
target_mourn_inferior, with the pid of the current inferior:

    target_mourn_inferior (ptid_t (inf->pid));

... which doesn't satisfy the assert in target_mourn_inferior:

    gdb_assert (ptid == inferior_ptid);

The reason for this assertion is that target_mourn_inferior is a
prototype shared between GDB and GDBserver, so that shared code in
gdb/nat (used in both GDB and GDBserver) can call target_mourn_inferior.
In GDB's implementation, it is likely that some targets still rely on
inferior_ptid being set to "the current thread we are working on".  So
until targets are completely decoupled from inferior_ptid (at least
their mourn_inferior implementations), we need to ensure the passed in
ptid matches inferior_ptid, to ensure the calling code called
target_mourn_inferior with the right global context.

However, I think the assert is a bit too restrictive.  The
mourn_inferior operation works on an inferior, not a specific thread.
And by the time we call mourn_inferior, the threads of the inferior
don't exist anymore, the process is gone, so it doesn't really make
sense to require inferior_ptid to point a specific thread.

I looked at all the target_ops::mourn_inferior implementations, those
that read inferior_ptid only care about the pid field, which supports
the idea that only the inferior matters.  Other implementations look at
the current inferior (call `current_inferior ()`).

I think it would make sense to change target_mourn_inferior to accept
only a pid rather than a ptid.  It would then assert that the pid is the
same as the current inferior's pid.  However, this would be a quite
involved change, so I'll keep it for later.

To fix the macOS issue immediately, I propose to relax the assert to
only compare the pids, as is done in this patch.

Another solution would obviously be to make darwin_nat_target::kill pass
inferior_ptid to target_mourn_inferior.  However, the solution I propose
is more in line with where I think we want to go (passing a pid to
target_mourn_inferior).

gdb/ChangeLog:

PR gdb/26861
* target.c (target_mourn_inferior): Only compare pids in
target_mourn_inferior.

Change-Id: If2439ccc5aa67272ea16148a43c5362ef23fb2b8
gdb/ChangeLog
gdb/target.c