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/infrun.c | |
parent | d2854d8d5a82946ace7f5b626f19c2b73f86d1f6 (diff) | |
download | gdb-b161a60d1fe2a7383c7940815687c6100b97204e.zip gdb-b161a60d1fe2a7383c7940815687c6100b97204e.tar.gz gdb-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/infrun.c')
-rw-r--r-- | gdb/infrun.c | 78 |
1 files changed, 76 insertions, 2 deletions
diff --git a/gdb/infrun.c b/gdb/infrun.c index ca850f8..7be1fed 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -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 } |