Re: [rfa] Don't refer to previous arch in rs6000_gdbarch_init()

[Kevin Buettner <kevinb at cygnus dot com>]

On Sep 25, 10:03pm, Andrew Cagney wrote:

> The attatched patch removes a reference to the old architecture (via 
> TDEP -> gdbarch_tdep (current_gdbarch)) in rs6000_gdbarch_init().
> 
> Given that the previous architecture may not be a member of the rs6000 
> family, using a value from the previous architectures tdep makes for a 
> high risk operation.  I think rs6000 should instead arrange to keep a 
> local copy of any stuff it wants to refer to.

I agree.  I do wonder though why the original author of this code (Nick D,
I think) found the need to refer to the old architecture.  The
comment says the following:

  /* Check word size.  If INFO is from a binary file, infer it from that,
     else use the previously-inferred size. */

But I don't see the motivation for using a previously-inferred size. 
Here is my reasoning...

rs6000_gdbarch_init() will sometimes be called without having an
executable to work with, but, if I'm not mistaken, it'll always
get called again once the user issues a ``file'' command (or an
implicit file command is performed as a result of providing the
executable file to gdb via the command line used to start up gdb).

So, by the time we get anywhere close to code which cares about
the word size, rs6000_gdbarch_init() will have been called in
a fashion such that one of the other two paths which set the word
size will have been executed.  Furthermore, it is not subsequently
called (prior to running the target) in such a way to require the 
need for the last chance setting of the word size (which is the
case that you're fixing).

Also, as you've observed, using the previously-inferred size could
be dangerous since the old architecture might be completely unrelated
to rs6000 or powerpc.  (It could get set to some oldball value which'd
make no sense at all for these architectures.)

> Is this change to address the immediate problem ok?

Yes.  But please change the comment mentioned above to read something
like this...

  /* Check word size.  If INFO is from a binary file, infer it from that,
     else choose a likely default.  */

(Feel free to improve upon the wording...)

> 	* rs6000-tdep.c (rs6000_gdbarch_init): Don't use the previous
> 	architecture to infer the wordsize.  Previous architecture may not
> 	be a PowerPC.

Thanks,

Kevin