[rfc] [00/16] Get rid of current gdbarch

[rfc] [00/16] Get rid of current gdbarch

[Markus Deuling]

Hi,

a while ago I sent a first patch set to get rid of current_gdbarch. I now added some patches and deferred some other and reworked the ChangeLog entries due to feedback from you. Now that 6.7 branch was created this is a reworked patch set for current head.

This patch set is another step in the direction of per-frame architecture. This set of 16 patches replaces (trivial to replace occurences of) current_gdbarch by appropriate methods like get_frame_arch or get_regcache_arch etc.

Currently get_frame_arch just returns current_gdbarch but this will be replaced by later patches.

I tested this patch set on x86 native and remote by compiling and running the testsuite. I also did a gdb_mbuild with all
targets to make sure it compiles.

Some later patches will be non-trivial and hard for me to test due to lack of those machines. It would be great if some of you could test patches for different archs like mips, arm, etc. but this will still take a while.

This patch set replaces some of the current_gdbarch's in following files:

* aix-thread.c
* arm-tdep.c
* alpha-{linux-nat,nat,tdep}.c
* findvar.c
* amd64-{nat,tdep}.c
* i386-tdep.c
* sh[64]-tdep.c
* remote.c
* irix5-nat.c
* regcache.c
* m68k{linux-nat, linux-tdep, -tdep}.c
* dwarf2{loc, -frame}.c
* cris-tdep.c
* hppa-{hpux-nat, hpux-tdep, linux-nat, tdep}.c

Is this ok? 




-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Ulrich Weigand]

Markus Deuling wrote:

> This patch set replaces some of the current_gdbarch's in following files:
> 
> * aix-thread.c
> * arm-tdep.c
> * alpha-{linux-nat,nat,tdep}.c
> * findvar.c
> * amd64-{nat,tdep}.c
> * i386-tdep.c
> * sh[64]-tdep.c
> * remote.c
> * irix5-nat.c
> * regcache.c
> * m68k{linux-nat, linux-tdep, -tdep}.c
> * dwarf2{loc, -frame}.c
> * cris-tdep.c
> * hppa-{hpux-nat, hpux-tdep, linux-nat, tdep}.c

I've checked all of those in, except for the i386-tdep.c one:

@@ -169,6 +169,8 @@ i386_register_name (int regnum)
 static int
 i386_dbx_reg_to_regnum (int reg)
 {
+  struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
+
   /* This implements what GCC calls the "default" register map
      (dbx_register_map[]).  */
 
You should not call get_current_frame at this point; this might
in fact fail as there is not necessarily a current frame selected
at the point this callback is called.

Instead, you should change the XXX_reg_to_regnum gdbarch entries
from type "f" to "m"; then the functions will be automatically
provided with a gdbarch parameter.  (This should be a separate
patch from the remaining trivial current_gdbarch replacements.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Markus Deuling]

Ulrich Weigand schrieb:
> Markus Deuling wrote:
> 

> I've checked all of those in, except for the i386-tdep.c one:

Thank you very much.

> @@ -169,6 +169,8 @@ i386_register_name (int regnum)
>  static int
>  i386_dbx_reg_to_regnum (int reg)
>  {
> +  struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
> +
>    /* This implements what GCC calls the "default" register map
>       (dbx_register_map[]).  */
>  

Ops, you're right. Thank you very much. This one shouldn't have been in there.

> You should not call get_current_frame at this point; this might
> in fact fail as there is not necessarily a current frame selected
> at the point this callback is called.
> 
> Instead, you should change the XXX_reg_to_regnum gdbarch entries
> from type "f" to "m"; then the functions will be automatically
> provided with a gdbarch parameter.  (This should be a separate
> patch from the remaining trivial current_gdbarch replacements.)
> 

Yes' I'll come up with a patch for that...

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Daniel Jacobowitz]

On Mon, Oct 08, 2007 at 03:02:51PM +0200, Ulrich Weigand wrote:
> You should not call get_current_frame at this point; this might
> in fact fail as there is not necessarily a current frame selected
> at the point this callback is called.
> 
> Instead, you should change the XXX_reg_to_regnum gdbarch entries
> from type "f" to "m"; then the functions will be automatically
> provided with a gdbarch parameter.  (This should be a separate
> patch from the remaining trivial current_gdbarch replacements.)

   /* This will hopefully provoke a warning.  */
-  return gdbarch_num_regs (current_gdbarch)
-        + gdbarch_num_pseudo_regs (current_gdbarch);
+  return gdbarch_num_regs (gdbarch)
+        + gdbarch_num_pseudo_regs (gdbarch);

That's the only thing it's used for.  Does it provoke a warning?  If
not, maybe -1 or something similar should...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] [00/16] Get rid of current gdbarch

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Mon, Oct 08, 2007 at 03:02:51PM +0200, Ulrich Weigand wrote:
> > You should not call get_current_frame at this point; this might
> > in fact fail as there is not necessarily a current frame selected
> > at the point this callback is called.
> > 
> > Instead, you should change the XXX_reg_to_regnum gdbarch entries
> > from type "f" to "m"; then the functions will be automatically
> > provided with a gdbarch parameter.  (This should be a separate
> > patch from the remaining trivial current_gdbarch replacements.)
> 
>    /* This will hopefully provoke a warning.  */
> -  return gdbarch_num_regs (current_gdbarch)
> -        + gdbarch_num_pseudo_regs (current_gdbarch);
> +  return gdbarch_num_regs (gdbarch)
> +        + gdbarch_num_pseudo_regs (gdbarch);
> 
> That's the only thing it's used for.  Does it provoke a warning?  If
> not, maybe -1 or something similar should...

Agreed for that particular usage.  In general, the reg_to_regnum
routines should probably still be converted to "m" -- e.g. the
rs6000 versions do make non-trivial use of the gdbarch ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Daniel Jacobowitz]

On Mon, Oct 08, 2007 at 04:01:36PM +0200, Ulrich Weigand wrote:
> Agreed for that particular usage.  In general, the reg_to_regnum
> routines should probably still be converted to "m" -- e.g. the
> rs6000 versions do make non-trivial use of the gdbarch ...

Isn't that problematic?  We do not know what the architecture will
be at this point.  Only the bits common to all architectures using
the same init routine are safe to use.

As for the rs6000 version, the patches I posted last week allow a
followup patch which propogates some constants.  Most of the values
being read from the tdep are now constants.  Some (e.g. ppc_mq_regnum)
are not constants, but have either a single constant value or -1 if
the register is not present; for those the constant is appropriate
in the reg_to_regnum routines anyway.

For a concrete example, suppose the default PowerPC architecture
selected by GDB did not include AltiVec registers.  Variables
living in such registers would get bogus locations from the
reg_to_regnum routine and it would not be fixed up when we
connected to an AltiVec-capable target.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] [00/16] Get rid of current gdbarch

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Mon, Oct 08, 2007 at 04:01:36PM +0200, Ulrich Weigand wrote:
> > Agreed for that particular usage.  In general, the reg_to_regnum
> > routines should probably still be converted to "m" -- e.g. the
> > rs6000 versions do make non-trivial use of the gdbarch ...
> 
> Isn't that problematic?  We do not know what the architecture will
> be at this point.  Only the bits common to all architectures using
> the same init routine are safe to use.

Hmm, good point, I'll have to think about this ...

> As for the rs6000 version, the patches I posted last week allow a
> followup patch which propogates some constants.  Most of the values
> being read from the tdep are now constants.  Some (e.g. ppc_mq_regnum)
> are not constants, but have either a single constant value or -1 if
> the register is not present; for those the constant is appropriate
> in the reg_to_regnum routines anyway.

I agree that everything would be much easier if the register numbers
were constants.  But note that rs6000 is not the only platform where
this is not the case, I see the same (or even worse) also in mips,
m32c, xtensa, ...  (The mep usage also seems suspicious.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Daniel Jacobowitz]

On Tue, Oct 09, 2007 at 09:59:39PM +0200, Ulrich Weigand wrote:
> I agree that everything would be much easier if the register numbers
> were constants.  But note that rs6000 is not the only platform where
> this is not the case, I see the same (or even worse) also in mips,
> m32c, xtensa, ...  (The mep usage also seems suspicious.)

Yes.  For some platforms (xtensa, mep) there may not be much we can
do... and MIPS presents its own unique problems because of the use
of pseudo registers for everything.  Hmm, this makes me wonder how
deep the broken goes.  Maybe we shouldn't translate from DWARF numbers
to internal ones until later.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] [00/16] Get rid of current gdbarch

[Mark Kettenis]

> Date: Tue, 9 Oct 2007 16:03:21 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Tue, Oct 09, 2007 at 09:59:39PM +0200, Ulrich Weigand wrote:
> > I agree that everything would be much easier if the register numbers
> > were constants.  But note that rs6000 is not the only platform where
> > this is not the case, I see the same (or even worse) also in mips,
> > m32c, xtensa, ...  (The mep usage also seems suspicious.)
> 
> Yes.  For some platforms (xtensa, mep) there may not be much we can
> do... and MIPS presents its own unique problems because of the use
> of pseudo registers for everything.  Hmm, this makes me wonder how
> deep the broken goes.  Maybe we shouldn't translate from DWARF numbers
> to internal ones until later.

The solution here is (and always has been) making GDB's register
number encoding a truly internal encoding.  Then we could easily unify
the different rs6000/powerpc variants.  I understand that people want
to keep the ability to talk to old stubs, so the remote target support
code in GDB should take care of doing the conversion.

Mark

Re: [rfc] [00/16] Get rid of current gdbarch

[Daniel Jacobowitz]

On Wed, Oct 10, 2007 at 11:03:50AM +0200, Mark Kettenis wrote:
> The solution here is (and always has been) making GDB's register
> number encoding a truly internal encoding.  Then we could easily unify
> the different rs6000/powerpc variants.  I understand that people want
> to keep the ability to talk to old stubs, so the remote target support
> code in GDB should take care of doing the conversion.

The patches I posted last week do this for PowerPC, in fact!

But there are places where we can't do it this way - at least not
without bigger changes.  For instance, on MIPS the
dwarf2_reg_to_regnum method returns pseudo registers.  The numbering
of pseudo registers starts at NUM_REGS, which varies depending on
the OS/ABI and on what target we end up connected to :-(

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] [00/16] Get rid of current gdbarch

[Joel Brobecker]

> Some later patches will be non-trivial and hard for me to test due to lack 
> of those machines. It would be great if some of you could test patches for 
> different archs like mips, arm, etc. but this will still take a while.

Sure, I can help with the testing on the machines that we have here.

-- 
Joel

Re: [rfc] [00/16] Get rid of current gdbarch

[Markus Deuling]

Joel Brobecker schrieb:
>> Some later patches will be non-trivial and hard for me to test due to lack 
>> of those machines. It would be great if some of you could test patches for 
>> different archs like mips, arm, etc. but this will still take a while.
> 
> Sure, I can help with the testing on the machines that we have here.
> 

Great! Thanks a lot. I'll CC you with the appropriate patches.

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: [rfc] [00/16] Get rid of current gdbarch

[Maxim Grigoriev]

Hello, Markus,

I think you missed xtenda-tdep.c.
First time, you asked for an approval but never checked the patch in.

What do you want me to do : to wait until you update it and then test 
it, or just go ahead and do it by myself ?

-- Maxim


Markus Deuling wrote:
> Hi,
>
> a while ago I sent a first patch set to get rid of current_gdbarch. I 
> now added some patches and deferred some other and reworked the 
> ChangeLog entries due to feedback from you. Now that 6.7 branch was 
> created this is a reworked patch set for current head.
>
> This patch set is another step in the direction of per-frame 
> architecture. This set of 16 patches replaces (trivial to replace 
> occurences of) current_gdbarch by appropriate methods like 
> get_frame_arch or get_regcache_arch etc.
>
> Currently get_frame_arch just returns current_gdbarch but this will be 
> replaced by later patches.
>
> I tested this patch set on x86 native and remote by compiling and 
> running the testsuite. I also did a gdb_mbuild with all
> targets to make sure it compiles.
>
> Some later patches will be non-trivial and hard for me to test due to 
> lack of those machines. It would be great if some of you could test 
> patches for different archs like mips, arm, etc. but this will still 
> take a while.
>
> This patch set replaces some of the current_gdbarch's in following files:
>
> * aix-thread.c
> * arm-tdep.c
> * alpha-{linux-nat,nat,tdep}.c
> * findvar.c
> * amd64-{nat,tdep}.c
> * i386-tdep.c
> * sh[64]-tdep.c
> * remote.c
> * irix5-nat.c
> * regcache.c
> * m68k{linux-nat, linux-tdep, -tdep}.c
> * dwarf2{loc, -frame}.c
> * cris-tdep.c
> * hppa-{hpux-nat, hpux-tdep, linux-nat, tdep}.c
>
> Is this ok?
>
>
>

Re: [rfc] [00/16] Get rid of current gdbarch

[Markus Deuling]

Hi Maxim,

Maxim Grigoriev schrieb:
> Hello, Markus,
> 
> I think you missed xtenda-tdep.c.
> First time, you asked for an approval but never checked the patch in.
I still cannot check in patches due to lack of FSF copyright assignment :-( Ulrich checks in my patches and he wasn't available
the last weeks.

> What do you want me to do : to wait until you update it and then test 
> it, or just go ahead and do it by myself ?
I did not send the xtensa patches with the new patch set because they're outdated a bit. They dont apply clean anymore and I have to 
rework them. 

Thank you very much! I'll come up with a new patch set including the xtensa patches soon.


Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com