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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 74e0dee1b0f9c2e5d8e06b8730e05cc180c275dc..842dc0a1374b65835ccaa556178fe278b4616ce7 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2021-02-25  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       PR gdb/26861
+       * target.c (target_mourn_inferior): Only compare pids in
+       target_mourn_inferior.
+
 2021-02-25  Jan Matyas  <jmatyas@codasip.com>
 
        PR gdb/26819

diff --git a/gdb/target.c b/gdb/target.c
index ba445e7fd34c9fe1aad86bc8bdda55eee4060ec3..0889da82ea593ad295741e75f6455c1fcd45d055 100644 (file)
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2129,7 +2129,7 @@ default_mourn_inferior (struct target_ops *self)
 void
 target_mourn_inferior (ptid_t ptid)
 {
-  gdb_assert (ptid == inferior_ptid);
+  gdb_assert (ptid.pid () == inferior_ptid.pid ());
   current_top_target ()->mourn_inferior ();
 
   /* We no longer need to keep handles on any of the object files.