diff options
author | Simon Marchi <simon.marchi@polymtl.ca> | 2021-02-25 15:52:29 -0500 |
---|---|---|
committer | Simon Marchi <simon.marchi@polymtl.ca> | 2021-02-25 15:52:29 -0500 |
commit | dffdd8b51f3fb086faf750592709baa8faa66fd1 (patch) | |
tree | ca3765eaf653d2c392f9d8ee8dde42c72ca0e334 /gdb/target.c | |
parent | 64d38fdd9956aefd1f0fbeb7e1d2774e71c0a9b8 (diff) | |
download | gdb-dffdd8b51f3fb086faf750592709baa8faa66fd1.zip gdb-dffdd8b51f3fb086faf750592709baa8faa66fd1.tar.gz gdb-dffdd8b51f3fb086faf750592709baa8faa66fd1.tar.bz2 |
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
Diffstat (limited to 'gdb/target.c')
-rw-r--r-- | gdb/target.c | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/gdb/target.c b/gdb/target.c index ba445e7..0889da8 100644 --- 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. |