patch for printing 64-bit values in i386 registers; STABS format

patch for printing 64-bit values in i386 registers; STABS format

[Colin Smith]

* Description of bug: 

On i386 with STABS debug format, if the debug info for a function
indicates that a long long variable is held in a register, for example
%ebx, GDB will assume that the other register follows the first one
in gdb register number order.  But GDB register numbering and GCC
register numbering do not correspond.

The patch fixes this by offering a new macro that will compute the 
"next" register given the first one in the event a value is spread
across multiple registers.  We implement this macro in the i386
case.

* Changelog Entry

2003-04-24  Colin Smith  <colins@google.com>

	* findvar.c (value_from_register): use new NEXT_LOCAL_REGNUM to
	find follow-on registers for values larger than one register, in
	the STABS case.  Implemented in tm-i386.h and i386-tdep.c.

* Example code that provokes the bug:

    #include <iostream>
    //using namespace std;

    typedef long long uint64;

    inline uint64 g(uint64 v, uint64 w) {
      cout << v << ' ' << w << endl;
      ++w;
      return v-w;
    }

    int k = 3;

    main() {
      uint64 v = 0x1111111122222222;

      if (v > 0) { 
	uint64 u = 0xaaaaaaaabbbbbbbb;

	for (int i = 0; i < k; ++i)
	  v = g(u+v,v);
      }
    }

... compile with gcc (2.95.x), -O0, -g; set breakpoint on 'g', run, 
    'print w', note that value is incorrect; do 'i addr w', see that
    it's supposed to be in %ebx, do 'i regi' and see that GDB thinks
    that the value is in %esp:%ebx, when in fact it's in %esi:%ebx.

* Text of Patch.

diff -cpr gdb-orig/gdb/config/i386/tm-i386.h gdb/gdb/config/i386/tm-i386.h
*** gdb-orig/gdb/config/i386/tm-i386.h	Thu Apr 24 15:27:35 2003
--- gdb/gdb/config/i386/tm-i386.h	Thu Apr 24 13:06:52 2003
***************
*** 22,27 ****
--- 22,36 ----
  #ifndef TM_I386_H
  #define TM_I386_H 1
  
+ /* This macro maps between GCC and GDB's register ordering.  This is 
+    used when fetching quantities larger than one register out of the
+    register file: GDB's register ordering is not indicative of the 
+    order in which these registers would have been allocated by the 
+    compiler. */
+ 
+ #define NEXT_LOCAL_REGNUM(regnum) \
+   i386_next_local_regnum (regnum)
+ 
  #define GDB_MULTI_ARCH GDB_MULTI_ARCH_PARTIAL
  
  /* FIXME: kettenis/2000-06-12: These do not belong here.  */
diff -cpr gdb-orig/gdb/findvar.c gdb/gdb/findvar.c
*** gdb-orig/gdb/findvar.c	Thu Apr 24 15:27:32 2003
--- gdb/gdb/findvar.c	Thu Apr 24 12:29:06 2003
***************
*** 46,51 ****
--- 46,62 ----
  you lose
  #endif
  
+ /* GCC's register ordering (the ordering it will follow when
+    allocating 64-bit quantities among multiple registers) is not
+    necessarily the same as GDB's register numbering.  In the event 
+    that these orderings don't agree, a config file can specify 
+    an alternate implementation of this macro.  (This case occurs
+    on gcc2 x i386, for example.) */
+    
+ #ifndef NEXT_LOCAL_REGNUM
+ #define NEXT_LOCAL_REGNUM(regnum) ((regnum)+1)
+ #endif
+ 
  LONGEST
  extract_signed_integer (const void *addr, int len)
  {
*************** value_from_register (struct type *type, 
*** 735,744 ****
  	}
        else
  #endif /* GDB_TARGET_IS_H8500 */
  	for (local_regnum = regnum;
  	     value_bytes_copied < len;
  	     (value_bytes_copied += REGISTER_RAW_SIZE (local_regnum),
! 	      ++local_regnum))
  	  {
  	    get_saved_register (value_bytes + value_bytes_copied,
  				&optim,
--- 746,756 ----
  	}
        else
  #endif /* GDB_TARGET_IS_H8500 */
+ 
  	for (local_regnum = regnum;
  	     value_bytes_copied < len;
  	     (value_bytes_copied += REGISTER_RAW_SIZE (local_regnum),
! 	      local_regnum = NEXT_LOCAL_REGNUM (local_regnum)))
  	  {
  	    get_saved_register (value_bytes + value_bytes_copied,
  				&optim,
diff -cpr gdb-orig/gdb/i386-tdep.c gdb/gdb/i386-tdep.c
*** gdb-orig/gdb/i386-tdep.c	Thu Apr 24 15:27:33 2003
--- gdb/gdb/i386-tdep.c	Thu Apr 24 12:59:01 2003
*************** static char *i386_register_names[] =
*** 58,63 ****
--- 58,86 ----
    "mxcsr"
  };
  
+ int 
+ i386_next_local_regnum (int reg)
+ {
+   /* Interpretation of the following table: because 
+      next_reg_map[regnum(ebx)] == regnum(esi), esi
+      follows ebx in multi-register data allocation
+      according to GCC. No register follows esp, since
+      esp is dedicated to another important function ;-) */
+ 
+                               /* eax ecx edx ebx esp ebp esi edi */
+   static int next_reg_map [] = {   2,  3,  1,  6, -1,  4,  7,  5 };
+   int n;
+ 
+   if (reg >= 0 && reg < 8)
+     if ((n = next_reg_map[reg]) >= 0)
+       return n;
+ 
+   /* Give up: we'll have to try gdb's standard algorithm.  We've
+      done no harm, though. */
+ 
+   return reg+1;
+ }
+ 
  /* MMX registers.  */
  
  static char *i386_mmx_names[] =

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

Hey, Mark, this sounds very much like a change you proposed.  What ever
happened to that patch?

On Thu, Apr 24, 2003 at 03:31:52PM -0700, Colin Smith wrote:
> 
> * Description of bug: 
> 
> On i386 with STABS debug format, if the debug info for a function
> indicates that a long long variable is held in a register, for example
> %ebx, GDB will assume that the other register follows the first one
> in gdb register number order.  But GDB register numbering and GCC
> register numbering do not correspond.
> 
> The patch fixes this by offering a new macro that will compute the 
> "next" register given the first one in the event a value is spread
> across multiple registers.  We implement this macro in the i386
> case.
> 
> * Changelog Entry
> 
> 2003-04-24  Colin Smith  <colins@google.com>
> 
> 	* findvar.c (value_from_register): use new NEXT_LOCAL_REGNUM to
> 	find follow-on registers for values larger than one register, in
> 	the STABS case.  Implemented in tm-i386.h and i386-tdep.c.
> 
> * Example code that provokes the bug:
> 
>     #include <iostream>
>     //using namespace std;
> 
>     typedef long long uint64;
> 
>     inline uint64 g(uint64 v, uint64 w) {
>       cout << v << ' ' << w << endl;
>       ++w;
>       return v-w;
>     }
> 
>     int k = 3;
> 
>     main() {
>       uint64 v = 0x1111111122222222;
> 
>       if (v > 0) { 
> 	uint64 u = 0xaaaaaaaabbbbbbbb;
> 
> 	for (int i = 0; i < k; ++i)
> 	  v = g(u+v,v);
>       }
>     }
> 
> ... compile with gcc (2.95.x), -O0, -g; set breakpoint on 'g', run, 
>     'print w', note that value is incorrect; do 'i addr w', see that
>     it's supposed to be in %ebx, do 'i regi' and see that GDB thinks
>     that the value is in %esp:%ebx, when in fact it's in %esi:%ebx.
> 
> * Text of Patch.
> 
> diff -cpr gdb-orig/gdb/config/i386/tm-i386.h gdb/gdb/config/i386/tm-i386.h
> *** gdb-orig/gdb/config/i386/tm-i386.h	Thu Apr 24 15:27:35 2003
> --- gdb/gdb/config/i386/tm-i386.h	Thu Apr 24 13:06:52 2003
> ***************
> *** 22,27 ****
> --- 22,36 ----
>   #ifndef TM_I386_H
>   #define TM_I386_H 1
>   
> + /* This macro maps between GCC and GDB's register ordering.  This is 
> +    used when fetching quantities larger than one register out of the
> +    register file: GDB's register ordering is not indicative of the 
> +    order in which these registers would have been allocated by the 
> +    compiler. */
> + 
> + #define NEXT_LOCAL_REGNUM(regnum) \
> +   i386_next_local_regnum (regnum)
> + 
>   #define GDB_MULTI_ARCH GDB_MULTI_ARCH_PARTIAL
>   
>   /* FIXME: kettenis/2000-06-12: These do not belong here.  */
> diff -cpr gdb-orig/gdb/findvar.c gdb/gdb/findvar.c
> *** gdb-orig/gdb/findvar.c	Thu Apr 24 15:27:32 2003
> --- gdb/gdb/findvar.c	Thu Apr 24 12:29:06 2003
> ***************
> *** 46,51 ****
> --- 46,62 ----
>   you lose
>   #endif
>   
> + /* GCC's register ordering (the ordering it will follow when
> +    allocating 64-bit quantities among multiple registers) is not
> +    necessarily the same as GDB's register numbering.  In the event 
> +    that these orderings don't agree, a config file can specify 
> +    an alternate implementation of this macro.  (This case occurs
> +    on gcc2 x i386, for example.) */
> +    
> + #ifndef NEXT_LOCAL_REGNUM
> + #define NEXT_LOCAL_REGNUM(regnum) ((regnum)+1)
> + #endif
> + 
>   LONGEST
>   extract_signed_integer (const void *addr, int len)
>   {
> *************** value_from_register (struct type *type, 
> *** 735,744 ****
>   	}
>         else
>   #endif /* GDB_TARGET_IS_H8500 */
>   	for (local_regnum = regnum;
>   	     value_bytes_copied < len;
>   	     (value_bytes_copied += REGISTER_RAW_SIZE (local_regnum),
> ! 	      ++local_regnum))
>   	  {
>   	    get_saved_register (value_bytes + value_bytes_copied,
>   				&optim,
> --- 746,756 ----
>   	}
>         else
>   #endif /* GDB_TARGET_IS_H8500 */
> + 
>   	for (local_regnum = regnum;
>   	     value_bytes_copied < len;
>   	     (value_bytes_copied += REGISTER_RAW_SIZE (local_regnum),
> ! 	      local_regnum = NEXT_LOCAL_REGNUM (local_regnum)))
>   	  {
>   	    get_saved_register (value_bytes + value_bytes_copied,
>   				&optim,
> diff -cpr gdb-orig/gdb/i386-tdep.c gdb/gdb/i386-tdep.c
> *** gdb-orig/gdb/i386-tdep.c	Thu Apr 24 15:27:33 2003
> --- gdb/gdb/i386-tdep.c	Thu Apr 24 12:59:01 2003
> *************** static char *i386_register_names[] =
> *** 58,63 ****
> --- 58,86 ----
>     "mxcsr"
>   };
>   
> + int 
> + i386_next_local_regnum (int reg)
> + {
> +   /* Interpretation of the following table: because 
> +      next_reg_map[regnum(ebx)] == regnum(esi), esi
> +      follows ebx in multi-register data allocation
> +      according to GCC. No register follows esp, since
> +      esp is dedicated to another important function ;-) */
> + 
> +                               /* eax ecx edx ebx esp ebp esi edi */
> +   static int next_reg_map [] = {   2,  3,  1,  6, -1,  4,  7,  5 };
> +   int n;
> + 
> +   if (reg >= 0 && reg < 8)
> +     if ((n = next_reg_map[reg]) >= 0)
> +       return n;
> + 
> +   /* Give up: we'll have to try gdb's standard algorithm.  We've
> +      done no harm, though. */
> + 
> +   return reg+1;
> + }
> + 
>   /* MMX registers.  */
>   
>   static char *i386_mmx_names[] =
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Mark Kettenis]

   Date: Thu, 24 Apr 2003 20:27:44 -0400
   From: Daniel Jacobowitz <drow@mvista.com>

   Hey, Mark, this sounds very much like a change you proposed.  What ever
   happened to that patch?

It's still happily sitting in my tree :-(.  There didn't seem to be
any consensus on whether making this change was a good idea.  I still
think it is.  It's an improvement for the majority of our users, and
it isn't making things worse for others.  Do you think I should
re-submit my patch?

Mark

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Fri, Apr 25, 2003 at 11:21:13PM +0200, Mark Kettenis wrote:
>    Date: Thu, 24 Apr 2003 20:27:44 -0400
>    From: Daniel Jacobowitz <drow@mvista.com>
> 
>    Hey, Mark, this sounds very much like a change you proposed.  What ever
>    happened to that patch?
> 
> It's still happily sitting in my tree :-(.  There didn't seem to be
> any consensus on whether making this change was a good idea.  I still
> think it is.  It's an improvement for the majority of our users, and
> it isn't making things worse for others.  Do you think I should
> re-submit my patch?

I do, definitely.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

> On Fri, Apr 25, 2003 at 11:21:13PM +0200, Mark Kettenis wrote:
> 
>>    Date: Thu, 24 Apr 2003 20:27:44 -0400
>>    From: Daniel Jacobowitz <drow@mvista.com>
>> 
>>    Hey, Mark, this sounds very much like a change you proposed.  What ever
>>    happened to that patch?
>> 
>> It's still happily sitting in my tree :-(.  There didn't seem to be
>> any consensus on whether making this change was a good idea.  I still
>> think it is.  It's an improvement for the majority of our users, and
>> it isn't making things worse for others.  Do you think I should
>> re-submit my patch?
> 
> 
> I do, definitely.

FYI,

It's possible to fix this without adding an architecture method, or 
implementing location expressions (the penny just dropped).  The basic 
problem is the same as for the MIPS - need a custom register area.  Hence:

- define a sequence of nameless cooked ([NUM_REGS .. 
NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
like them
- modify the existing stabs_regnum_to_regnum to map the messed up 
registers onto those values

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Fri, Apr 25, 2003 at 06:29:02PM -0400, Andrew Cagney wrote:
> >On Fri, Apr 25, 2003 at 11:21:13PM +0200, Mark Kettenis wrote:
> >
> >>   Date: Thu, 24 Apr 2003 20:27:44 -0400
> >>   From: Daniel Jacobowitz <drow@mvista.com>
> >>
> >>   Hey, Mark, this sounds very much like a change you proposed.  What ever
> >>   happened to that patch?
> >>
> >>It's still happily sitting in my tree :-(.  There didn't seem to be
> >>any consensus on whether making this change was a good idea.  I still
> >>think it is.  It's an improvement for the majority of our users, and
> >>it isn't making things worse for others.  Do you think I should
> >>re-submit my patch?
> >
> >
> >I do, definitely.
> 
> FYI,
> 
> It's possible to fix this without adding an architecture method, or 
> implementing location expressions (the penny just dropped).  The basic 
> problem is the same as for the MIPS - need a custom register area.  Hence:
> 
> - define a sequence of nameless cooked ([NUM_REGS .. 
> NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
> like them
> - modify the existing stabs_regnum_to_regnum to map the messed up 
> registers onto those values

Could you explain why you think that (which I personally think is much
grosser, since it perpuates the assumption that values continue into
sequential registers) is a better solution than Mark's approach?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

>> It's possible to fix this without adding an architecture method, or 
>> implementing location expressions (the penny just dropped).  The basic 
>> problem is the same as for the MIPS - need a custom register area.  Hence:
>> 
>> - define a sequence of nameless cooked ([NUM_REGS .. 
>> NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
>> like them
>> - modify the existing stabs_regnum_to_regnum to map the messed up 
>> registers onto those values
> 
> 
> Could you explain why you think that (which I personally think is much
> grosser, since it perpuates the assumption that values continue into
> sequential registers) is a better solution than Mark's approach?

The assumption that values continue into sequential registers is, 
unfortunatly. how stabs works :-(

The consequence of `without adding an architecture method' is that it 
confinds the i386 case to the i386.  The MIPS case, which is far worse, 
will also be the same (at present there is a slew of per-architecture 
methods that can be eliminated when the MIPS switches to the same strategy).

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Fri, Apr 25, 2003 at 10:44:37PM -0400, Andrew Cagney wrote:
> 
> >>It's possible to fix this without adding an architecture method, or 
> >>implementing location expressions (the penny just dropped).  The basic 
> >>problem is the same as for the MIPS - need a custom register area.  Hence:
> >>
> >>- define a sequence of nameless cooked ([NUM_REGS .. 
> >>NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
> >>like them
> >>- modify the existing stabs_regnum_to_regnum to map the messed up 
> >>registers onto those values
> >
> >
> >Could you explain why you think that (which I personally think is much
> >grosser, since it perpuates the assumption that values continue into
> >sequential registers) is a better solution than Mark's approach?
> 
> The assumption that values continue into sequential registers is, 
> unfortunatly. how stabs works :-(

So?  I don't want to bind anything in GDB's design to how stabs works. 
That's gotten us in all sorts of trouble.

> The consequence of `without adding an architecture method' is that it 
> confinds the i386 case to the i386.  The MIPS case, which is far worse, 
> will also be the same (at present there is a slew of per-architecture 
> methods that can be eliminated when the MIPS switches to the same strategy).

I'm afraid I don't understand, and I still don't see your reasoning
against this approach.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

> I'm afraid I don't understand, and I still don't see your reasoning
> against this approach.

It isn't necessary, just like register convertible and register 
raw/virtual size; .... that go before it, also were not necessary.  And 
now all these years later, GDB is still yet to expunge.

Until someone does the right think - add support for values scattered 
across registers and memory - hacks should be confined to architecture 
specific code.

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Fri, Apr 25, 2003 at 11:32:47PM -0400, Andrew Cagney wrote:
> 
> >I'm afraid I don't understand, and I still don't see your reasoning
> >against this approach.
> 
> It isn't necessary, just like register convertible and register 
> raw/virtual size; .... that go before it, also were not necessary.  And 
> now all these years later, GDB is still yet to expunge.
> 
> Until someone does the right think - add support for values scattered 
> across registers and memory - hacks should be confined to architecture 
> specific code.

I think the hack of introducing unnamed pseudo registers for this
purpose would do a lot more harm and cause a lot more problems; I guess
I just have to disagree with your reaction here.

Think about the day when we have proper support for DW_OP_piece.  For
compatibility with current debug info we're going to have to have a way
for the debug reader to ask the architecture "if I have a value of this
size listed as living in this register, where (probably) is it really
stored?".  That is _exactly_ the same question.  Asking it would move
from read_var_value to stabsread/dwarf2read, but the interface would
have to be just the same.

That's why I see Mark's patch as a monotonic step forwards.  And it
fixes a real problem.  We have to fix problems eventually, you know -
and no one has taken the initiative to implement scattered values.  I
tried.  It was a major pain; after a day working on it, I went back to
other projects.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Mark Kettenis]

   Date: Fri, 25 Apr 2003 23:32:47 -0400
   From: Andrew Cagney <ac131313@redhat.com>

   > I'm afraid I don't understand, and I still don't see your reasoning
   > against this approach.

   It isn't necessary, just like register convertible and register 
   raw/virtual size; .... that go before it, also were not necessary.  And 
   now all these years later, GDB is still yet to expunge.

I still don't see how you can get rid of the register convertible
stuff.  On the i386 I still need it for variables stuffed into the
floating point registers.

   Until someone does the right think - add support for values scattered 
   across registers and memory - hacks should be confined to architecture 
   specific code.

But even if someone does add support for values scattered across
multiple registers and/or memory, we still need the architecture
method I proposed.  There simply is too much debugging info out there
that can't express values being scattered across multiple registers.

And I don't think the hack you proposed is a good idea.  I think it's
better to add a new architecture method with a clear purpose than
abuse an existing mechanism for something that it wasn't quite
intended for.  Even if the architecture method in question would only
be used by a single target.

Mark

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

>    Date: Fri, 25 Apr 2003 23:32:47 -0400
>    From: Andrew Cagney <ac131313@redhat.com>
> 
>    > I'm afraid I don't understand, and I still don't see your reasoning
>    > against this approach.
> 
>    It isn't necessary, just like register convertible and register 
>    raw/virtual size; .... that go before it, also were not necessary.  And 
>    now all these years later, GDB is still yet to expunge.
> 
> I still don't see how you can get rid of the register convertible
> stuff.  On the i386 I still need it for variables stuffed into the
> floating point registers.

That case is fine.  A while ago I split the mechanism in half:

- given a single FP register convert the type into its true form
- the MIPS jungle of combining sub-parts and adjacent FP and integer 
register values

>    Until someone does the right think - add support for values scattered 
>    across registers and memory - hacks should be confined to architecture 
>    specific code.
> 
> But even if someone does add support for values scattered across
> multiple registers and/or memory, we still need the architecture
> method I proposed.  There simply is too much debugging info out there
> that can't express values being scattered across multiple registers.

The stabs reader will need to be modified so that it generates a proper 
location description.  Note that it is STABS centric.  dwarf2 doesn't 
need that mechanism since (presumably) GCC is generating the correct 
info (....).

> And I don't think the hack you proposed is a good idea.  I think it's
> better to add a new architecture method with a clear purpose than
> abuse an existing mechanism for something that it wasn't quite
> intended for.  Even if the architecture method in question would only
> be used by a single target.

This is one of the intended purposes of this mechanism, and as I 
indicated, is needed by MIPS.  Being able to project an arbitrary [debug 
info] view of the registers onto the raw register buffer.

BTW, what happens when there is an attempt to write a long long value? 
GDB again assumes that it can write to contigious registers - the reason 
why REGISTER_BYTE can't be killed.

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 11:22:52AM -0400, Andrew Cagney wrote:
> >   Date: Fri, 25 Apr 2003 23:32:47 -0400
> >   From: Andrew Cagney <ac131313@redhat.com>
> >
> >   > I'm afraid I don't understand, and I still don't see your reasoning
> >   > against this approach.
> >
> >   It isn't necessary, just like register convertible and register 
> >   raw/virtual size; .... that go before it, also were not necessary.  And 
> >   now all these years later, GDB is still yet to expunge.
> >
> >I still don't see how you can get rid of the register convertible
> >stuff.  On the i386 I still need it for variables stuffed into the
> >floating point registers.
> 
> That case is fine.  A while ago I split the mechanism in half:
> 
> - given a single FP register convert the type into its true form
> - the MIPS jungle of combining sub-parts and adjacent FP and integer 
> register values
> 
> >   Until someone does the right think - add support for values scattered 
> >   across registers and memory - hacks should be confined to architecture 
> >   specific code.
> >
> >But even if someone does add support for values scattered across
> >multiple registers and/or memory, we still need the architecture
> >method I proposed.  There simply is too much debugging info out there
> >that can't express values being scattered across multiple registers.
> 
> The stabs reader will need to be modified so that it generates a proper 
> location description.  Note that it is STABS centric.  dwarf2 doesn't 
> need that mechanism since (presumably) GCC is generating the correct 
> info (....).

No, that's incorrect.  GDB wouldn't even be able to find half the value
if GCC was putting out correct information.  We can't fix that until
GDB is ready to not choke on the result.  We will have to handle the
incorrect debug info probably forever.

> >And I don't think the hack you proposed is a good idea.  I think it's
> >better to add a new architecture method with a clear purpose than
> >abuse an existing mechanism for something that it wasn't quite
> >intended for.  Even if the architecture method in question would only
> >be used by a single target.
> 
> This is one of the intended purposes of this mechanism, and as I 
> indicated, is needed by MIPS.  Being able to project an arbitrary [debug 
> info] view of the registers onto the raw register buffer.
> 
> BTW, what happens when there is an attempt to write a long long value? 
> GDB again assumes that it can write to contigious registers - the reason 
> why REGISTER_BYTE can't be killed.

That ugliness could go away too with Mark's introduced method.  GDB
could be fixed to find the next register properly.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

>> The stabs reader will need to be modified so that it generates a proper 
>> location description.  Note that it is STABS centric.  dwarf2 doesn't 
>> need that mechanism since (presumably) GCC is generating the correct 
>> info (....).
> 
> 
> No, that's incorrect.  GDB wouldn't even be able to find half the value
> if GCC was putting out correct information.  We can't fix that until
> GDB is ready to not choke on the result.  We will have to handle the
> incorrect debug info probably forever.

I made two assertions:

- stabs
- dwarf2 (where I included a ``presumably'')

You're saying that both are incorrect?

> This is one of the intended purposes of this mechanism, and as I 
>> indicated, is needed by MIPS.  Being able to project an arbitrary [debug 
>> info] view of the registers onto the raw register buffer.
>> 
>> BTW, what happens when there is an attempt to write a long long value? 
>> GDB again assumes that it can write to contigious registers - the reason 
>> why REGISTER_BYTE can't be killed.
> 
> 
> That ugliness could go away too with Mark's introduced method.  GDB
> could be fixed to find the next register properly.

GDB also uses it to encode offsets into a register.  It also does not 
help the MIPS where the debug register does need to be projected into 
the raw registers.   Why have add more mechanisms when the existing one 
is sufficient.  Focus the effort on fixing the real problem.

BTW, my comment about no names was wrong.  They can be named, that 
restriction should have been removed by the introduction of reggroups.

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 12:08:17PM -0400, Andrew Cagney wrote:
> 
> >>The stabs reader will need to be modified so that it generates a proper 
> >>location description.  Note that it is STABS centric.  dwarf2 doesn't 
> >>need that mechanism since (presumably) GCC is generating the correct 
> >>info (....).
> >
> >
> >No, that's incorrect.  GDB wouldn't even be able to find half the value
> >if GCC was putting out correct information.  We can't fix that until
> >GDB is ready to not choke on the result.  We will have to handle the
> >incorrect debug info probably forever.
> 
> I made two assertions:
> 
> - stabs
> - dwarf2 (where I included a ``presumably'')
> 
> You're saying that both are incorrect?

I guess that depends where you draw the line between the two assertions
:)

 - It's not stabs centric; I imagine that if someone went in to update
mdebug or hp support they'd have the same problem.  Well, maybe not hp. 
That's a real kitchen sink format from what I recall.

 - generated dwarf2 is not correct

but

 - stabs would have to be modified (if we did this fixup in each and
every debug reader, instead of in read_var_value and friends; I see
good arguments both ways)

> >This is one of the intended purposes of this mechanism, and as I 
> >>indicated, is needed by MIPS.  Being able to project an arbitrary [debug 
> >>info] view of the registers onto the raw register buffer.
> >>
> >>BTW, what happens when there is an attempt to write a long long value? 
> >>GDB again assumes that it can write to contigious registers - the reason 
> >>why REGISTER_BYTE can't be killed.
> >
> >
> >That ugliness could go away too with Mark's introduced method.  GDB
> >could be fixed to find the next register properly.
> 
> GDB also uses it to encode offsets into a register.  It also does not 
> help the MIPS where the debug register does need to be projected into 
> the raw registers.   Why have add more mechanisms when the existing one 
> is sufficient.  Focus the effort on fixing the real problem.
> 
> BTW, my comment about no names was wrong.  They can be named, that 
> restriction should have been removed by the introduction of reggroups.

Well, in that case I guess it would work.  Let's do it?

It still feels much more like a hack to me than Mark's approach; I'll
just quietly disagree I suppose.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

>  - stabs would have to be modified (if we did this fixup in each and
> every debug reader, instead of in read_var_value and friends; I see
> good arguments both ways)

It would hopefully be a shared function that those debug readers could 
call.  The important thing is that the core code only knows about one 
mechanism.

>> >This is one of the intended purposes of this mechanism, and as I 
> 
>> >>indicated, is needed by MIPS.  Being able to project an arbitrary [debug 
>> >>info] view of the registers onto the raw register buffer.
>> >>
>> >>BTW, what happens when there is an attempt to write a long long value? 
>> >>GDB again assumes that it can write to contigious registers - the reason 
>> >>why REGISTER_BYTE can't be killed.
> 
>> >
>> >
>> >That ugliness could go away too with Mark's introduced method.  GDB
>> >could be fixed to find the next register properly.
> 
>> 
>> GDB also uses it to encode offsets into a register.  It also does not 
>> help the MIPS where the debug register does need to be projected into 
>> the raw registers.   Why have add more mechanisms when the existing one 
>> is sufficient.  Focus the effort on fixing the real problem.
>> 
>> BTW, my comment about no names was wrong.  They can be named, that 
>> restriction should have been removed by the introduction of reggroups.
> 
> 
> Well, in that case I guess it would work.  Let's do it?
> 
> It still feels much more like a hack to me than Mark's approach; I'll
> just quietly disagree I suppose.

Hmm, I think it will be needed anyway, what happens when the user is 
debugging an i386 mode function (with 32 bit register based long long 
debug info) on an x86-64 target?  That's the MIPS problem, and it needs 
that projection(1).

Also, the next_regnum method assumes that all debug infos use the same 
register sequencing.

A word of caution though, the projection, at the register level works. 
Frame's might need tweaking.  The alternative is to start out with 
deprecated_next_regnum so that it is clear where this stands.

Andrew

(1) dwarf2 debug info makes certain assumptions about the size of registers

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 12:37:12PM -0400, Andrew Cagney wrote:
> Hmm, I think it will be needed anyway, what happens when the user is 
> debugging an i386 mode function (with 32 bit register based long long 
> debug info) on an x86-64 target?  That's the MIPS problem, and it needs 
> that projection(1).
> 
> Also, the next_regnum method assumes that all debug infos use the same 
> register sequencing.
> 
> A word of caution though, the projection, at the register level works. 
> Frame's might need tweaking.  The alternative is to start out with 
> deprecated_next_regnum so that it is clear where this stands.

Here's a discussion piece.  I've implemented your suggestion.  Two
notes:
  - Having done it, I still don't like it :)  Using the register cache
in this way seems very wrong to me.

  - It doesn't work for frames, because by the time
i386_pseudo_register_read is called the regcache is always
current_regcache.  I believe this is because of
legacy_saved_regs_prev_register:

975       if (get_frame_saved_regs (frame) != NULL
976           && get_frame_saved_regs (frame)[regnum] != 0)

I guess doing this much without doing the rest of the conversion makes
the frame machinery quite sad.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-04-28  Daniel Jacobowitz  <drow@mvista.com>

	* i386-tdep.c (debug_register_to_gdb, gdb_to_debug_register)
	(i386_num_debuginfo_regs, FIRST_DEBUGINFO_REGNUM)
	(i386_debuginfo_regnum_p): New.
	(i386_register_name, i386_stab_reg_to_regnum)
	(i386_dwarf_reg_to_regnum, i386_pseudo_register_read)
	(i386_pseudo_register_write, i386_register_reggroup_p): Use
	i386_debuginfo_regnum_p and associates.
	(i386_gdbarch_init): Update num_pseudo_regs.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.139
diff -u -p -r1.139 i386-tdep.c
--- i386-tdep.c	21 Apr 2003 18:55:52 -0000	1.139
+++ i386-tdep.c	28 Apr 2003 19:08:50 -0000
@@ -85,6 +85,43 @@ i386_mmx_regnum_p (int regnum)
 	  && regnum < MM0_REGNUM + i386_num_mmx_regs);
 }
 
+/* Debug info registers.  This is necessary because GDB assumes that
+   multi-word (double, long long) variables are held in consecutively
+   numbered registers.  In the absence of better debug information to
+   explicitly say where the pieces are, arrange a set of extra
+   registers to match GCC's normal allocation order.  These are used
+   by the various REG_TO_REGNUM methods.  */
+
+/* GDB's registers are in the order:
+     eax ecx edx ebx esp ebp esi edi
+   GCC's are in the order:
+     eax edx ecx ebx esi edi ebp esp
+*/
+
+static int debug_register_to_gdb[] =
+{
+  0, 2, 1, 3, 6, 7, 5, 4
+};
+
+static int gdb_to_debug_register[] =
+{
+  0, 2, 1, 3, 7, 6, 4, 5
+};
+
+static const int i386_num_debuginfo_regs =
+  (sizeof (debug_register_to_gdb) / sizeof (debug_register_to_gdb[0]));
+
+#define FIRST_DEBUGINFO_REGNUM (MM0_REGNUM + i386_num_mmx_regs)
+
+static int
+i386_debuginfo_regnum_p (int regnum)
+{
+  if (regnum >= FIRST_DEBUGINFO_REGNUM
+      && regnum < FIRST_DEBUGINFO_REGNUM + i386_num_debuginfo_regs)
+    return 1;
+  return 0;
+}
+
 /* FP register?  */
 
 int
@@ -128,6 +165,12 @@ i386_register_name (int reg)
   if (i386_mmx_regnum_p (reg))
     return i386_mmx_names[reg - MM0_REGNUM];
 
+  if (i386_debuginfo_regnum_p (reg))
+    {
+      int real_regno = debug_register_to_gdb[reg - FIRST_DEBUGINFO_REGNUM];
+      return i386_register_names[real_regno];
+    }
+
   return NULL;
 }
 
@@ -141,7 +184,7 @@ i386_stab_reg_to_regnum (int reg)
   if (reg >= 0 && reg <= 7)
     {
       /* General registers.  */
-      return reg;
+      return FIRST_DEBUGINFO_REGNUM + gdb_to_debug_register[reg];
     }
   else if (reg >= 12 && reg <= 19)
     {
@@ -171,11 +214,14 @@ i386_dwarf_reg_to_regnum (int reg)
 {
   /* The DWARF register numbering includes %eip and %eflags, and
      numbers the floating point registers differently.  */
-  if (reg >= 0 && reg <= 9)
+
+  if (reg >= 0 && reg <= 7)
     {
       /* General registers.  */
-      return reg;
+      return FIRST_DEBUGINFO_REGNUM + gdb_to_debug_register[reg];
     }
+  else if (reg >= 8 && reg <= 9)
+    return reg;
   else if (reg >= 11 && reg <= 18)
     {
       /* Floating-point registers.  */
@@ -1104,6 +1150,11 @@ i386_pseudo_register_read (struct gdbarc
       regcache_raw_read (regcache, fpnum, mmx_buf);
       memcpy (buf, mmx_buf, REGISTER_RAW_SIZE (regnum));
     }
+  else if (i386_debuginfo_regnum_p (regnum))
+    {
+      int real_regnum = debug_register_to_gdb[regnum - FIRST_DEBUGINFO_REGNUM];
+      regcache_raw_read (regcache, real_regnum, buf);
+    }
   else
     regcache_raw_read (regcache, regnum, buf);
 }
@@ -1124,6 +1175,11 @@ i386_pseudo_register_write (struct gdbar
       /* ... Write.  */
       regcache_raw_write (regcache, fpnum, mmx_buf);
     }
+  else if (i386_debuginfo_regnum_p (regnum))
+    {
+      int real_regnum = debug_register_to_gdb[regnum - FIRST_DEBUGINFO_REGNUM];
+      regcache_raw_write (regcache, real_regnum, buf);
+    }
   else
     regcache_raw_write (regcache, regnum, buf);
 }
@@ -1407,6 +1463,8 @@ i386_register_reggroup_p (struct gdbarch
   int fp_regnum_p = (i386_fp_regnum_p (regnum)
 		     || i386_fpc_regnum_p (regnum));
   int mmx_regnum_p = (i386_mmx_regnum_p (regnum));
+  if (i386_debuginfo_regnum_p (regnum))
+    return 0;
   if (group == i386_mmx_reggroup)
     return mmx_regnum_p;
   if (group == i386_sse_reggroup)
@@ -1541,8 +1599,8 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_frame_num_args (gdbarch, frame_num_args_unknown);
   set_gdbarch_pc_in_sigtramp (gdbarch, i386_pc_in_sigtramp);
 
-  /* Wire in the MMX registers.  */
-  set_gdbarch_num_pseudo_regs (gdbarch, i386_num_mmx_regs);
+  /* Wire in the MMX and debuginfo registers.  */
+  set_gdbarch_num_pseudo_regs (gdbarch, i386_num_mmx_regs + i386_num_debuginfo_regs);
   set_gdbarch_pseudo_register_read (gdbarch, i386_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
 

re-ordered i386 regcache

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]

[I changed subjects, this thread is too long]

> On Mon, Apr 28, 2003 at 12:37:12PM -0400, Andrew Cagney wrote:
> 
>> Hmm, I think it will be needed anyway, what happens when the user is 
>> debugging an i386 mode function (with 32 bit register based long long 
>> debug info) on an x86-64 target?  That's the MIPS problem, and it needs 
>> that projection(1).
>> 
>> Also, the next_regnum method assumes that all debug infos use the same 
>> register sequencing.
>> 
>> A word of caution though, the projection, at the register level works. 
>> Frame's might need tweaking.  The alternative is to start out with 
>> deprecated_next_regnum so that it is clear where this stands.
> 
> 
> Here's a discussion piece.  I've implemented your suggestion.  Two
> notes:
>   - Having done it, I still don't like it :)  Using the register cache
> in this way seems very wrong to me.

Got another way of getting  MIPS (32 on 64), i386 on x86-64 (or even 
ia64?), e500 on PPC, sh4 on sh64, ... all working?

>   - It doesn't work for frames, because by the time
> i386_pseudo_register_read is called the regcache is always
> current_regcache.  I believe this is because of
> legacy_saved_regs_prev_register:
> 
> 975       if (get_frame_saved_regs (frame) != NULL
> 976           && get_frame_saved_regs (frame)[regnum] != 0)
> 
> I guess doing this much without doing the rest of the conversion makes
> the frame machinery quite sad.

Should there be a frame equivalent to the regcache's cooked->raw 
projection?  Should the read side of the cooked->raw projection be moved 
to the frame?

Note that things like the m68hc11 some of the cooked registers are 
mapped onto memory so the cooked->raw writes would likely still need to 
remain.

Andrew

[-- Attachment #2: mailbox-message://ac131313@movemail/fsf/gdb/patches#3703853 --]
[-- Type: message/rfc822, Size: 9464 bytes --]

From: Daniel Jacobowitz <drow@mvista.com>
To: Andrew Cagney <ac131313@redhat.com>
Cc: Mark Kettenis <kettenis@chello.nl>, colins@google.com, gdb-patches@sources.redhat.com
Subject: Re: patch for printing 64-bit values in i386 registers; STABS format
Date: Mon, 28 Apr 2003 15:25:07 -0400
Message-ID: <20030428192506.GA11978@nevyn.them.org>

On Mon, Apr 28, 2003 at 12:37:12PM -0400, Andrew Cagney wrote:
> Hmm, I think it will be needed anyway, what happens when the user is 
> debugging an i386 mode function (with 32 bit register based long long 
> debug info) on an x86-64 target?  That's the MIPS problem, and it needs 
> that projection(1).
> 
> Also, the next_regnum method assumes that all debug infos use the same 
> register sequencing.
> 
> A word of caution though, the projection, at the register level works. 
> Frame's might need tweaking.  The alternative is to start out with 
> deprecated_next_regnum so that it is clear where this stands.

Here's a discussion piece.  I've implemented your suggestion.  Two
notes:
  - Having done it, I still don't like it :)  Using the register cache
in this way seems very wrong to me.

  - It doesn't work for frames, because by the time
i386_pseudo_register_read is called the regcache is always
current_regcache.  I believe this is because of
legacy_saved_regs_prev_register:

975       if (get_frame_saved_regs (frame) != NULL
976           && get_frame_saved_regs (frame)[regnum] != 0)

I guess doing this much without doing the rest of the conversion makes
the frame machinery quite sad.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-04-28  Daniel Jacobowitz  <drow@mvista.com>

	* i386-tdep.c (debug_register_to_gdb, gdb_to_debug_register)
	(i386_num_debuginfo_regs, FIRST_DEBUGINFO_REGNUM)
	(i386_debuginfo_regnum_p): New.
	(i386_register_name, i386_stab_reg_to_regnum)
	(i386_dwarf_reg_to_regnum, i386_pseudo_register_read)
	(i386_pseudo_register_write, i386_register_reggroup_p): Use
	i386_debuginfo_regnum_p and associates.
	(i386_gdbarch_init): Update num_pseudo_regs.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.139
diff -u -p -r1.139 i386-tdep.c
--- i386-tdep.c	21 Apr 2003 18:55:52 -0000	1.139
+++ i386-tdep.c	28 Apr 2003 19:08:50 -0000
@@ -85,6 +85,43 @@ i386_mmx_regnum_p (int regnum)
 	  && regnum < MM0_REGNUM + i386_num_mmx_regs);
 }
 
+/* Debug info registers.  This is necessary because GDB assumes that
+   multi-word (double, long long) variables are held in consecutively
+   numbered registers.  In the absence of better debug information to
+   explicitly say where the pieces are, arrange a set of extra
+   registers to match GCC's normal allocation order.  These are used
+   by the various REG_TO_REGNUM methods.  */
+
+/* GDB's registers are in the order:
+     eax ecx edx ebx esp ebp esi edi
+   GCC's are in the order:
+     eax edx ecx ebx esi edi ebp esp
+*/
+
+static int debug_register_to_gdb[] =
+{
+  0, 2, 1, 3, 6, 7, 5, 4
+};
+
+static int gdb_to_debug_register[] =
+{
+  0, 2, 1, 3, 7, 6, 4, 5
+};
+
+static const int i386_num_debuginfo_regs =
+  (sizeof (debug_register_to_gdb) / sizeof (debug_register_to_gdb[0]));
+
+#define FIRST_DEBUGINFO_REGNUM (MM0_REGNUM + i386_num_mmx_regs)
+
+static int
+i386_debuginfo_regnum_p (int regnum)
+{
+  if (regnum >= FIRST_DEBUGINFO_REGNUM
+      && regnum < FIRST_DEBUGINFO_REGNUM + i386_num_debuginfo_regs)
+    return 1;
+  return 0;
+}
+
 /* FP register?  */
 
 int
@@ -128,6 +165,12 @@ i386_register_name (int reg)
   if (i386_mmx_regnum_p (reg))
     return i386_mmx_names[reg - MM0_REGNUM];
 
+  if (i386_debuginfo_regnum_p (reg))
+    {
+      int real_regno = debug_register_to_gdb[reg - FIRST_DEBUGINFO_REGNUM];
+      return i386_register_names[real_regno];
+    }
+
   return NULL;
 }
 
@@ -141,7 +184,7 @@ i386_stab_reg_to_regnum (int reg)
   if (reg >= 0 && reg <= 7)
     {
       /* General registers.  */
-      return reg;
+      return FIRST_DEBUGINFO_REGNUM + gdb_to_debug_register[reg];
     }
   else if (reg >= 12 && reg <= 19)
     {
@@ -171,11 +214,14 @@ i386_dwarf_reg_to_regnum (int reg)
 {
   /* The DWARF register numbering includes %eip and %eflags, and
      numbers the floating point registers differently.  */
-  if (reg >= 0 && reg <= 9)
+
+  if (reg >= 0 && reg <= 7)
     {
       /* General registers.  */
-      return reg;
+      return FIRST_DEBUGINFO_REGNUM + gdb_to_debug_register[reg];
     }
+  else if (reg >= 8 && reg <= 9)
+    return reg;
   else if (reg >= 11 && reg <= 18)
     {
       /* Floating-point registers.  */
@@ -1104,6 +1150,11 @@ i386_pseudo_register_read (struct gdbarc
       regcache_raw_read (regcache, fpnum, mmx_buf);
       memcpy (buf, mmx_buf, REGISTER_RAW_SIZE (regnum));
     }
+  else if (i386_debuginfo_regnum_p (regnum))
+    {
+      int real_regnum = debug_register_to_gdb[regnum - FIRST_DEBUGINFO_REGNUM];
+      regcache_raw_read (regcache, real_regnum, buf);
+    }
   else
     regcache_raw_read (regcache, regnum, buf);
 }
@@ -1124,6 +1175,11 @@ i386_pseudo_register_write (struct gdbar
       /* ... Write.  */
       regcache_raw_write (regcache, fpnum, mmx_buf);
     }
+  else if (i386_debuginfo_regnum_p (regnum))
+    {
+      int real_regnum = debug_register_to_gdb[regnum - FIRST_DEBUGINFO_REGNUM];
+      regcache_raw_write (regcache, real_regnum, buf);
+    }
   else
     regcache_raw_write (regcache, regnum, buf);
 }
@@ -1407,6 +1463,8 @@ i386_register_reggroup_p (struct gdbarch
   int fp_regnum_p = (i386_fp_regnum_p (regnum)
 		     || i386_fpc_regnum_p (regnum));
   int mmx_regnum_p = (i386_mmx_regnum_p (regnum));
+  if (i386_debuginfo_regnum_p (regnum))
+    return 0;
   if (group == i386_mmx_reggroup)
     return mmx_regnum_p;
   if (group == i386_sse_reggroup)
@@ -1541,8 +1599,8 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_frame_num_args (gdbarch, frame_num_args_unknown);
   set_gdbarch_pc_in_sigtramp (gdbarch, i386_pc_in_sigtramp);
 
-  /* Wire in the MMX registers.  */
-  set_gdbarch_num_pseudo_regs (gdbarch, i386_num_mmx_regs);
+  /* Wire in the MMX and debuginfo registers.  */
+  set_gdbarch_num_pseudo_regs (gdbarch, i386_num_mmx_regs + i386_num_debuginfo_regs);
   set_gdbarch_pseudo_register_read (gdbarch, i386_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
 

Re: re-ordered i386 regcache

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 06:52:05PM -0400, Andrew Cagney wrote:
> [I changed subjects, this thread is too long]
> 
> >On Mon, Apr 28, 2003 at 12:37:12PM -0400, Andrew Cagney wrote:
> >
> >>Hmm, I think it will be needed anyway, what happens when the user is 
> >>debugging an i386 mode function (with 32 bit register based long long 
> >>debug info) on an x86-64 target?  That's the MIPS problem, and it needs 
> >>that projection(1).
> >>
> >>Also, the next_regnum method assumes that all debug infos use the same 
> >>register sequencing.
> >>
> >>A word of caution though, the projection, at the register level works. 
> >>Frame's might need tweaking.  The alternative is to start out with 
> >>deprecated_next_regnum so that it is clear where this stands.
> >
> >
> >Here's a discussion piece.  I've implemented your suggestion.  Two
> >notes:
> >  - Having done it, I still don't like it :)  Using the register cache
> >in this way seems very wrong to me.
> 
> Got another way of getting  MIPS (32 on 64), i386 on x86-64 (or even 
> ia64?), e500 on PPC, sh4 on sh64, ... all working?

What problem besides the REGISTER_BYTE() problem are you solving with
this mechanism?

Of the above, at least the e500 issues I consider to be completely
different; that's dealing with a very particular set of debug info that
only has part of the register.  That's a handy thing to solve in the
register cache.  Besides, it doesn't play sequential-register-numbering
tricks.  That was the whole point - the other numbers are way off in
the 1200 range.

I'd say the MIPS/i386 issues are also more like e500 than like this
debug info ordering problem that we're talking about here.

I'm not dismissing the value of the powerful pseudo technique that
you've put together.  It's really handy.  I just don't think it's the
answer to this problem.

> >  - It doesn't work for frames, because by the time
> >i386_pseudo_register_read is called the regcache is always
> >current_regcache.  I believe this is because of
> >legacy_saved_regs_prev_register:
> >
> >975       if (get_frame_saved_regs (frame) != NULL
> >976           && get_frame_saved_regs (frame)[regnum] != 0)
> >
> >I guess doing this much without doing the rest of the conversion makes
> >the frame machinery quite sad.
> 
> Should there be a frame equivalent to the regcache's cooked->raw 
> projection?  Should the read side of the cooked->raw projection be moved 
> to the frame?
> 
> Note that things like the m68hc11 some of the cooked registers are 
> mapped onto memory so the cooked->raw writes would likely still need to 
> remain.

Hmm, certainly something will have to change.  Invoking
gdbarch_pseudo_register_read on the current regcache when we're
actually several frames away doesn't respond right.  It seems to me
that moving the logic such that gdbarch_pseudo_register_read takes a
frame parameter might work better, but I'm not sure of the
implications.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: re-ordered i386 regcache

[Andrew Cagney]

>> >Here's a discussion piece.  I've implemented your suggestion.  Two
>> >notes:
>> >  - Having done it, I still don't like it :)  Using the register cache
>> >in this way seems very wrong to me.
> 
>> 
>> Got another way of getting  MIPS (32 on 64), i386 on x86-64 (or even 
>> ia64?), e500 on PPC, sh4 on sh64, ... all working?
> 
> 
> What problem besides the REGISTER_BYTE() problem are you solving with
> this mechanism?

REGISTER_BYTE() is a side issue.  It is eliminated with proper location 
support.

> Of the above, at least the e500 issues I consider to be completely
> different; that's dealing with a very particular set of debug info that
> only has part of the register.  That's a handy thing to solve in the
> register cache.  Besides, it doesn't play sequential-register-numbering
> tricks.  That was the whole point - the other numbers are way off in
> the 1200 range.
> 
> I'd say the MIPS/i386 issues are also more like e500 than like this
> debug info ordering problem that we're talking about here.

(are you sure you're refering to the correct architectures here?)

> I'm not dismissing the value of the powerful pseudo technique that
> you've put together.  It's really handy.  I just don't think it's the
> answer to this problem.

Going back to an earlier point it contains an i386 specific problem to 
the i386.  That way yet another hack doesn't get in the core code. 
However, the knowledge of needing to handle that case does.

>> Should there be a frame equivalent to the regcache's cooked->raw 
>> projection?  Should the read side of the cooked->raw projection be moved 
>> to the frame?
>> 
>> Note that things like the m68hc11 some of the cooked registers are 
>> mapped onto memory so the cooked->raw writes would likely still need to 
>> remain.
> 
> 
> Hmm, certainly something will have to change.  Invoking
> gdbarch_pseudo_register_read on the current regcache when we're
> actually several frames away doesn't respond right.  It seems to me
> that moving the logic such that gdbarch_pseudo_register_read takes a
> frame parameter might work better, but I'm not sure of the
> implications.

The e500 and sh both handle this by doing the maps on a per-frame basis.

Andrew

Re: re-ordered i386 regcache

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 10:15:03PM -0400, Andrew Cagney wrote:
> 
> >>>Here's a discussion piece.  I've implemented your suggestion.  Two
> >>>notes:
> >>>  - Having done it, I still don't like it :)  Using the register cache
> >>>in this way seems very wrong to me.
> >
> >>
> >>Got another way of getting  MIPS (32 on 64), i386 on x86-64 (or even 
> >>ia64?), e500 on PPC, sh4 on sh64, ... all working?
> >
> >
> >What problem besides the REGISTER_BYTE() problem are you solving with
> >this mechanism?
> 
> REGISTER_BYTE() is a side issue.  It is eliminated with proper location 
> support.

Sure.  That's why I'm trying to get a grasp on what you think the real
issue is.

> >Of the above, at least the e500 issues I consider to be completely
> >different; that's dealing with a very particular set of debug info that
> >only has part of the register.  That's a handy thing to solve in the
> >register cache.  Besides, it doesn't play sequential-register-numbering
> >tricks.  That was the whole point - the other numbers are way off in
> >the 1200 range.
> >
> >I'd say the MIPS/i386 issues are also more like e500 than like this
> >debug info ordering problem that we're talking about here.
> 
> (are you sure you're refering to the correct architectures here?)

Absolutely.  I know the e500 situation (limited form of DW_OP_piece for
64-bit registers), and both MIPS/MIPS and x86-64/i386 are trying to
expose the same register in multiple ways.

> >I'm not dismissing the value of the powerful pseudo technique that
> >you've put together.  It's really handy.  I just don't think it's the
> >answer to this problem.
> 
> Going back to an earlier point it contains an i386 specific problem to 
> the i386.  That way yet another hack doesn't get in the core code. 
> However, the knowledge of needing to handle that case does.

If the choice is:
  - contain an i386 specific problem to i386 specific code by using a
    hack
  - provide a general, well-defined mechanism that at least now we only
    need for i386
then I'm all for plan B.

> >>Should there be a frame equivalent to the regcache's cooked->raw 
> >>projection?  Should the read side of the cooked->raw projection be moved 
> >>to the frame?
> >>
> >>Note that things like the m68hc11 some of the cooked registers are 
> >>mapped onto memory so the cooked->raw writes would likely still need to 
> >>remain.
> >
> >
> >Hmm, certainly something will have to change.  Invoking
> >gdbarch_pseudo_register_read on the current regcache when we're
> >actually several frames away doesn't respond right.  It seems to me
> >that moving the logic such that gdbarch_pseudo_register_read takes a
> >frame parameter might work better, but I'm not sure of the
> >implications.
> 
> The e500 and sh both handle this by doing the maps on a per-frame basis.

Not familiar with the SH code... skimming the e500 code I can't quite
tell how it works, but it looks as if the pseudo registers are found in
get_frame_saved_regs and e500_pseudo_register_read will only behave
correctly for the topmost frame.  Which makes sense given the way we
handle e500 regs.

I suppose I could mark both versions of %eax saved in i386's
frame_get_saved_regs to fix the immediate problem...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: re-ordered i386 regcache

[Andrew Cagney]

>> >Of the above, at least the e500 issues I consider to be completely
>> >different; that's dealing with a very particular set of debug info that
>> >only has part of the register.  That's a handy thing to solve in the
>> >register cache.  Besides, it doesn't play sequential-register-numbering
>> >tricks.  That was the whole point - the other numbers are way off in
>> >the 1200 range.
>> >
>> >I'd say the MIPS/i386 issues are also more like e500 than like this
>> >debug info ordering problem that we're talking about here.
> 
>> 
>> (are you sure you're refering to the correct architectures here?)
> 
> 
> Absolutely.

The MIPS/i386 in the above be interpreted as i386 VS long long and not 
i386/x86-64.  The reword below clarified it.

>  I know the e500 situation (limited form of DW_OP_piece for
> 64-bit registers), and both MIPS/MIPS and x86-64/i386 are trying to
> expose the same register in multiple ways.

All three require a projection such that, assumed contigious, debug info 
registers project onto raw registers.  Isn't this what i386 has?

>>I'm not dismissing the value of the powerful pseudo technique that
>> >you've put together.  It's really handy.  I just don't think it's the
>> >answer to this problem.
> 
>> 
>> Going back to an earlier point it contains an i386 specific problem to 
>> the i386.  That way yet another hack doesn't get in the core code. 
>> However, the knowledge of needing to handle that case does.
> 
> 
> If the choice is:
>   - contain an i386 specific problem to i386 specific code by using a
>     hack

You mean that the cooked->raw mechanism is a hack?

>   - provide a general, well-defined mechanism that at least now we only
>     need for i386

(and I'm calling it a quick and tempoary hack, because it isn't the 
final solution)

> then I'm all for plan B.

Seems we've each taken to calling the alternative solution ``a hack'' :-(

GDB can't afford to accumulate yet more mechanisms just on the off 
chance that a second architecture might just happen to need it.  This is 
how GDB ended up with so many architecture methods used by 1 (then zero) 
targets (what's the harm in #define ...; #ifdef ...?).  Instead, where 
possible, target dependand code should start out by using existing 
mechanisms; while the core is extended to use a more complete solution.

> I suppose I could mark both versions of %eax saved in i386's
> frame_get_saved_regs to fix the immediate problem...

That is what was done (minus possible missed edge cases).

Being able to project cooked onto raw registers at the frame level, 
should simplify this.

Andrew

Re: re-ordered i386 regcache

[Daniel Jacobowitz]

On Tue, Apr 29, 2003 at 10:30:03AM -0400, Andrew Cagney wrote:
> 
> >>>Of the above, at least the e500 issues I consider to be completely
> >>>different; that's dealing with a very particular set of debug info that
> >>>only has part of the register.  That's a handy thing to solve in the
> >>>register cache.  Besides, it doesn't play sequential-register-numbering
> >>>tricks.  That was the whole point - the other numbers are way off in
> >>>the 1200 range.
> >>>
> >>>I'd say the MIPS/i386 issues are also more like e500 than like this
> >>>debug info ordering problem that we're talking about here.
> >
> >>
> >>(are you sure you're refering to the correct architectures here?)
> >
> >
> >Absolutely.
> 
> The MIPS/i386 in the above be interpreted as i386 VS long long and not 
> i386/x86-64.  The reword below clarified it.
> 
> > I know the e500 situation (limited form of DW_OP_piece for
> >64-bit registers), and both MIPS/MIPS and x86-64/i386 are trying to
> >expose the same register in multiple ways.
> 
> All three require a projection such that, assumed contigious, debug info 
> registers project onto raw registers.  Isn't this what i386 has?

I don't think so.  All three need a view of registers slightly
different from the "normal" one, but they don't have "assumed
contiguous debug info registers".  In fact in e500 they're assumed
not-contiguous.

> >>I'm not dismissing the value of the powerful pseudo technique that
> >>>you've put together.  It's really handy.  I just don't think it's the
> >>>answer to this problem.
> >
> >>
> >>Going back to an earlier point it contains an i386 specific problem to 
> >>the i386.  That way yet another hack doesn't get in the core code. 
> >>However, the knowledge of needing to handle that case does.
> >
> >
> >If the choice is:
> >  - contain an i386 specific problem to i386 specific code by using a
> >    hack
> 
> You mean that the cooked->raw mechanism is a hack?

As I said above, the mechanism isn't.  This use of it is.

> >  - provide a general, well-defined mechanism that at least now we only
> >    need for i386
> 
> (and I'm calling it a quick and tempoary hack, because it isn't the 
> final solution)

Why not?  This is my view of the road:
  - If we have multiple parts reported for a variable's location, use
    that.
  - Otherwise if the variable fits in the reported location, just use
    that.
  - Otherwise, use a defined mechanism to guess where the next part is.
    Instead of assuming "oh, it's a register, the next part must be
    in register <this + 1>", which isn't true.  Instead of fudging the
    regcache using cooked->raw in order to make this invalid assumption
    appear true.

> GDB can't afford to accumulate yet more mechanisms just on the off 
> chance that a second architecture might just happen to need it.  This is 
> how GDB ended up with so many architecture methods used by 1 (then zero) 
> targets (what's the harm in #define ...; #ifdef ...?).  Instead, where 
> possible, target dependand code should start out by using existing 
> mechanisms; while the core is extended to use a more complete solution.

The "more complete solution" won't solve the problem for the debug info
we have today.  That'll be around for a very long time.  I'm more
worried about the fragility of doing complex things with the regcache,
which will have a maintenance cost every time we (you?) work on the
regcache.  When we could just do a simple method with low to no
maintenance cost.

Why _not_ acquire this mechanism?  There's only a problem with buildup
of not-thought-out and not-well-documented mechanisms, and this doesn't
have to be either.

> >I suppose I could mark both versions of %eax saved in i386's
> >frame_get_saved_regs to fix the immediate problem...
> 
> That is what was done (minus possible missed edge cases).
> 
> Being able to project cooked onto raw registers at the frame level, 
> should simplify this.

OK.  If I ever get frustrated/bored enough, I'll try to fix the patch I
posted.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: re-ordered i386 regcache

[Andrew Cagney]

> All three require a projection such that, assumed contigious, debug info 
>> registers project onto raw registers.  Isn't this what i386 has?
> 
> 
> I don't think so.  All three need a view of registers slightly
> different from the "normal" one, but they don't have "assumed
> contiguous debug info registers".  In fact in e500 they're assumed
> not-contiguous.

Please study the code.  A 32 bit MIPS ABI on a 64 bit MIPS represents 
long long as two 32 bit sub-parts of two 64 bit registers; the two 32 
bit parts are assumed to be adjacent.

The adjacent assumption will eventually be lifted, but that will be done 
by someone implementing something like location descriptions.

Andrew

Re: re-ordered i386 regcache

[Daniel Jacobowitz]

On Tue, Apr 29, 2003 at 11:20:39AM -0400, Andrew Cagney wrote:
> >All three require a projection such that, assumed contigious, debug info 
> >>registers project onto raw registers.  Isn't this what i386 has?
> >
> >
> >I don't think so.  All three need a view of registers slightly
> >different from the "normal" one, but they don't have "assumed
> >contiguous debug info registers".  In fact in e500 they're assumed
> >not-contiguous.
> 
> Please study the code.  A 32 bit MIPS ABI on a 64 bit MIPS represents 
> long long as two 32 bit sub-parts of two 64 bit registers; the two 32 
> bit parts are assumed to be adjacent.
> 
> The adjacent assumption will eventually be lifted, but that will be done 
> by someone implementing something like location descriptions.

I posit that this is the exact same problem as the i386 is currently
facing.

Remember, location descriptions come from the debug reader, not from
the target.  Where do you think these hypothetical descriptors to tell
us which two registers the long long value occupies will come from?
Current debug info won't give them to us.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Re: patch for printing 64-bit values in i386 registers; STABS format

[Colin Smith]

> > BTW, what happens when there is an attempt to write a long long value?
> > GDB again assumes that it can write to contigious registers - the reason
> > why REGISTER_BYTE can't be killed.
>
> That ugliness could go away too with Mark's introduced method.  GDB
> could be fixed to find the next register properly.

I forgot about the write case.  It seems to route through write_register_bytes,
which is unfortunate because the register order problem is tougher to solve
there.  It might make more sense to avoid using write_register_bytes up in 
value_assign, having it iterate over the registers.

Mark, could you pass along your patch?  I might be able to use it.  

Thanks!

_____
colin

> --
> Daniel Jacobowitz
> MontaVistaSoftware                         Debian GNU/Linux Developer
>

Re: patch for printing 64-bit values in i386 registers; STABS format

[Mark Kettenis]

   Date: Fri, 25 Apr 2003 18:29:02 -0400
   From: Andrew Cagney <ac131313@redhat.com>
   User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030223
   X-Accept-Language: en-us, en
   Cc: Mark Kettenis <kettenis@chello.nl>, colins@google.com,
      gdb-patches@sources.redhat.com
   Content-Type: text/plain; charset=us-ascii; format=flowed

   > On Fri, Apr 25, 2003 at 11:21:13PM +0200, Mark Kettenis wrote:
   > 
   >>    Date: Thu, 24 Apr 2003 20:27:44 -0400
   >>    From: Daniel Jacobowitz <drow@mvista.com>
   >> 
   >>    Hey, Mark, this sounds very much like a change you proposed.  What ever
   >>    happened to that patch?
   >> 
   >> It's still happily sitting in my tree :-(.  There didn't seem to be
   >> any consensus on whether making this change was a good idea.  I still
   >> think it is.  It's an improvement for the majority of our users, and
   >> it isn't making things worse for others.  Do you think I should
   >> re-submit my patch?
   > 
   > 
   > I do, definitely.

   FYI,

   It's possible to fix this without adding an architecture method, or 
   implementing location expressions (the penny just dropped).  The basic 
   problem is the same as for the MIPS - need a custom register area.  Hence:

   - define a sequence of nameless cooked ([NUM_REGS .. 
   NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
   like them
   - modify the existing stabs_regnum_to_regnum to map the messed up 
   registers onto those values

Ugh, Yuck!  Yes it works, but isn't this a terrible hack?  Oh and
using nameless cooked registers means that

   info address variable

no longer prints the right thing if variable lives in a register.

Mark

Re: patch for printing 64-bit values in i386 registers; STABS format

[Andrew Cagney]

>    It's possible to fix this without adding an architecture method, or 
>    implementing location expressions (the penny just dropped).  The basic 
>    problem is the same as for the MIPS - need a custom register area.  Hence:
> 
>    - define a sequence of nameless cooked ([NUM_REGS .. 
>    NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
>    like them
>    - modify the existing stabs_regnum_to_regnum to map the messed up 
>    registers onto those values
> 
> Ugh, Yuck!  Yes it works, but isn't this a terrible hack?  Oh and
> using nameless cooked registers means that
> 
>    info address variable
> 
> no longer prints the right thing if variable lives in a register.

Hmm, so that's the command I can never remember.  Fortunatly, I think 
the `unnamed' restriction is removed.  They can be named as GDB should 
now be relying on reggroups to determine which registers are valid where.

Andrew

Re: patch for printing 64-bit values in i386 registers; STABS format

[Daniel Jacobowitz]

On Mon, Apr 28, 2003 at 12:09:53PM -0400, Andrew Cagney wrote:
> 
> >   It's possible to fix this without adding an architecture method, or 
> >   implementing location expressions (the penny just dropped).  The basic 
> >   problem is the same as for the MIPS - need a custom register area.  
> >   Hence:
> >
> >   - define a sequence of nameless cooked ([NUM_REGS .. 
> >   NUM_REGS+NUM_PSEUDO_REGS) range) registers ordered the way stabs would 
> >   like them
> >   - modify the existing stabs_regnum_to_regnum to map the messed up 
> >   registers onto those values
> >
> >Ugh, Yuck!  Yes it works, but isn't this a terrible hack?  Oh and
> >using nameless cooked registers means that
> >
> >   info address variable
> >
> >no longer prints the right thing if variable lives in a register.
> 
> Hmm, so that's the command I can never remember.  Fortunatly, I think 
> the `unnamed' restriction is removed.  They can be named as GDB should 
> now be relying on reggroups to determine which registers are valid where.

Nowadays print &variable will do it also.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

RE: patch for printing 64-bit values in i386 registers; STABS format

[Colin Smith]

... I should have mentioned that this patch was developed on GDB 5.3
for RedHat 7.2, and that I've run DejaGNU on the GDB before and after.
The results get better with this patch.

_____
colin