FreeChainXenon/binutils-gdb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

gdb: handle the ptid.is_pid () case in registers_changed_ptid

Browse source 
As reported by Tom here [1], commit 888bdb2b74 ("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
This commit is contained in:
Simon Marchi 2020-08-20 10:11:09 -04:00
parent cdd9148a19
commit b70e516e89
2 changed files with 120 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

132
gdb/regcache.c
View file

@ -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);
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 ())
return;
auto &ptid_regc_map = ptid_regc_map_it->second;
/* 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;
/* 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);