Re: mark raw registers the target doesn't have access to as unavailable

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 23 Mar 2011 21:10:52 +0000
> 
> This change:
> 
> <http://sourceware.org/ml/gdb-cvs/2011-03/msg00235.html> :
> 
> 2011-03-18  Pedro Alves  <pedro@codesourcery.com>
> 
>         ...
>         * regcache.c: ...
>         (regcache_save): Adjust to handle REG_UNAVAILABLE registers.
>         ...
> 
> Broke amd64 and ppc freebsd gdbs, and probably other targets.  They're
> hitting this new assertion:
> 
> void
> regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read,
> 	       void *src)
> {
> ...
>   for (regnum = 0; regnum < dst->descr->nr_cooked_registers; regnum++)
>     {
>       if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
> 	{
> 	  enum register_status status = cooked_read (src, regnum, buf);
> 
> 	  if (status == REG_VALID)
> 	    memcpy (register_buffer (dst, regnum), buf,
> 		    register_size (gdbarch, regnum));
> 	  else
> 	    {
> 	      gdb_assert (status != REG_UNKNOWN);
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 	      memset (register_buffer (dst, regnum), 0,
> 		      register_size (gdbarch, regnum));
> 	    }
> 	  dst->register_status[regnum] = status;
> 	}
>     }
> 
> In fact, the only thing relevant that I changed was adding
> the assert.
> 
> The idea was, if we've just read a register, we should know whether
> its value is valid, or unavailable.  Shouldn't be "unknown" any more.
> 
> Now, what I missed is that some targets have debug apis that
> don't give debugger access to all the raw registers the architecture
> supports.  E.g., from amd64fbsd-tdep.c:
> 
> /* Mapping between the general-purpose registers in `struct reg'
>    format and GDB's register cache layout.
> ...
> static int amd64fbsd_r_reg_offset[] =
> {
>   14 * 8,			/* %rax */
> ...
>   21 * 8,			/* %ss */
>   -1,				/* %ds */
>   -1,				/* %es */
>   -1,				/* %fs */
>   -1				/* %gs */
> };
> 
> All those -1's mean that those registers aren't retrievable
> from fbsd's `struct reg'.
> 
> I see amd64-darwin-tdep.c:amd64_darwin_thread_state_reg_offset
> also doesn't expose a few of the segment registers.
> 
> I think this calls for marking these registers as "unavailable".
> That is, they exist in the target machine, so it's correct for
> the target's description to include them, but, GDB just
> doesn't know their value.  That's what the patch below does.
> 
> Here's the result on amd64-fbsd:
> 
> (gdb) info registers
> rax            0xffffffffffffffff       -1
> rbx            0x1      1
> rcx            0x800da59d0      34374048208
> rdx            0x7fffffffda48   140737488345672
> rsi            0x7fffffffda38   140737488345656
> rdi            0x1      1
> rbp            0x7fffffffd9e0   0x7fffffffd9e0
> rsp            0x7fffffffd9c0   0x7fffffffd9c0
> r8             0x3a     58
> r9             0x7fffffffef42   140737488351042
> r10            0x18     24
> r11            0x0      0
> r12            0x7fffffffda48   140737488345672
> r13            0x7fffffffda38   140737488345656
> r14            0x0      0
> r15            0x0      0
> rip            0x400733 0x400733 <main+67>
> eflags         0x293    [ CF AF SF IF ]
> cs             0x43     67
> ss             0x3b     59
> ds             *value not available*
> es             *value not available*
> fs             *value not available*
> gs             *value not available*
> 
> Notice the "*value not available*".  Working with that register
> will yield:
> 
> (gdb) p $ds
> $1 = <unavailable>
> (gdb) p $ds + 1
> value is not available
> 
> I think this is appropriate.

I think what you're proposing makes tons of sense.  Please go ahead.