gdb: split regcaches management selftest

The selftest `regcaches` selftest is a bit too broad for my taste,
testing the behavior of get_thread_arch_aspace_regcache and various
cases of registers_changed_ptid.  Since I'll want to test even more
scenarios of registers_changed_ptid, passing different sets of
parameters, it will be difficult to do in a single test case.  It is
difficult to change something at some point in the test case while make
sure it doesn't compromise what comes after, that we still test the
scenarios that we intended to test.  So, split the test case in multiple
smaller ones.

- Split the test case in multiple, where each test case starts from
  scratch and tests one specific scenario.

- Introduce the populate_regcaches_for_test function, which is used by
  the various test cases to start with a regcache container populated
  with a few regcaches for two different targets.

- populate_regcaches_for_test returns a regcache_test_data object, which
  contains the test targets that were used to create the regcaches.  It
  also takes care to call registers_changed at the beginning and end of
  the test to ensure the test isn't influenced by existing regcaches,
  and cleans up after itself.

- Move the regcache_count lambda function out of
  regcache_thread_ptid_changed, so it can be used in
  other tests.

- For get_thread_arch_aspace_regcache, test that getting a regcache that
  already exists does not increase the count of existing regcaches.

- For registers_changed_ptid, test the three cases we handle today:
  (nullptr, minus_one_ptid), (target, minus_one_ptid) and (target,
  ptid).  The (target, minus_one_ptid) case was not tested prior to this
  patch.

gdb/ChangeLog:

	* regcache.c (regcache_count): New.
	(struct regcache_test_data): New.
	(regcache_test_data_up): New.
	(populate_regcaches_for_test): New.
	(regcaches_test): Remove.
	(get_thread_arch_aspace_regcache_test): New.
	(registers_changed_ptid_all_test): New.
	(registers_changed_ptid_target_test): New.
	(registers_changed_ptid_target_ptid_test): New.
	(regcache_thread_ptid_changed): Remove regcache_count lambda.
	(_initialize_regcache): Register new tests.

Change-Id: Id4280879fb40ff3aeae49b01b95359e1359c3d4b

2 files changed, 139 insertions, 60 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ef36c2f..330d7f3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2020-08-20  Simon Marchi  <simon.marchi@polymtl.ca>
 
+	* regcache.c (regcache_count): New.
+	(struct regcache_test_data): New.
+	(regcache_test_data_up): New.
+	(populate_regcaches_for_test): New.
+	(regcaches_test): Remove.
+	(get_thread_arch_aspace_regcache_test): New.
+	(registers_changed_ptid_all_test): New.
+	(registers_changed_ptid_target_test): New.
+	(registers_changed_ptid_target_ptid_test): New.
+	(regcache_thread_ptid_changed): Remove regcache_count lambda.
+	(_initialize_regcache): Register new tests.
+
+2020-08-20  Simon Marchi  <simon.marchi@polymtl.ca>
+
 	* regcache.c (test_get_thread_arch_aspace_regcache): Rename to...
 	(get_thread_arch_aspace_regcache_and_check): ... this.  Remove
 	gdbarch and aspace parameter.  Use current inferior's aspace.
diff --git a/gdb/regcache.c b/gdb/regcache.c
index c27f604..d9af453 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1488,6 +1488,23 @@ regcaches_size ()
   return size;
 }
 
+/* Return the count of regcaches for (TARGET, PTID) in REGCACHES.  */
+
+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 ())
+    {
+      auto &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;
+};
+
 /* Wrapper around get_thread_arch_aspace_regcache that does some self checks.  */
 
 static void
@@ -1509,60 +1526,115 @@ get_thread_arch_aspace_regcache_and_check (process_stratum_target *target,
   SELF_CHECK (regcache->aspace () == aspace);
 }
 
-static void
-regcaches_test ()
+/* The data that the regcaches selftests must hold onto for the duration of the
+   test.  */
+
+struct regcache_test_data
 {
-  /* Ensure the regcaches container is empty at the start.  */
-  registers_changed ();
-  SELF_CHECK (regcaches_size () == 0);
+  regcache_test_data ()
+  {
+    /* Ensure the regcaches container is empty at the start.  */
+    registers_changed ();
+  }
 
-  ptid_t ptid1 (1), ptid2 (2), ptid3 (3);
+  ~regcache_test_data ()
+  {
+    /* Make sure to leave the global regcaches container empty.  */
+    registers_changed ();
+  }
 
   test_target_ops test_target1;
   test_target_ops test_target2;
+};
+
+using regcache_test_data_up = std::unique_ptr<regcache_test_data>;
+
+/* Set up a few regcaches from two different targets, for use in
+   regcache-management tests.
+
+   Return a pointer, because the `regcache_test_data` type is not moveable.  */
+
+static regcache_test_data_up
+populate_regcaches_for_test ()
+{
+  regcache_test_data_up data (new regcache_test_data);
+  size_t expected_regcache_size = 0;
+
+  SELF_CHECK (regcaches_size () == 0);
+
+  /* Populate the regcache container with a few regcaches for the two test
+     targets. */
+  for (int pid : { 1, 2 })
+    {
+      for (long lwp : { 1, 2, 3 })
+	{
+	  get_thread_arch_aspace_regcache_and_check
+	    (&data->test_target1, ptid_t (pid, lwp));
+	  expected_regcache_size++;
+	  SELF_CHECK (regcaches_size () == expected_regcache_size);
+
+	  get_thread_arch_aspace_regcache_and_check
+	    (&data->test_target2, ptid_t (pid, lwp));
+	  expected_regcache_size++;
+	  SELF_CHECK (regcaches_size () == expected_regcache_size);
+	}
+    }
+
+  return data;
+}
+
+static void
+get_thread_arch_aspace_regcache_test ()
+{
+  /* populate_regcaches_for_test already tests most of the
+     get_thread_arch_aspace_regcache functionality.  */
+  regcache_test_data_up data = populate_regcaches_for_test ();
+  size_t regcaches_size_before = regcaches_size ();
+
+  /* Test that getting an existing regcache doesn't create a new one.  */
+  get_thread_arch_aspace_regcache_and_check (&data->test_target1, ptid_t (2, 2));
+  SELF_CHECK (regcaches_size () == regcaches_size_before);
+}
+
+  /* Test marking all regcaches of all targets as changed.  */
+
+static void
+registers_changed_ptid_all_test ()
+{
+  regcache_test_data_up data = populate_regcaches_for_test ();
 
-  /* Get regcache from (target1,ptid1), a new regcache is added to
-     REGCACHES.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target1, ptid1);
-  SELF_CHECK (regcaches_size () == 1);
-
-  /* Get regcache from (target1,ptid2), a new regcache is added to
-     REGCACHES.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target1, ptid2);
-  SELF_CHECK (regcaches_size () == 2);
-
-  /* Get regcache from (target1,ptid3), a new regcache is added to
-     REGCACHES.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target1, ptid3);
-  SELF_CHECK (regcaches_size () == 3);
-
-  /* Get regcache from (target1,ptid2) again, nothing is added to
-     REGCACHES.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target1, ptid2);
-  SELF_CHECK (regcaches_size () == 3);
-
-  /* Get regcache from (target2,ptid2), a new regcache is added to
-     REGCACHES, since this time we're using a different target.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target2, ptid2);
-  SELF_CHECK (regcaches_size () == 4);
-
-  /* Mark that (target1,ptid2) changed.  The regcache of (target1,
-     ptid2) should be removed from REGCACHES.  */
-  registers_changed_ptid (&test_target1, ptid2);
-  SELF_CHECK (regcaches_size () == 3);
-
-  /* Get the regcache from (target2,ptid2) again, confirming the
-     registers_changed_ptid call above did not delete it.  */
-  get_thread_arch_aspace_regcache_and_check (&test_target2, ptid2);
-  SELF_CHECK (regcaches_size () == 3);
-
-  /* Confirm that marking all regcaches of all targets as changed
-     clears REGCACHES.  */
   registers_changed_ptid (nullptr, minus_one_ptid);
   SELF_CHECK (regcaches_size () == 0);
+}
 
-  /* Make sure to leave the global regcaches container empty.  */
-  registers_changed ();
+/* Test marking regcaches of a specific target as changed.  */
+
+static void
+registers_changed_ptid_target_test ()
+{
+  regcache_test_data_up data = populate_regcaches_for_test ();
+
+  registers_changed_ptid (&data->test_target1, minus_one_ptid);
+  SELF_CHECK (regcaches_size () == 6);
+
+  /* Check that we deleted the regcache for the right target.  */
+  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
+registers_changed_ptid_target_ptid_test ()
+{
+  regcache_test_data_up data = populate_regcaches_for_test ();
+
+  registers_changed_ptid (&data->test_target1, ptid_t (2, 2));
+  SELF_CHECK (regcaches_size () == 11);
+
+  /* Check that we deleted the regcache for the right target.  */
+  SELF_CHECK (regcache_count (&data->test_target1, ptid_t (2, 2)) == 0);
+  SELF_CHECK (regcache_count (&data->test_target2, ptid_t (2, 2)) == 1);
 }
 
 class target_ops_no_register : public test_target_ops
@@ -1900,20 +1972,6 @@ regcache_thread_ptid_changed ()
   get_thread_arch_aspace_regcache (&target2.mock_target, old_ptid, arch,
 				   nullptr);
 
-  /* Return the count of regcaches for (TARGET, PTID) in REGCACHES.  */
-  auto regcache_count = [] (process_stratum_target *target, ptid_t ptid)
-    -> int
-    {
-      auto ptid_regc_map_it = regcaches.find (target);
-      if (ptid_regc_map_it != regcaches.end ())
-	{
-	  auto &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;
-    };
-
   gdb_assert (regcaches.size () == 2);
   gdb_assert (regcache_count (&target1.mock_target, old_ptid) == 1);
   gdb_assert (regcache_count (&target1.mock_target, new_ptid) == 0);
@@ -1950,7 +2008,14 @@ _initialize_regcache ()
 	   _("Force gdb to flush its register cache (maintainer command)."));
 
 #if GDB_SELF_TEST
-  selftests::register_test ("regcaches", selftests::regcaches_test);
+  selftests::register_test ("get_thread_arch_aspace_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_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);