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

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

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