Re: RFA: assert that target_fetch_registers did its job

[Jim Blandy <jimb@redhat.com>]

"Nathan J. Williams" <nathanw@wasabisystems.com> writes:

> Jim Blandy <jimb@redhat.com> writes:
> 
> > Does anyone see anything wrong with this?  Should it be an error, or a
> > warning, instead of an internal error?  It seems to me that the error
> > should be furnished by the target-specific code; if
> > target_fetch_registers returns silently, it should have done its job.
> 
> I noticed that "info reg" tripped over this with the BSD KVM target,
> because the KVM backend only loads in the registers that are present
> in the PCB. Should the KVM backend be zeroing out everything else
> explicitly (is there a regcache call to do the wipe for us)?

Silently supplying dummy register contents is confusing to developers
and users.  That just seems uncool.

There's no way to return an 'unavailable' indication from the regcache
cooked read functions.  And there are so many clients of that
interface it might be hard to introduce such an indication.  (Although
most of the uses are in -tdep.c files; the only real use in core code
is in sentinal_frame_prev_register, which does have a way to return
'unavailable'.  Hmm.)

Throwing an error would interrupt the register listing, even if later
registers were available.

So that leaves printing a warning message and supplying a dummy
value.  

Perhaps another option would be something like the below: it informs
users that things are not as they seem, and encourages developers to
fix up their targets.  Comments?


2004-08-06  Jim Blandy  <jimb@redhat.com>

	* regcache.c (regcache_raw_read): Replace assertion with a warning
	message.

Index: gdb/regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.125
diff -c -p -r1.125 regcache.c
*** gdb/regcache.c	4 Aug 2004 17:50:55 -0000	1.125
--- gdb/regcache.c	6 Aug 2004 23:42:56 -0000
*************** regcache_raw_read (struct regcache *regc
*** 614,620 ****
  	}
        if (!register_cached (regnum))
  	target_fetch_registers (regnum);
!       gdb_assert (register_cached (regnum));
      }
    /* Copy the value directly into the register cache.  */
    memcpy (buf, register_buffer (regcache, regnum),
--- 614,649 ----
  	}
        if (!register_cached (regnum))
  	target_fetch_registers (regnum);
! 
!       /* FIXME: jimb/2004-08-06: Ideally, this would just be an
!          assert: target_fetch_registers should either throw an error,
!          or print a warning and furnish dummy register contents.  But
!          it should never say "okay" without actually supplying the
!          register.
! 
!          But in reality, there are a lot of arch / target combinations
!          where the arch's regset has grown beyond what the target
!          actually supplies.  Having GDB crash is too disruptive, but
!          silently using uninitialized bits is misleading as well.
! 
!          So this warning is here to prompt people to work on their
!          targets and get the mismatches sorted out, and to warn users
!          that they're not getting real data.  */
!       if (! register_cached (regnum))
!         {
!           struct gdbarch *arch = get_regcache_arch (regcache);
!           const char *name = gdbarch_register_name (arch, regnum);
! 
!           if (name)
!             warning ("unable to retrieve contents of raw register '%s' (#%d).",
!                      name, regnum);
!           else
!             warning ("unable to retrieve contents of raw register #%d.",
!                      regnum);
! 
!           memset (register_buffer (regcache, regnum), 0xff,
!                   regcache->descr->sizeof_register[regnum]);
!         }
      }
    /* Copy the value directly into the register cache.  */
    memcpy (buf, register_buffer (regcache, regnum),