[PATCH 3/6] gdb/amd-dbgapi: handle regno == -1 in fetch_registers and store_registers

[Simon Marchi <simon.marchi@efficios.com>]

Running "gcore" while a GPU wave is stopped at a breakpoint makes GDB
crash:

  $ gdb -q -nx testsuite/outputs/gdb.rocm/simple/simple \
      -ex "with breakpoint pending on -- break do_an_addition" \
      -ex r -ex gcore -batch
  ...
  regcache.c:162: internal-error: register_type: Assertion `regnum >= 0 && regnum < descr->nr_cooked_registers' failed.

The backtrace is:

    #0  internal_error_loc (file=0x55556355d500 "/home/smarchi/src/binutils-gdb/gdb/regcache.c", line=162, fmt=0x55556355cf60 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:53
    #1  register_type (gdbarch=0x51c000073880, regnum=-1) at /home/smarchi/src/binutils-gdb/gdb/regcache.c:162
    #2  amd_dbgapi_target::fetch_registers (this=0x555568df2760 <the_amd_dbgapi_target>, regcache=0x5070000c9420, regno=-1) at /home/smarchi/src/binutils-gdb/gdb/amd-dbgapi-target.c:2039
    #3  target_fetch_registers (regcache=0x5070000c9420, regno=-1) at /home/smarchi/src/binutils-gdb/gdb/target.c:3925
    #4  gcore_elf_build_thread_register_notes (gdbarch=0x51c000073880, info=0x51800008b480, stop_signal=GDB_SIGNAL_TRAP, obfd=0x5120001c0b40, note_data=0x7fffe698d420, note_size=0x7fffe68c0e30) at /home/smarchi/src/binutils-gdb/gdb/gcore-elf.c:132
    #5  linux_corefile_thread (info=0x51800008b480, gdbarch=0x51c000073880, obfd=0x5120001c0b40, note_data=std::unique_ptr<char> = {...}, note_size=0x7fffe68c0e30, stop_signal=GDB_SIGNAL_TRAP) at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:1957
    #6  linux_make_corefile_notes (gdbarch=0x51c00002d080, obfd=0x5120001c0b40, note_size=0x7fffe68c0e30) at /home/smarchi/src/binutils-gdb/gdb/linux-tdep.c:2543
    #7  gdbarch_make_corefile_notes (gdbarch=0x51c00002d080, obfd=0x5120001c0b40, note_size=0x7fffe68c0e30) at /home/smarchi/src/binutils-gdb/gdb/gdbarch-gen.c:3664
    #8  write_gcore_file_1 (obfd=0x5120001c0b40) at /home/smarchi/src/binutils-gdb/gdb/gcore.c:95
    #9  write_gcore_file (obfd=0x5120001c0b40) at /home/smarchi/src/binutils-gdb/gdb/gcore.c:137
    #10 gcore_command (args=0x0, from_tty=0) at /home/smarchi/src/binutils-gdb/gdb/gcore.c:175

gcore_elf_build_thread_register_notes calls target_fetch_registers with
regno == -1, meaning "fetch all registers".  The amd-dbgapi target
assumes regno is always a valid register number, and passes -1 directly
to register_type.  store_registers has the same problem.

Make both functions handle regno == -1 by iterating over all raw
registers.  This code is mostly taken from ROCgdb, except that it also
emits the warning when some registers fail to read or write in the
"regno == -1" cases.  ROCgdb only warns in the single register case.

Like ROCgdb, invalidate volatile registers once, after all registers are
written, so that writing a register with the "invalidate volatile"
property (e.g. VCC) does not invalidate a volatile register (e.g.
STATUS) that has yet to be written.

With this fix, the gcore command above gets a bit further, but now hits:

  gdbarch-gen.c:3637: internal-error: gdbarch_iterate_over_regset_sections: Assertion `gdbarch->iterate_over_regset_sections != nullptr' failed.

This is because linux_make_corefile_notes tries to dump GPU waves as if
they were host threads, using the amdgpu gdbarch, which does not
implement iterate_over_regset_sections.  This is addressed by a
following patch.

Change-Id: Ie669eb8a538ddfd3f35745e3af53eaf5541aa01b
---
 gdb/amd-dbgapi-target.c | 100 +++++++++++++++++++++++++---------------
 1 file changed, 64 insertions(+), 36 deletions(-)

diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index 2fd175de6f7d..099deceaabad 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -2035,20 +2035,32 @@ amd_dbgapi_target::fetch_registers (struct regcache *regcache, int regno)
 
   amdgpu_gdbarch_tdep *tdep = get_amdgpu_gdbarch_tdep (gdbarch);
   amd_dbgapi_wave_id_t wave_id = get_amd_dbgapi_wave_id (regcache->ptid ());
-  gdb_byte raw[AMDGPU_MAX_REGISTER_SIZE];
-  ULONGEST reg_size = register_type (gdbarch, regno)->length ();
-  gdb_assert (reg_size <= AMDGPU_MAX_REGISTER_SIZE);
 
-  amd_dbgapi_status_t status
-    = amd_dbgapi_read_register (wave_id, tdep->register_ids[regno], 0,
-				reg_size, raw);
+  auto fetch_one_register = [&] (int r)
+    {
+      gdb_byte raw[AMDGPU_MAX_REGISTER_SIZE];
+      ULONGEST reg_size = register_type (gdbarch, r)->length ();
+      gdb_assert (reg_size <= AMDGPU_MAX_REGISTER_SIZE);
 
-  if (status == AMD_DBGAPI_STATUS_SUCCESS)
-    regcache->raw_supply (regno, raw);
-  else if (status != AMD_DBGAPI_STATUS_ERROR_REGISTER_NOT_AVAILABLE)
-    warning (_("Couldn't read register %s (#%d) (%s)."),
-	     gdbarch_register_name (gdbarch, regno), regno,
-	     get_status_string (status));
+      amd_dbgapi_status_t status
+	= amd_dbgapi_read_register (wave_id, tdep->register_ids[r], 0,
+				    reg_size, raw);
+
+      if (status == AMD_DBGAPI_STATUS_SUCCESS)
+	regcache->raw_supply (r, raw);
+      else if (status != AMD_DBGAPI_STATUS_ERROR_REGISTER_NOT_AVAILABLE)
+	warning (_("Couldn't read register %s (#%d) (%s)."),
+		 gdbarch_register_name (gdbarch, r), r,
+		 get_status_string (status));
+    };
+
+  if (regno == -1)
+    {
+      for (int r = 0; r < gdbarch_num_regs (gdbarch); ++r)
+	fetch_one_register (r);
+    }
+  else
+    fetch_one_register (regno);
 }
 
 void
@@ -2063,38 +2075,54 @@ amd_dbgapi_target::store_registers (struct regcache *regcache, int regno)
   struct gdbarch *gdbarch = regcache->arch ();
   gdb_assert (is_amdgpu_arch (gdbarch));
 
-  gdb_byte raw[AMDGPU_MAX_REGISTER_SIZE];
-  ULONGEST reg_size = register_type (gdbarch, regno)->length ();
-  gdb_assert (reg_size <= AMDGPU_MAX_REGISTER_SIZE);
-  regcache->raw_collect (regno, &raw);
-
   amdgpu_gdbarch_tdep *tdep = get_amdgpu_gdbarch_tdep (gdbarch);
+  amd_dbgapi_wave_id_t wave_id = get_amd_dbgapi_wave_id (regcache->ptid ());
+  bool invalidate_volatile = false;
 
-  /* If the register has read-only bits, invalidate the value in the regcache
-     as the value actually written may differ.  */
-  if (tdep->register_properties[regno]
-      & AMD_DBGAPI_REGISTER_PROPERTY_READONLY_BITS)
-    regcache->invalidate (regno);
+  auto store_one_register = [&] (int r)
+    {
+      gdb_byte raw[AMDGPU_MAX_REGISTER_SIZE];
+      ULONGEST reg_size = register_type (gdbarch, r)->length ();
+      gdb_assert (reg_size <= AMDGPU_MAX_REGISTER_SIZE);
+      regcache->raw_collect (r, &raw);
 
-  /* Invalidate all volatile registers if this register has the invalidate
-     volatile property.  For example, writing to VCC may change the content
-     of STATUS.VCCZ.  */
-  if (tdep->register_properties[regno]
-      & AMD_DBGAPI_REGISTER_PROPERTY_INVALIDATE_VOLATILE)
+      /* If the register has read-only bits, invalidate the value in the
+	 regcache as the value actually written may differ.  */
+      if (tdep->register_properties[r]
+	  & AMD_DBGAPI_REGISTER_PROPERTY_READONLY_BITS)
+	regcache->invalidate (r);
+
+      /* Writing this register may change the value of volatile registers.
+	 For example, writing to VCC may change the content of STATUS.VCCZ.
+	 Invalidate them once all registers are written, so that we don't
+	 invalidate a register we have yet to write.  */
+      if (tdep->register_properties[r]
+	  & AMD_DBGAPI_REGISTER_PROPERTY_INVALIDATE_VOLATILE)
+	invalidate_volatile = true;
+
+      amd_dbgapi_status_t status
+	= amd_dbgapi_write_register (wave_id, tdep->register_ids[r], 0,
+				     reg_size, raw);
+
+      if (status != AMD_DBGAPI_STATUS_SUCCESS)
+	warning (_("Couldn't write register %s (#%d)."),
+		 gdbarch_register_name (gdbarch, r), r);
+    };
+
+  if (regno == -1)
+    {
+      for (int r = 0; r < gdbarch_num_regs (gdbarch); ++r)
+	store_one_register (r);
+    }
+  else
+    store_one_register (regno);
+
+  if (invalidate_volatile)
     {
       for (size_t r = 0; r < tdep->register_properties.size (); ++r)
 	if (tdep->register_properties[r] & AMD_DBGAPI_REGISTER_PROPERTY_VOLATILE)
 	  regcache->invalidate (r);
     }
-
-  amd_dbgapi_wave_id_t wave_id = get_amd_dbgapi_wave_id (regcache->ptid ());
-  amd_dbgapi_status_t status
-    = amd_dbgapi_write_register (wave_id, tdep->register_ids[regno], 0,
-				 reg_size, raw);
-
-  if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    warning (_("Couldn't write register %s (#%d)."),
-	     gdbarch_register_name (gdbarch, regno), regno);
 }
 
 struct gdbarch *
-- 
2.56.0