public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PUSHED] gdb/nds32: update features/nds32.c
@ 2022-08-14 13:54 Andrew Burgess
  2022-08-14 13:55 ` [PUSHED] gdb/riscv: improve a comment about fcsr, fflags, and frm registers Andrew Burgess
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2022-08-14 13:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

After this commit:

  commit 7b7c365c5c663ffdfb2b3f696db35c23cdccd921
  Date:   Wed Sep 15 10:10:46 2021 +0200

      [bfd] Ensure unique printable names for bfd archs

The printable name field of the default nds32 bfd_arch_info changed
from 'n1h' to 'n1'.  As a consequence the generated feature file
within GDB should have been recreated.  Recreate it now.
---
 gdb/features/nds32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/features/nds32.c b/gdb/features/nds32.c
index ed535b5ab9f..4e520163931 100644
--- a/gdb/features/nds32.c
+++ b/gdb/features/nds32.c
@@ -10,7 +10,7 @@ static void
 initialize_tdesc_nds32 (void)
 {
   target_desc_up result = allocate_target_description ();
-  set_tdesc_architecture (result.get (), bfd_scan_arch ("n1h"));
+  set_tdesc_architecture (result.get (), bfd_scan_arch ("n1"));
 
   struct tdesc_feature *feature;
 
-- 
2.25.4


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PUSHED] gdb/riscv: improve a comment about fcsr, fflags, and frm registers
  2022-08-14 13:54 [PUSHED] gdb/nds32: update features/nds32.c Andrew Burgess
@ 2022-08-14 13:55 ` Andrew Burgess
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2022-08-14 13:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

There's a comment in riscv-tdep.c that explains some of the background
about how we check for the fcsr, fflags, and frm registers within a
riscv target description.

This comment (and the functionality it describes) relates to how QEMU
advertises these registers within its target description.

Unfortunately, QEMU includes these three registers in both the fpu and
crs target description features.  To work around this GDB uses one of
the register declarations, and ignores the other, this means the GDB
user sees a single copy of each register, and things just work.

When I originally wrote the comment I thought it didn't matter which
copy of the register GDB selected, the fpu copy or the csr copy, so
long as we just used one of them.  The comment reflected this belief.

Upon further investigation, it turns out I was wrong.  GDB has to use
the csr copy of the register.  If GDB tries to use the register from
the fpu feature then QEMU will return an error when GDB tries to read
or write the register.

Luckily, the code within GDB (currently) will always select the csr
copy of the register, so nothing is broken, but the comment is wrong.
This commit updates the comment to better describe what is actually
going on.

Of course, I should probably also send a patch to QEMU to fix up the
target description that is sent to GDB.
---
 gdb/riscv-tdep.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index b9a51f7ae6a..9ec430d8a10 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -3591,22 +3591,25 @@ riscv_tdesc_unknown_reg (struct gdbarch *gdbarch, tdesc_feature *feature,
      and CSR register sets.
 
      Some targets (QEMU) copied these target descriptions into their source
-     tree, and so we're currently stuck working with some targets that
+     tree, and so we're now stuck working with some versions of QEMU that
      declare the same registers twice.
 
-     There's not much we can do about this any more.  Assuming the target
-     will direct a request for either register number to the correct
-     underlying hardware register then it doesn't matter which one GDB
-     uses, so long as we (GDB) are consistent (so that we don't end up with
-     invalid cache misses).
-
-     As we always scan the FPU registers first, then the CSRs, if the
-     target has included the offending registers in both sets then we will
-     always see the FPU copies here, as the CSR versions will replace them
-     in the register list.
-
-     To prevent these duplicates showing up in any of the register list,
-     record their register numbers here.  */
+     To make matters worse, if GDB tries to read or write to these
+     registers using the register number assigned in the FPU feature set,
+     then QEMU will fail to read the register, so we must use the register
+     number declared in the CSR feature set.
+
+     Luckily, GDB scans the FPU feature first, and then the CSR feature,
+     which means that the CSR feature will be the one we end up using, the
+     versions of these registers in the FPU feature will appear as unknown
+     registers and will be passed through to this code.
+
+     To prevent these duplicate registers showing up in any of the register
+     lists, and to prevent GDB every trying to access the FPU feature copies,
+     we spot the three problematic registers here, and record the register
+     number that GDB has assigned them.  Then in riscv_register_name we will
+     return no name for the three duplicates, this hides the duplicates from
+     the user.  */
   if (strcmp (tdesc_feature_name (feature), riscv_freg_feature.name ()) == 0)
     {
       riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
-- 
2.25.4


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-08-14 13:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-14 13:54 [PUSHED] gdb/nds32: update features/nds32.c Andrew Burgess
2022-08-14 13:55 ` [PUSHED] gdb/riscv: improve a comment about fcsr, fflags, and frm registers Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).