public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: gdb-patches@sourceware.org
Cc: Nelson Chu <nelson.chu@sifive.com>, Jim Wilson <jimw@sifive.com>,
	Tom Tromey <tom@tromey.com>,
	palmer@dabbelt.com, Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCH 6/8] gdb: Extend target description processing of unknown registers
Date: Tue, 16 Jun 2020 18:14:45 +0100	[thread overview]
Message-ID: <d2860182feb4b9887831ea3584efc25c06072ae4.1592327296.git.andrew.burgess@embecosm.com> (raw)
In-Reply-To: <cover.1592327296.git.andrew.burgess@embecosm.com>

This commit adds a new step to the processing of a target description
done in tdesc_use_registers, this new step is about how unknown
registers are processed.

Currently an architecture looks through the target description and
calls tdesc_numbered_register for each register is was expecting (or
hoping) to find.  This builds up a map from GDB's register numbers to
the tdesc_reg object.  Later the architecture calls
tdesc_use_registers.

In tdesc_use_registers we build a hash with keys being all the
tdesc_reg object pointers, from this hash we remove all of the
tdesc_reg objects that were assigned register numbers using
tdesc_numbered_register.

Finally we walk through all of the tdesc_reg objects, and if it was
not already assigned a number we assign that register the next
available number.

The problem with this is that the architecture has no visibility of
which unknown registers exist, and which tdesc_feature the register
came from, in some cases this might be important.

For example, on RISC-V GDB overrides the use of
tdesc_register_reggroup_p, with riscv_register_reggroup_p to modify
some of the register group choices.  In this function GDB wants to
treat all registers from a particular feature in a certain way.  This
is fine for registers that GDB knows might be in that feature, but for
unknown registers the RISC-V parts of GDB have no easy way to figure
out which unknown registers exist, and what numbers they were
assigned.

We could figure this information out by probing the register
structures after calling tdesc_use_registers, but this would be
horrible, much better to have tdesc_use_registers tell the
architecture about unknown registers.

This is what this commit does.  A new phase of tdesc_use_registers,
just before the unknown registers are assigned a number, we loop over
each tdesc_reg object, if it has not been assigned a number then we
figure out what number would be assigned and then call back into the
architecture passing the tdesc_feature, register name, and the
proposed register number.

The architecture is free to return the proposed register number, or it
can return a different number (which has a result identical to having
called tdesc_numbered_register).  Alternatively the architecture can
return -1 to indicate the register should be numbered later.

After calling the callback for every tdesc_reg object any registers
still don't have a number assigned (because the architecture returned
-1), then a new register number is assigned, which might be different
from the proposed number that was suggested earlier.

This commit adds the general target-description parts of this
mechanism.  No targets are currently using this code.  The RISC-V
target will make use of this in the next commit.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_use_registers): Add new parameter a
	callback, use the callback (when not null) to help number unknown
	registers.
	* target-descriptions.h (tdesc_unknown_register_ftype): New typedef.
	(tdesc_use_registers): Add extra parameter to declaration.
---
 gdb/ChangeLog             |  8 ++++++++
 gdb/target-descriptions.c | 31 ++++++++++++++++++++++++++++++-
 gdb/target-descriptions.h | 27 ++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 20a3a640f4f..9a661c8487b 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1056,7 +1056,8 @@ set_tdesc_pseudo_register_reggroup_p
 void
 tdesc_use_registers (struct gdbarch *gdbarch,
 		     const struct target_desc *target_desc,
-		     struct tdesc_arch_data *early_data)
+		     struct tdesc_arch_data *early_data,
+		     tdesc_unknown_register_ftype unk_reg_cb)
 {
   int num_regs = gdbarch_num_regs (gdbarch);
   struct tdesc_arch_data *data;
@@ -1105,6 +1106,34 @@ tdesc_use_registers (struct gdbarch *gdbarch,
   while (data->arch_regs.size () < num_regs)
     data->arch_regs.emplace_back (nullptr, nullptr);
 
+  /* First we give the target a chance to number previously unknown
+     registers.  This allows targets to record the numbers assigned based
+     on which feature the register was from.  */
+  if (unk_reg_cb != NULL)
+    {
+      for (const tdesc_feature_up &feature : target_desc->features)
+	for (const tdesc_reg_up &reg : feature->registers)
+	  if (htab_find (reg_hash, reg.get ()) != NULL)
+	    {
+	      int regno = unk_reg_cb (gdbarch, feature.get (),
+				      reg->name.c_str (), num_regs);
+	      gdb_assert (regno == -1 || regno >= num_regs);
+	      if (regno != -1)
+		{
+		  while (regno >= data->arch_regs.size ())
+		    data->arch_regs.emplace_back (nullptr, nullptr);
+		  data->arch_regs[regno] = tdesc_arch_reg (reg.get (), NULL);
+		  num_regs = regno + 1;
+		  htab_remove_elt (reg_hash, reg.get ());
+		}
+	    }
+    }
+
+  /* Ensure the array was sized correctly above.  */
+  gdb_assert (data->arch_regs.size () == num_regs);
+
+  /* Now in a final pass we assign register numbers to any remaining
+     unnumbered registers.  */
   for (const tdesc_feature_up &feature : target_desc->features)
     for (const tdesc_reg_up &reg : feature->registers)
       if (htab_find (reg_hash, reg.get ()) != NULL)
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 96d283fb379..6d842bf07ed 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -80,6 +80,30 @@ void set_tdesc_pseudo_register_reggroup_p
   (struct gdbarch *gdbarch,
    gdbarch_register_reggroup_p_ftype *pseudo_reggroup_p);
 
+/* Pointer to a function that should be called for each unknown register in
+   a target description, used by TDESC_USE_REGISTERS.
+
+   GDBARCH is the architecture the target description is for, FEATURE is
+   the feature the unknown register is in, and REG_NAME is the name of the
+   register from the target description.  The POSSIBLE_REGNUM is a proposed
+   (GDB internal) number for this register.
+
+   The callback function can return, (-1) to indicate that the register
+   should not be assigned POSSIBLE_REGNUM now (though it might be later),
+   GDB will number the register automatically later on.  Return
+   POSSIBLE_REGNUM (or greater) to have this register assigned that number.
+   Returning a value less that POSSIBLE_REGNUM is also acceptable, but take
+   care not to clash with a register number that has already been
+   assigned.
+
+   The callback will always be called on the registers in the order they
+   appear in the target description.  This means all unknown registers
+   within a single feature will be called one after another.  */
+
+typedef int (*tdesc_unknown_register_ftype)
+	(struct gdbarch *gdbarch, tdesc_feature *feature,
+	 const char *reg_name, int possible_regnum);
+
 /* Update GDBARCH to use the TARGET_DESC for registers.  TARGET_DESC
    may be GDBARCH's target description or (if GDBARCH does not have
    one which describes registers) another target description
@@ -95,7 +119,8 @@ void set_tdesc_pseudo_register_reggroup_p
 
 void tdesc_use_registers (struct gdbarch *gdbarch,
 			  const struct target_desc *target_desc,
-			  struct tdesc_arch_data *early_data);
+			  struct tdesc_arch_data *early_data,
+			  tdesc_unknown_register_ftype unk_reg_cb = NULL);
 
 /* Allocate initial data for validation of a target description during
    gdbarch initialization.  */
-- 
2.25.4


  parent reply	other threads:[~2020-06-16 17:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16 17:14 [PATCH 0/8] RISC-V target description and register handling fixes Andrew Burgess
2020-06-16 17:14 ` [PATCH 1/8] gdb/riscv: Improved register alias name creation Andrew Burgess
2020-06-18 20:36   ` Tom Tromey
2020-06-16 17:14 ` [PATCH 2/8] gdb/riscv: Fix whitespace error Andrew Burgess
2020-06-16 17:14 ` [PATCH 3/8] gdb/riscv: Take CSR names from target description Andrew Burgess
2020-06-16 17:14 ` [PATCH 4/8] gdb/riscv: Remove CSR feature file Andrew Burgess
2020-06-16 17:14 ` [PATCH 5/8] gdb/riscv: Improve support for matching against target descriptions Andrew Burgess
2020-06-16 17:14 ` Andrew Burgess [this message]
2020-06-16 17:14 ` [PATCH 7/8] gdb/riscv: Record information about unknown tdesc registers Andrew Burgess
2020-06-16 17:14 ` [PATCH 8/8] gdb/riscv: Loop over all registers for 'info all-registers' Andrew Burgess
2020-06-17  1:31 ` [PATCH 0/8] RISC-V target description and register handling fixes Nelson Chu
2020-06-18 20:45 ` Tom Tromey
2020-06-18 20:54   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2860182feb4b9887831ea3584efc25c06072ae4.1592327296.git.andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jimw@sifive.com \
    --cc=nelson.chu@sifive.com \
    --cc=palmer@dabbelt.com \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).