gdb: pass target to thread_ptid_changed observable

I noticed what I think is a potential bug.  I did not observe it nor was
I able to reproduce it using actual debugging.  It's quite unlikely,
because it involves multi-target and ptid clashes.  I added selftests
that demonstrate it though.

The thread_ptid_changed observer says that thread with OLD_PTID now has
NEW_PTID.  Now, if for some reason we happen to have two targets
defining a thread with OLD_PTID, the observers don't know which thread
this is about.

regcache::regcache_thread_ptid_changed changes all regcaches with
OLD_PTID.  If there is a regcache for a thread with ptid OLD_PTID, but
that belongs to a different target, this regcache will be erroneously
changed.

Similarly, infrun_thread_ptid_changed updates inferior_ptid if
inferior_ptid matches OLD_PTID.  But if inferior_ptid currently refers
not to the thread is being changed, but to a thread with the same ptid
belonging to a different target, then inferior_ptid will erroneously be
changed.

This patch adds a `process_stratum_target *` parameter to the
`thread_ptid_changed` observable and makes the two observers use it.
Tests for both are added, which would fail if the corresponding fix
wasn't done.

gdb/ChangeLog:

	* observable.h (thread_ptid_changed): Add parameter
	`process_stratum_target *`.
	* infrun.c (infrun_thread_ptid_changed): Add parameter
	`process_stratum_target *` and use it.
	(selftests): New namespace.
	(infrun_thread_ptid_changed): New function.
	(_initialize_infrun): Register selftest.
	* regcache.c (regcache_thread_ptid_changed): Add parameter
	`process_stratum_target *` and use it.
	(regcache_thread_ptid_changed): New function.
	(_initialize_regcache): Register selftest.
	* thread.c (thread_change_ptid): Pass target to
	thread_ptid_changed observable.

Change-Id: I0599e61224b6d154a7b55088a894cb88298c3c71
This commit is contained in:
Simon Marchi
2020-08-07 10:59:33 -04:00
committed by Simon Marchi
parent d2854d8d5a
commit b161a60d1f
5 changed files with 167 additions and 7 deletions

View File

@@ -67,6 +67,9 @@
#include "gdbsupport/gdb_select.h"
#include <unordered_map>
#include "async-event.h"
#include "gdbsupport/selftest.h"
#include "scoped-mock-context.h"
#include "test-target.h"
/* Prototypes for local functions */
@@ -2068,9 +2071,11 @@ start_step_over (void)
/* Update global variables holding ptids to hold NEW_PTID if they were
holding OLD_PTID. */
static void
infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
infrun_thread_ptid_changed (process_stratum_target *target,
ptid_t old_ptid, ptid_t new_ptid)
{
if (inferior_ptid == old_ptid)
if (inferior_ptid == old_ptid
&& current_inferior ()->process_target () == target)
inferior_ptid = new_ptid;
}
@@ -9455,6 +9460,70 @@ infrun_async_inferior_event_handler (gdb_client_data data)
inferior_event_handler (INF_REG_EVENT);
}
namespace selftests
{
/* Verify that when two threads with the same ptid exist (from two different
targets) and one of them changes ptid, we only update inferior_ptid if
it is appropriate. */
static void
infrun_thread_ptid_changed ()
{
gdbarch *arch = current_inferior ()->gdbarch;
/* The thread which inferior_ptid represents changes ptid. */
{
scoped_restore_current_pspace_and_thread restore;
scoped_mock_context<test_target_ops> target1 (arch);
scoped_mock_context<test_target_ops> target2 (arch);
target2.mock_inferior.next = &target1.mock_inferior;
ptid_t old_ptid (111, 222);
ptid_t new_ptid (111, 333);
target1.mock_inferior.pid = old_ptid.pid ();
target1.mock_thread.ptid = old_ptid;
target2.mock_inferior.pid = old_ptid.pid ();
target2.mock_thread.ptid = old_ptid;
auto restore_inferior_ptid = make_scoped_restore (&inferior_ptid, old_ptid);
set_current_inferior (&target1.mock_inferior);
thread_change_ptid (&target1.mock_target, old_ptid, new_ptid);
gdb_assert (inferior_ptid == new_ptid);
}
/* A thread with the same ptid as inferior_ptid, but from another target,
changes ptid. */
{
scoped_restore_current_pspace_and_thread restore;
scoped_mock_context<test_target_ops> target1 (arch);
scoped_mock_context<test_target_ops> target2 (arch);
target2.mock_inferior.next = &target1.mock_inferior;
ptid_t old_ptid (111, 222);
ptid_t new_ptid (111, 333);
target1.mock_inferior.pid = old_ptid.pid ();
target1.mock_thread.ptid = old_ptid;
target2.mock_inferior.pid = old_ptid.pid ();
target2.mock_thread.ptid = old_ptid;
auto restore_inferior_ptid = make_scoped_restore (&inferior_ptid, old_ptid);
set_current_inferior (&target2.mock_inferior);
thread_change_ptid (&target1.mock_target, old_ptid, new_ptid);
gdb_assert (inferior_ptid == old_ptid);
}
}
} /* namespace selftests */
void _initialize_infrun ();
void
_initialize_infrun ()
@@ -9756,4 +9825,9 @@ or signalled."),
show_observer_mode,
&setlist,
&showlist);
#if GDB_SELF_TEST
selftests::register_test ("infrun_thread_ptid_changed",
selftests::infrun_thread_ptid_changed);
#endif
}