diff mbox

gdb/riscv: Remove redundant code, and catch more errors when accessing MISA

Message ID 20181023112631.9722-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Oct. 23, 2018, 11:26 a.m. UTC 
  This is a cleanup of a patch I proposed here:

   https://sourceware.org/ml/gdb-patches/2018-09/msg00749.html

I think Jim said he was happy with this, but this is a final chance to
review.

Thanks,
Andrew

---

When reading the MISA register, the RISC-V specification says that, if
MISA can't be found then a default value of 0 should be assumed.

As such, this patch ensures that GDB ignores errors when accessing
both the new and old locations for the MISA register.

Additionally, this patch removes an unneeded flag parameter which
didn't provide any additional functionality beyond checking the MISA
for the default value of 0.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_read_misa_reg): Update comment, remove
	READ_P parameter, catch and ignore register access errors from
	either the old or new MISA location.
	(riscv_has_feature): Update call to riscv_read_misa_reg.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/riscv-tdep.c | 46 ++++++++++++++++++++++++----------------------
 2 files changed, 31 insertions(+), 22 deletions(-)

Comments

Jim Wilson Oct. 23, 2018, 4:30 p.m. UTC | #1 
On Tue, Oct 23, 2018 at 4:26 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> This is a cleanup of a patch I proposed here:
>    https://sourceware.org/ml/gdb-patches/2018-09/msg00749.html
> I think Jim said he was happy with this, but this is a final chance to
> review.

Yes, I'm OK with this.  Now that we check the target instruction to
determine breakpoint size, misa isn't a critical register anymore.  We
still need a way to determine FP register size, but the linux kernel
only supports 64-bit FP registers or no FP registers, so the linux
target support doesn't need to worry about 32-bit FP registers yet.
When we do, we may be able to use the auxvec/hwcap info instead of
misa for that.

Jim
John Baldwin Oct. 23, 2018, 4:31 p.m. UTC | #2 
On 10/23/18 4:26 AM, Andrew Burgess wrote:
> This is a cleanup of a patch I proposed here:
> 
>    https://sourceware.org/ml/gdb-patches/2018-09/msg00749.html
> 
> I think Jim said he was happy with this, but this is a final chance to
> review.
> 
> Thanks,
> Andrew

Looks good to me.
diff mbox

Patch

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 48ca2accb4a..a3c3e833e93 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -290,34 +290,39 @@  static unsigned int riscv_debug_infcall = 0;
 
 static unsigned int riscv_debug_unwinder = 0;
 
-/* Read the MISA register from the target.  The register will only be read
-   once, and the value read will be cached.  If the register can't be read
-   from the target then a default value (0) will be returned.  If the
-   pointer READ_P is not null, then the bool pointed to is updated  to
-   indicate if the value returned was read from the target (true) or is the
-   default (false).  */
+/* Read the MISA register from the target.  There are a couple of locations
+   that the register might be found, these are all tried.  If the MISA
+   register can't be found at all then the default value of 0 is returned,
+   this is inline with the RISC-V specification.  */
 
 static uint32_t
-riscv_read_misa_reg (bool *read_p)
+riscv_read_misa_reg ()
 {
   uint32_t value = 0;
 
   if (target_has_registers)
     {
+      /* Old cores might have MISA located at a different offset.  */
+      static int misa_regs[] =
+	{ RISCV_CSR_MISA_REGNUM, RISCV_CSR_LEGACY_MISA_REGNUM };
+
       struct frame_info *frame = get_current_frame ();
 
-      TRY
-	{
-	  value = get_frame_register_unsigned (frame,
-					       RISCV_CSR_MISA_REGNUM);
-	}
-      CATCH (ex, RETURN_MASK_ERROR)
+      for (int i = 0; i < ARRAY_SIZE (misa_regs); ++i)
 	{
-	  /* Old cores might have MISA located at a different offset.  */
-	  value = get_frame_register_unsigned (frame,
-					       RISCV_CSR_LEGACY_MISA_REGNUM);
+	  TRY
+	    {
+	      value = get_frame_register_unsigned (frame, misa_regs[i]);
+	      break;
+	    }
+	  CATCH (ex, RETURN_MASK_ERROR)
+	    {
+	      /* Ignore errors, it is acceptable for a target to not
+		 provide a MISA register, in which case the default value
+		 of 0 should be used.  */
+	    }
+	  END_CATCH
 	}
-      END_CATCH
     }
 
   return value;
@@ -330,13 +335,10 @@  riscv_read_misa_reg (bool *read_p)
 static bool
 riscv_has_feature (struct gdbarch *gdbarch, char feature)
 {
-  bool have_read_misa = false;
-  uint32_t misa;
-
   gdb_assert (feature >= 'A' && feature <= 'Z');
 
-  misa = riscv_read_misa_reg (&have_read_misa);
-  if (!have_read_misa || misa == 0)
+  uint32_t misa = riscv_read_misa_reg ();
+  if (misa == 0)
     misa = gdbarch_tdep (gdbarch)->core_features;
 
   return (misa & (1 << (feature - 'A'))) != 0;