MatCat.OpenSource/8sa1-binutils-gdb
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

gdb: relax assertion in target_mourn_inferior

Browse Source 
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
This commit is contained in:
Simon Marchi 2021-02-25 15:52:29 -05:00
parent 64d38fdd99
commit dffdd8b51f
2 changed files with 7 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
gdb/ChangeLog
View File

@ -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

2
gdb/target.c
View File

@ -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.