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]

Re: [PATCH]: Don't use deprecated regcache functions


David S. Miller wrote:
From: Daniel Jacobowitz <drow@false.org>
Date: Tue, 11 Apr 2006 09:00:57 -0400


On Tue, Apr 11, 2006 at 01:39:09AM -0700, David S. Miller wrote:

Otherwise, ok to apply?

-      deprecated_read_register_gen (regno, raw);
+      regcache_raw_collect (current_regcache, regno, raw);
      thread_db_fetch_registers (-1);
      regcache_raw_supply (current_regcache, regno, raw);

I might be mistaken, but what the heck does the call to thread_db_fetch_registers accomplish? I think nothing.

Daniel, David's got the right of it.



The solaris thread support does the same thing.  And that code
uses the more correct regcache_raw_collect() which is partly how
I noticed this.

What's happening here is that if we're only writing one register
we make sure to read in all the other registers first, then we
write the changing register back into the regcache.

This is interrelated with another piece of gdb code history I
was researching last night when I ran across this.  Several
platforms used to define CHILD_PREPARE_TO_STORE in order to
deal with interfaces, such as some ptrace() variants, that can
only set the whole set of registers at once.

There are some really nasty cases on Sparc for example, say you
want to write just the stack pointer.  We obtain the local and
in registers from the on-stack save area, so if we were not careful
just changing the stack or the frame pointer would cause all of
those other registers to change even though that is not what we
intended.  So to do it right, you have to read all the registers
into the regcache, then modify the stack or frame pointer.

This CHILD_PREPARE_TO_STORE macro would get invoked by the
target_prepare_to_store().  If you look at the regcache code it always
does a sequence like this:

  target_prepare_to_store ();
  memcpy (register_buffer (regcache, regnum), buf,
	  regcache->descr->sizeof_register[regnum]);
  regcache->register_valid_p[regnum] = 1;
  target_store_registers (regnum);

So, if necessary, target_prepare_to_store() would make sure the
regcache was fully read in if not already, then it would be safe to
write into the register cache.

But it seems that Mark Kettenis has been trying to transition
away from this deprecated scheme, and instead handle this issue
inside of the store register methods of the individual targets
and native support modules.

In fact, the only platform defining CHILD_PREPARE_TO_STORE is
GNU Hurd on x86, and probably that will eventually be eliminated
as well, and along with it target_prepare_to_store() and all
assosciated hooks.

In any event, the code we are discussing here in the Linux thread_db
code really is needed and it's related to the issues I've discussed
above.

So I think it's ok to apply this :-)



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