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

[Daniel Jacobowitz <drow at false dot org>]

On Tue, Apr 11, 2006 at 02:13:34PM -0700, 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.
> 
> 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.

Well, yes.  Sorry for sending you off in the wrong direction; that's
not actually what I meant.  The bit that doesn't follow is:

> 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.

No, it really shouldn't be, for those same reasons.  But...

[long, mostly incorrect analysis deleted here.  This is confusing.]

[There's a lot more prepare-to-store methods than you
thought; while only one target defines the macro, many set it in the
target vector to_prepare_to_store.]

-      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);

fill_gregset (at least the example I checked, SPARC) will not read
registers as necessary - it also uses regcache_raw_collect.  So some
members of the gregset won't necessarily be filled in as valid unless
all registers are fetched first.  So that suggests the regno == -1 case
is bogus too.  Fortunately the lion's share of calls to these functions
come from regcache.c, which only ever writes one register at a time.

I think:

  - The calls to fill_gregset and fill_fpregset necessitate using
    target_fetch_registers (-1).  That's more or less the same as
    thread_db_fetch_registers (-1).  Either works.

  - What was previously unclear was that thread_db_fetch_registers
    may clobber already fetched registers.  How naughty.

  - It's totally unclear how the -1 case could work, since we can't
    fetch the registers without clobbering what we wanted to write
    out, which may explain why we previously didn't.  We'd need to
    preserve all the cached registers.  Another reason not to use
    the gregset functions this way.

What a mess.

Anyway, I guess the mess is long-standing and currently not a problem,
so go ahead and check in the change to use regcache_raw_collect.

-- 
Daniel Jacobowitz
CodeSourcery