This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] gdb/regcache: When saving, ignore registers that can't be read


If during a call to reg_buffer::save GDB encounters an error trying to
read a register then this should not cause GDB to crash, nor should it
force the save to quit.  Instead, GDB should just treat the register
as unavailable and push on.

The specific example I encountered was a RISC-V remote target that
claimed in its target description to have floating point register
support.  However, this was not true, when GDB tried to read a
floating point register the remote sent back an error.

Mostly this was fine, the program I was testing were integer only,
however, when trying to make an inferior call, GDB would try to
preserve the current values of the floating point registers, this
result in a read of a register that threw an error, and GDB would
crash like this:

    (gdb) call some_inferior_function ()
    ../../src/gdb/regcache.c:310: internal-error: void regcache::restore(readonly_detached_regcache*): Assertion `src != NULL' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

I acknowledge that the target description sent back in this case is
wrong, and the target should be fixed.  However, I think that GDB
should, at a minimum, not crash and burn in this case, and better, I
think GDB can probably just push on, ignoring the registers that can't
be read.

The solution I propose in this patch is to catch errors in
reg_buffer::save while calling cooked_read, and register that throws
an error should be considered unavailable.  GDB will not try to
restore these registers after the inferior call.

What I haven't done in this commit is provide any user feedback that
GDB would like to backup a particular register, but can't.  Right now
I figure that if the user cares about this they would probably try 'p
$reg_name' themselves, at which point it becomes obvious that the
register can't be read.  That said, I'm open to adding a warning that
the regiter failed to save if that is thought important.

I've tested this using on X86-64/Linux native, and for
native-gdbserver with no regressions.  Against my miss-behaving target
I can now make inferior calls without any problems.

gdb/ChangeLog:

	* regcache.c (reg_buffer::save): When saving the current register
	state, ignore registers that can't be read.
---
 gdb/ChangeLog  |  5 +++++
 gdb/regcache.c | 12 +++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 946035ae67a..b89be24ccb6 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -277,7 +277,17 @@ reg_buffer::save (register_read_ftype cooked_read)
       if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
 	{
 	  gdb_byte *dst_buf = register_buffer (regnum);
-	  enum register_status status = cooked_read (regnum, dst_buf);
+	  enum register_status status;
+
+	  TRY
+	    {
+	      status = cooked_read (regnum, dst_buf);
+	    }
+	  CATCH (ex, RETURN_MASK_ERROR)
+	    {
+	      status = REG_UNAVAILABLE;
+	    }
+	  END_CATCH
 
 	  gdb_assert (status != REG_UNKNOWN);
 
-- 
2.14.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]