[PATCH 4/4] gdb: handle the `ptid.is_pid ()` case in registers_changed_ptid
Simon Marchi
simon.marchi@efficios.com
Tue Aug 18 20:59:26 GMT 2020
From: Simon Marchi <simon.marchi@polymtl.ca>
As reported by Tom here [1], commit 888bdb2b7445 ("gdb: change regcache
list to be a map") overlooked an important case, causing a regression.
When registers_changed_ptid is called with a pid-like ptid, it used to
clear all the regcaches for that pid. That commit accidentally removed
that behavior. We need to handle the `ptid.is_pid ()` case in
registers_changed_ptid.
The most trivial way of fixing it would be to iterate on all ptids of a
target and delete the entries where the ptid match the pid. But the
point of that commit was to avoid having to iterate on ptids to
invalidate regcaches, so that would feel like a step backwards.
The only logical solution I see is to add yet another map level, so that
we now have:
target -> (pid -> (ptid -> regcaches))
This patch implements that and adds a test for the case of calling
registers_changed_ptid with a pid-like ptid.
[1] https://sourceware.org/pipermail/gdb-patches/2020-August/171222.html
gdb/ChangeLog:
* regcache.c (pid_ptid_regcache_map): New type.
(target_ptid_regcache_map): Remove.
(target_pid_ptid_regcache_map): New type.
(regcaches): Change type to target_pid_ptid_regcache_map.
(get_thread_arch_aspace_regcache): Update.
(regcache_thread_ptid_changed): Update, handle pid-like ptid
case.
(regcaches_size): Update.
(regcache_count): Update.
(registers_changed_ptid_target_pid_test): New.
(_initialize_regcache): Register new test.
Change-Id: I4c46e26d8225c177dbac9488b549eff4c68fa0d8
---
gdb/regcache.c | 130 +++++++++++++++++++++++++++++++++++++++----------
1 file changed, 105 insertions(+), 25 deletions(-)
diff --git a/gdb/regcache.c b/gdb/regcache.c
index d9af4539ab9..91d3202b94b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -319,11 +319,14 @@ reg_buffer::assert_regnum (int regnum) const
using ptid_regcache_map
= std::unordered_multimap<ptid_t, regcache_up, hash_ptid>;
-/* Type to map a target to a ptid_regcache_map, holding the regcaches for the
- threads defined by that target. */
+/* Type holding regcaches for a given pid. */
-using target_ptid_regcache_map
- = std::unordered_map<process_stratum_target *, ptid_regcache_map>;
+using pid_ptid_regcache_map = std::unordered_map<int, ptid_regcache_map>;
+
+/* Type holding regcaches for a given target. */
+
+using target_pid_ptid_regcache_map
+ = std::unordered_map<process_stratum_target *, pid_ptid_regcache_map>;
/* Global structure containing the existing regcaches. */
@@ -331,7 +334,7 @@ using target_ptid_regcache_map
recording if the register values have been changed (eg. by the
user). Therefore all registers must be written back to the
target when appropriate. */
-static target_ptid_regcache_map regcaches;
+static target_pid_ptid_regcache_map regcaches;
struct regcache *
get_thread_arch_aspace_regcache (process_stratum_target *target,
@@ -340,8 +343,11 @@ get_thread_arch_aspace_regcache (process_stratum_target *target,
{
gdb_assert (target != nullptr);
- /* Find the ptid -> regcache map for this target. */
- auto &ptid_regc_map = regcaches[target];
+ /* Find the map for this target. */
+ pid_ptid_regcache_map &pid_ptid_regc_map = regcaches[target];
+
+ /* Find the map for this pid. */
+ ptid_regcache_map &ptid_regc_map = pid_ptid_regc_map[ptid.pid ()];
/* Check first if a regcache for this arch already exists. */
auto range = ptid_regc_map.equal_range (ptid);
@@ -436,12 +442,19 @@ static void
regcache_thread_ptid_changed (process_stratum_target *target,
ptid_t old_ptid, ptid_t new_ptid)
{
- auto ptid_regc_map_it = regcaches.find (target);
+ /* Look up map for target. */
+ auto pid_ptid_regc_map_it = regcaches.find (target);
+ if (pid_ptid_regc_map_it == regcaches.end ())
+ return;
- if (ptid_regc_map_it == regcaches.end ())
+ /* Look up map for pid. */
+ pid_ptid_regcache_map &pid_ptid_regc_map = pid_ptid_regc_map_it->second;
+ auto ptid_regc_map_it = pid_ptid_regc_map.find (old_ptid.pid ());
+ if (ptid_regc_map_it == pid_ptid_regc_map.end ())
return;
- auto &ptid_regc_map = ptid_regc_map_it->second;
+ /* Update all regcaches belonging to old_ptid. */
+ ptid_regcache_map &ptid_regc_map = ptid_regc_map_it->second;
auto range = ptid_regc_map.equal_range (old_ptid);
for (auto it = range.first; it != range.second;)
{
@@ -478,15 +491,43 @@ registers_changed_ptid (process_stratum_target *target, ptid_t ptid)
/* Delete all the regcaches of all targets. */
regcaches.clear ();
}
+ else if (ptid.is_pid ())
+ {
+ /* Non-NULL target and pid ptid, delete all regcaches belonging
+ to this (TARGET, PID). */
+
+ /* Look up map for target. */
+ auto pid_ptid_regc_map_it = regcaches.find (target);
+ if (pid_ptid_regc_map_it != regcaches.end ())
+ {
+ pid_ptid_regcache_map &pid_ptid_regc_map
+ = pid_ptid_regc_map_it->second;
+
+ pid_ptid_regc_map.erase (ptid.pid ());
+ }
+ }
else if (ptid != minus_one_ptid)
{
/* Non-NULL target and non-minus_one_ptid, delete all regcaches belonging
- to this (TARGET, PTID). */
- auto ptid_regc_map_it = regcaches.find (target);
- if (ptid_regc_map_it != regcaches.end ())
+ to this (TARGET, PTID). */
+
+ /* Look up map for target. */
+ auto pid_ptid_regc_map_it = regcaches.find (target);
+ if (pid_ptid_regc_map_it != regcaches.end ())
{
- auto &ptid_regc_map = ptid_regc_map_it->second;
- ptid_regc_map.erase (ptid);
+ pid_ptid_regcache_map &pid_ptid_regc_map
+ = pid_ptid_regc_map_it->second;
+
+ /* Look up map for pid. */
+ auto ptid_regc_map_it
+ = pid_ptid_regc_map.find (ptid.pid ());
+ if (ptid_regc_map_it != pid_ptid_regc_map.end ())
+ {
+ ptid_regcache_map &ptid_regc_map
+ = ptid_regc_map_it->second;
+
+ ptid_regc_map.erase (ptid);
+ }
}
}
else
@@ -1479,10 +1520,23 @@ static size_t
regcaches_size ()
{
size_t size = 0;
- for (auto it = regcaches.begin (); it != regcaches.end (); ++it)
+
+ for (auto pid_ptid_regc_map_it = regcaches.cbegin ();
+ pid_ptid_regc_map_it != regcaches.cend ();
+ ++pid_ptid_regc_map_it)
{
- auto &ptid_regc_map = it->second;
- size += ptid_regc_map.size ();
+ const pid_ptid_regcache_map &pid_ptid_regc_map
+ = pid_ptid_regc_map_it->second;
+
+ for (auto ptid_regc_map_it = pid_ptid_regc_map.cbegin ();
+ ptid_regc_map_it != pid_ptid_regc_map.cend ();
+ ++ptid_regc_map_it)
+ {
+ const ptid_regcache_map &ptid_regc_map
+ = ptid_regc_map_it->second;
+
+ size += ptid_regc_map.size ();
+ }
}
return size;
@@ -1493,13 +1547,21 @@ regcaches_size ()
static int
regcache_count (process_stratum_target *target, ptid_t ptid)
{
- auto ptid_regc_map_it = regcaches.find (target);
- if (ptid_regc_map_it != regcaches.end ())
+ /* Look up map for target. */
+ auto pid_ptid_regc_map_it = regcaches.find (target);
+ if (pid_ptid_regc_map_it != regcaches.end ())
{
- auto &ptid_regc_map = ptid_regc_map_it->second;
- auto range = ptid_regc_map.equal_range (ptid);
+ pid_ptid_regcache_map &pid_ptid_regc_map = pid_ptid_regc_map_it->second;
- return std::distance (range.first, range.second);
+ /* Look map for pid. */
+ auto ptid_regc_map_it = pid_ptid_regc_map.find (ptid.pid ());
+ if (ptid_regc_map_it != pid_ptid_regc_map.end ())
+ {
+ ptid_regcache_map &ptid_regc_map = ptid_regc_map_it->second;
+ auto range = ptid_regc_map.equal_range (ptid);
+
+ return std::distance (range.first, range.second);
+ }
}
return 0;
@@ -1622,6 +1684,22 @@ registers_changed_ptid_target_test ()
SELF_CHECK (regcache_count (&data->test_target2, ptid_t (2, 2)) == 1);
}
+/* Test marking regcaches of a specific (target, pid) as changed. */
+
+static void
+registers_changed_ptid_target_pid_test ()
+{
+ regcache_test_data_up data = populate_regcaches_for_test ();
+
+ registers_changed_ptid (&data->test_target1, ptid_t (2));
+ SELF_CHECK (regcaches_size () == 9);
+
+ /* Regcaches from target1 should not exist, while regcaches from target2
+ should exist. */
+ SELF_CHECK (regcache_count (&data->test_target1, ptid_t (2, 2)) == 0);
+ SELF_CHECK (regcache_count (&data->test_target2, ptid_t (2, 2)) == 1);
+}
+
/* Test marking regcaches of a specific (target, ptid) as changed. */
static void
@@ -2012,10 +2090,12 @@ _initialize_regcache ()
selftests::get_thread_arch_aspace_regcache_test);
selftests::register_test ("registers_changed_ptid_all",
selftests::registers_changed_ptid_all_test);
+ selftests::register_test ("registers_changed_ptid_target",
+ selftests::registers_changed_ptid_target_test);
+ selftests::register_test ("registers_changed_ptid_target_pid",
+ selftests::registers_changed_ptid_target_pid_test);
selftests::register_test ("registers_changed_ptid_target_ptid",
selftests::registers_changed_ptid_target_ptid_test);
- selftests::register_test ("registers_changed_ptid_target",
- selftests::registers_changed_ptid_target_test);
selftests::register_test_foreach_arch ("regcache::cooked_read_test",
selftests::cooked_read_test);
--
2.28.0
More information about the Gdb-patches
mailing list