diff options
author | Simon Marchi <simon.marchi@efficios.com> | 2020-08-07 10:59:33 -0400 |
---|---|---|
committer | Simon Marchi <simon.marchi@polymtl.ca> | 2020-08-07 10:59:35 -0400 |
commit | b161a60d1fe2a7383c7940815687c6100b97204e (patch) | |
tree | f12e3423a7fe7902cd700b3594e3de8ebe09bf15 /gdb/regcache.c | |
parent | d2854d8d5a82946ace7f5b626f19c2b73f86d1f6 (diff) | |
download | binutils-b161a60d1fe2a7383c7940815687c6100b97204e.zip binutils-b161a60d1fe2a7383c7940815687c6100b97204e.tar.gz binutils-b161a60d1fe2a7383c7940815687c6100b97204e.tar.bz2 |
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
Diffstat (limited to 'gdb/regcache.c')
-rw-r--r-- | gdb/regcache.c | 72 |
1 files changed, 70 insertions, 2 deletions
diff --git a/gdb/regcache.c b/gdb/regcache.c index 3c460f2..71e528f 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -414,11 +414,12 @@ regcache_observer_target_changed (struct target_ops *target) /* Update regcaches related to OLD_PTID to now use NEW_PTID. */ static void -regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid) +regcache_thread_ptid_changed (process_stratum_target *target, + ptid_t old_ptid, ptid_t new_ptid) { for (auto ®cache : regcaches) { - if (regcache->ptid () == old_ptid) + if (regcache->ptid () == old_ptid && regcache->target () == target) regcache->set_ptid (new_ptid); } } @@ -1817,6 +1818,71 @@ cooked_write_test (struct gdbarch *gdbarch) } } +/* Verify that when two threads with the same ptid exist (from two different + targets) and one of them changes ptid, we only update the appropriate + regcaches. */ + +static void +regcache_thread_ptid_changed () +{ + /* This test relies on the global regcache list to initially be empty. */ + registers_changed (); + + /* Any arch will do. */ + gdbarch *arch = current_inferior ()->gdbarch; + + /* Prepare two targets with one thread each, with the same ptid. */ + 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; + + gdb_assert (regcaches.empty ()); + + /* Populate the regcaches container. */ + get_thread_arch_aspace_regcache (&target1.mock_target, old_ptid, arch, + nullptr); + get_thread_arch_aspace_regcache (&target2.mock_target, old_ptid, arch, + nullptr); + + /* Return whether a regcache for (TARGET, PTID) exists in REGCACHES. */ + auto regcache_exists = [] (process_stratum_target *target, ptid_t ptid) + { + for (regcache *rc : regcaches) + { + if (rc->target () == target && rc->ptid () == ptid) + return true; + } + + return false; + }; + + gdb_assert (regcaches_size () == 2); + gdb_assert (regcache_exists (&target1.mock_target, old_ptid)); + gdb_assert (!regcache_exists (&target1.mock_target, new_ptid)); + gdb_assert (regcache_exists (&target2.mock_target, old_ptid)); + gdb_assert (!regcache_exists (&target2.mock_target, new_ptid)); + + thread_change_ptid (&target1.mock_target, old_ptid, new_ptid); + + gdb_assert (regcaches_size () == 2); + gdb_assert (!regcache_exists (&target1.mock_target, old_ptid)); + gdb_assert (regcache_exists (&target1.mock_target, new_ptid)); + gdb_assert (regcache_exists (&target2.mock_target, old_ptid)); + gdb_assert (!regcache_exists (&target2.mock_target, new_ptid)); + + /* Leave the regcache list empty. */ + registers_changed (); + gdb_assert (regcaches.empty ()); +} + } // namespace selftests #endif /* GDB_SELF_TEST */ @@ -1840,5 +1906,7 @@ _initialize_regcache () selftests::cooked_read_test); selftests::register_test_foreach_arch ("regcache::cooked_write_test", selftests::cooked_write_test); + selftests::register_test ("regcache_thread_ptid_changed", + selftests::regcache_thread_ptid_changed); #endif } |