[PATCH] Fix sparc-*-linux register fetching/storing

[PATCH] Fix sparc-*-linux register fetching/storing

[Jakub Jelinek]

Hi!

On sparc-*-linux, bfd automatically supports both 32bit and 64bit ABI and
thus CORE_ADDR is 64bit type. Unfortunately, this means %l0-%i7 registers
are read from incorrect place (and stored too), particularly from caller's
instruction chain. This means even simple commands like next or bt don't
work at all.
Ok to commit?

2001-11-23  Jakub Jelinek  <jakub@redhat.com>

	* sparc-nat.c (fetch_inferior_registers): Don't rely
	on CORE_ADDR being 32-bit.
	(store_inferior_registers): Likewise.

--- gdb/sparc-nat.c.jj	Sun Oct 14 19:15:14 2001
+++ gdb/sparc-nat.c	Fri Nov 23 16:45:58 2001
@@ -120,15 +120,15 @@ fetch_inferior_registers (int regno)
      all (16 ptrace calls!) if we really need them.  */
   if (regno == -1)
     {
-      target_read_memory (*(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)],
-			  &registers[REGISTER_BYTE (L0_REGNUM)],
+      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
+      target_read_memory (sp, &registers[REGISTER_BYTE (L0_REGNUM)],
 			  16 * REGISTER_RAW_SIZE (L0_REGNUM));
       for (i = L0_REGNUM; i <= I7_REGNUM; i++)
 	register_valid[i] = 1;
     }
   else if (regno >= L0_REGNUM && regno <= I7_REGNUM)
     {
-      CORE_ADDR sp = *(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)];
+      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
       i = REGISTER_BYTE (regno);
       if (register_valid[regno])
 	printf_unfiltered ("register %d valid and read\n", regno);
@@ -190,7 +190,7 @@ store_inferior_registers (int regno)
 
   if (wanna_store & STACK_REGS)
     {
-      CORE_ADDR sp = *(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)];
+      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
 
       if (regno < 0 || regno == SP_REGNUM)
 	{

	Jakub

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Fri, Nov 23, 2001 at 03:42:21PM +0100, Jakub Jelinek wrote:
> Hi!
> 
> On sparc-*-linux, bfd automatically supports both 32bit and 64bit ABI and
> thus CORE_ADDR is 64bit type. Unfortunately, this means %l0-%i7 registers
> are read from incorrect place (and stored too), particularly from caller's
> instruction chain. This means even simple commands like next or bt don't
> work at all.
> Ok to commit?

After this patch, Sparc still seems to be a little badly off
(particularly in calling inferior functions), but much better than
before.  I'm a little confused about it though; I don't think it's
correct.

> -      target_read_memory (*(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)],
> -			  &registers[REGISTER_BYTE (L0_REGNUM)],
> +      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
> +      target_read_memory (sp, &registers[REGISTER_BYTE (L0_REGNUM)],

How was this going wrong exactly?  We don't have any assurance that I
can think of that the stack will always be under the 32-bit mark in a
true 64-bit userland.

Is the entry in registers[] only four bytes long?  If so, it seems that
using regcache_collect here is the way to go.  For Sparc, which doesn't
sign-extend the way MIPS does, collecting four bytes out of the
register cache should be fine.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Jakub Jelinek]

On Sun, Nov 25, 2001 at 02:01:47AM -0500, Daniel Jacobowitz wrote:
> On Fri, Nov 23, 2001 at 03:42:21PM +0100, Jakub Jelinek wrote:
> > Hi!
> > 
> > On sparc-*-linux, bfd automatically supports both 32bit and 64bit ABI and
> > thus CORE_ADDR is 64bit type. Unfortunately, this means %l0-%i7 registers
> > are read from incorrect place (and stored too), particularly from caller's
> > instruction chain. This means even simple commands like next or bt don't
> > work at all.
> > Ok to commit?
> 
> After this patch, Sparc still seems to be a little badly off
> (particularly in calling inferior functions), but much better than
> before.  I'm a little confused about it though; I don't think it's
> correct.

I was fixing what I saw (and the next thing I got hit was a ld bug that
cleared some .stab values in shared libs, so I had to recompile all shared
libs).

> > -      target_read_memory (*(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)],
> > -			  &registers[REGISTER_BYTE (L0_REGNUM)],
> > +      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
> > +      target_read_memory (sp, &registers[REGISTER_BYTE (L0_REGNUM)],
> 
> How was this going wrong exactly?  We don't have any assurance that I
> can think of that the stack will always be under the 32-bit mark in a
> true 64-bit userland.

The code in sparc-nat.c is not able to do 64bit userland.
Solaris I believe uses completely different code, SunOS cannot go 64bit and
for Linux it would have to use PT_GETREGS64 and the like.
Actually, Dave Miller and myself used to have a patch for this which made
gdb work at least a little bit with 64bit binaries, but it was not combo
32/64bit gdb which would require far more work (with most complicated stuff
like solib.c for 32bit and 64bit in the same binary).

> Is the entry in registers[] only four bytes long?  If so, it seems that
> using regcache_collect here is the way to go.  For Sparc, which doesn't
> sign-extend the way MIPS does, collecting four bytes out of the
> register cache should be fine.

I don't understand how regcache_collect would help here, since the assertion
REGISTER_RAW_SIZE() == 4 would stay. That's just trading a dereference with
a memcpy to an int variable.

	Jakub

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Sun, Nov 25, 2001 at 11:32:01AM -0500, Jakub Jelinek wrote:
> I was fixing what I saw (and the next thing I got hit was a ld bug that
> cleared some .stab values in shared libs, so I had to recompile all shared
> libs).

Ugh.  Not my fault, I hope :)

> > > -      target_read_memory (*(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)],
> > > -			  &registers[REGISTER_BYTE (L0_REGNUM)],
> > > +      CORE_ADDR sp = *(unsigned int *) & registers[REGISTER_BYTE (SP_REGNUM)];
> > > +      target_read_memory (sp, &registers[REGISTER_BYTE (L0_REGNUM)],
> > 
> > How was this going wrong exactly?  We don't have any assurance that I
> > can think of that the stack will always be under the 32-bit mark in a
> > true 64-bit userland.
> 
> The code in sparc-nat.c is not able to do 64bit userland.
> Solaris I believe uses completely different code, SunOS cannot go 64bit and
> for Linux it would have to use PT_GETREGS64 and the like.
> Actually, Dave Miller and myself used to have a patch for this which made
> gdb work at least a little bit with 64bit binaries, but it was not combo
> 32/64bit gdb which would require far more work (with most complicated stuff
> like solib.c for 32bit and 64bit in the same binary).
> 
> > Is the entry in registers[] only four bytes long?  If so, it seems that
> > using regcache_collect here is the way to go.  For Sparc, which doesn't
> > sign-extend the way MIPS does, collecting four bytes out of the
> > register cache should be fine.
> 
> I don't understand how regcache_collect would help here, since the assertion
> REGISTER_RAW_SIZE() == 4 would stay. That's just trading a dereference with
> a memcpy to an int variable.

Well, regcache_collect is the only approved interface to the contents
of registers[] for one thing.  It would also prevent the need for the
cast (although you'd have to clear the upper half of the variable
first and make sure to stuff it into the low bytes since we're
big-endian.  Ew.).

Andrew?  Do we need to have a regcache_collect_core_addr, to sign
extend and shift appropriately for each architecture?


-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Jakub Jelinek]

On Sun, Nov 25, 2001 at 11:54:46AM -0500, Daniel Jacobowitz wrote:
> Well, regcache_collect is the only approved interface to the contents
> of registers[] for one thing.

Even in the routine where half of it is direct registers[] setting?
If that is rewritten, surely it makes sense to access sp that way too.

> It would also prevent the need for the
> cast (although you'd have to clear the upper half of the variable
> first and make sure to stuff it into the low bytes since we're
> big-endian.  Ew.).
> 
> Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> extend and shift appropriately for each architecture?

Without such routine the code would be very ugly:
CORE_ADDR sp = 0;
regcache_collect (SP_REGNUM, ((char *)&sp) + sizeof(CORE_ADDR) - REGISTER_RAW_SIZE(SP_REGNUM));

	Jakub

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Sun, Nov 25, 2001 at 12:02:50PM -0500, Jakub Jelinek wrote:
> On Sun, Nov 25, 2001 at 11:54:46AM -0500, Daniel Jacobowitz wrote:
> > Well, regcache_collect is the only approved interface to the contents
> > of registers[] for one thing.
> 
> Even in the routine where half of it is direct registers[] setting?
> If that is rewritten, surely it makes sense to access sp that way too.

Yes.  Accessing registers[] is deprecated.

> > It would also prevent the need for the
> > cast (although you'd have to clear the upper half of the variable
> > first and make sure to stuff it into the low bytes since we're
> > big-endian.  Ew.).
> > 
> > Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> > extend and shift appropriately for each architecture?
> 
> Without such routine the code would be very ugly:
> CORE_ADDR sp = 0;
> regcache_collect (SP_REGNUM, ((char *)&sp) + sizeof(CORE_ADDR) - REGISTER_RAW_SIZE(SP_REGNUM));

I guess we do, then :)  I'll wait for Andrew's thoughts, this is his
turf.  But other than this, your patch looks good.  Since Sparc is
listed as maintenance only (i.e. no active maintainer), you can go
ahead and commit it, I think; we'll clean up the registers[] stuff
later.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> I don't understand how regcache_collect would help here, since the assertion
> REGISTER_RAW_SIZE() == 4 would stay. That's just trading a dereference with
> a memcpy to an int variable.

Today, yes.  Tomorrow, no.

The underlying implementation is going to be changed.  Can't do that 
when people assume it is a big global byte array :-(

enjoy,
Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> 
> Well, regcache_collect is the only approved interface to the contents
> of registers[] for one thing.  It would also prevent the need for the
> cast (although you'd have to clear the upper half of the variable
> first and make sure to stuff it into the low bytes since we're
> big-endian.  Ew.).
> 
> Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> extend and shift appropriately for each architecture?

That sounds like overkill.  If you need to be doing sign/zero extension 
stuff then I'd be looking at explicit calls to extract_signed_integer() 
and/or extract_unsigned_integer() in the nat code.

A sequence like:

	void *buf = alloca (MAX_REGISTER_RAW_SIZE);
	regcache_collect (my reg, buf);
	LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
	store_unsigned_integer (dest, dest size, val);

should insulate it from the current problems.

Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 12:18:18PM -0500, Andrew Cagney wrote:
> >
> >Well, regcache_collect is the only approved interface to the contents
> >of registers[] for one thing.  It would also prevent the need for the
> >cast (although you'd have to clear the upper half of the variable
> >first and make sure to stuff it into the low bytes since we're
> >big-endian.  Ew.).
> >
> >Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> >extend and shift appropriately for each architecture?
> 
> That sounds like overkill.  If you need to be doing sign/zero extension 
> stuff then I'd be looking at explicit calls to extract_signed_integer() 
> and/or extract_unsigned_integer() in the nat code.
> 
> A sequence like:
> 
> 	void *buf = alloca (MAX_REGISTER_RAW_SIZE);
> 	regcache_collect (my reg, buf);
> 	LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
> 	store_unsigned_integer (dest, dest size, val);
> 
> should insulate it from the current problems.

But won't we want this absolutely every time we extract a CORE_ADDR? 
And for that matter, I'm talking about getting a target memory address
out of a register; is store_*signed_integer right for that?  Is there
an extract_pointer or so?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

>> That sounds like overkill.  If you need to be doing sign/zero extension 
>> stuff then I'd be looking at explicit calls to extract_signed_integer() 
>> and/or extract_unsigned_integer() in the nat code.
>> 
>> A sequence like:
>> 
>> void *buf = alloca (MAX_REGISTER_RAW_SIZE);
>> regcache_collect (my reg, buf);
>> LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
>> store_unsigned_integer (dest, dest size, val);
>> 
>> should insulate it from the current problems.
> 
> 
> But won't we want this absolutely every time we extract a CORE_ADDR? 
> And for that matter, I'm talking about getting a target memory address
> out of a register; is store_*signed_integer right for that?  Is there
> an extract_pointer or so?

In *-nat.c?  Now I'm confused :-)

Doesn't the *-nat.c file just copy raw register bytes between the 
regcache and the /proc or ptrace() interface?  The only complication I 
could see is if someone used 32 bit ptrace calls to get the values for a 
regcache that had space for 64 bit registers - the above code snipit 
would handle that.

The reason for suggesting extract/store signed/unsigned integer is that 
they have clear, machine independant, semantics that work on 
uninterpreted (well apart from assuming they are integers :-) bytes.

?
Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 03:34:41PM -0500, Andrew Cagney wrote:
> 
> >>That sounds like overkill.  If you need to be doing sign/zero extension 
> >>stuff then I'd be looking at explicit calls to extract_signed_integer() 
> >>and/or extract_unsigned_integer() in the nat code.
> >>
> >>A sequence like:
> >>
> >>void *buf = alloca (MAX_REGISTER_RAW_SIZE);
> >>regcache_collect (my reg, buf);
> >>LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
> >>store_unsigned_integer (dest, dest size, val);
> >>
> >>should insulate it from the current problems.
> >
> >
> >But won't we want this absolutely every time we extract a CORE_ADDR? 
> >And for that matter, I'm talking about getting a target memory address
> >out of a register; is store_*signed_integer right for that?  Is there
> >an extract_pointer or so?
> 
> In *-nat.c?  Now I'm confused :-)
> 
> Doesn't the *-nat.c file just copy raw register bytes between the 
> regcache and the /proc or ptrace() interface?  The only complication I 
> could see is if someone used 32 bit ptrace calls to get the values for a 
> regcache that had space for 64 bit registers - the above code snipit 
> would handle that.
> 
> The reason for suggesting extract/store signed/unsigned integer is that 
> they have clear, machine independant, semantics that work on 
> uninterpreted (well apart from assuming they are integers :-) bytes.

Well, remember that we can't cast things to (CORE_ADDR *) reliably. 
With --enable-64-bit-bfd, that has a tendency to turn into a 'long long *'.
What was happening was reading $sp out of the regcache, and then
passing it to target_read_memory.  If this were MIPS, I think we'd have
to sign extend there, for "correctness".  We'd eventually truncate it
back down with a cast in infptrace.c, though.

I just don't like duplicating that above code sequence everywhere we
get a pointer out of a register into a CORE_ADDR.  It seems like a very
frequent operation, in nat or in tdep.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> Well, remember that we can't cast things to (CORE_ADDR *) reliably. 
> With --enable-64-bit-bfd, that has a tendency to turn into a 'long long *'.
> What was happening was reading $sp out of the regcache, and then
> passing it to target_read_memory.  If this were MIPS, I think we'd have
> to sign extend there, for "correctness".  We'd eventually truncate it
> back down with a cast in infptrace.c, though.
> 
> I just don't like duplicating that above code sequence everywhere we
> get a pointer out of a register into a CORE_ADDR.  It seems like a very
> frequent operation, in nat or in tdep.


[Carefully read code ... Hmm, why is it reading registers from memory 
(stack) .....  Hmm, why were all the registers written direct to the 
stack   ...?  Hmm, nothing does something that stupid except something 
with register windows ....  Er, Oh dear....]

Sorry, yes that code, as it stands, looks like it does need to do an 
extract_XXXX_address() call on the register.

As it stands?  gdbarch_register_read/write and the fetch registers for 
frame function should be mapping those registers onto memory addresses 
instead of passing the problem down to the target fetch register code. 
That way, those stored-in-memory registers are fetched via the memory 
and not the data cache.  That, however, is currently just theory :-)

Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> I don't understand how regcache_collect would help here, since the assertion
> REGISTER_RAW_SIZE() == 4 would stay. That's just trading a dereference with
> a memcpy to an int variable.

Today, yes.  Tomorrow, no.

The underlying implementation is going to be changed.  Can't do that 
when people assume it is a big global byte array :-(

enjoy,
Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> 
> Well, regcache_collect is the only approved interface to the contents
> of registers[] for one thing.  It would also prevent the need for the
> cast (although you'd have to clear the upper half of the variable
> first and make sure to stuff it into the low bytes since we're
> big-endian.  Ew.).
> 
> Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> extend and shift appropriately for each architecture?

That sounds like overkill.  If you need to be doing sign/zero extension 
stuff then I'd be looking at explicit calls to extract_signed_integer() 
and/or extract_unsigned_integer() in the nat code.

A sequence like:

	void *buf = alloca (MAX_REGISTER_RAW_SIZE);
	regcache_collect (my reg, buf);
	LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
	store_unsigned_integer (dest, dest size, val);

should insulate it from the current problems.

Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 12:18:18PM -0500, Andrew Cagney wrote:
> >
> >Well, regcache_collect is the only approved interface to the contents
> >of registers[] for one thing.  It would also prevent the need for the
> >cast (although you'd have to clear the upper half of the variable
> >first and make sure to stuff it into the low bytes since we're
> >big-endian.  Ew.).
> >
> >Andrew?  Do we need to have a regcache_collect_core_addr, to sign
> >extend and shift appropriately for each architecture?
> 
> That sounds like overkill.  If you need to be doing sign/zero extension 
> stuff then I'd be looking at explicit calls to extract_signed_integer() 
> and/or extract_unsigned_integer() in the nat code.
> 
> A sequence like:
> 
> 	void *buf = alloca (MAX_REGISTER_RAW_SIZE);
> 	regcache_collect (my reg, buf);
> 	LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
> 	store_unsigned_integer (dest, dest size, val);
> 
> should insulate it from the current problems.

But won't we want this absolutely every time we extract a CORE_ADDR? 
And for that matter, I'm talking about getting a target memory address
out of a register; is store_*signed_integer right for that?  Is there
an extract_pointer or so?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

>> That sounds like overkill.  If you need to be doing sign/zero extension 
>> stuff then I'd be looking at explicit calls to extract_signed_integer() 
>> and/or extract_unsigned_integer() in the nat code.
>> 
>> A sequence like:
>> 
>> void *buf = alloca (MAX_REGISTER_RAW_SIZE);
>> regcache_collect (my reg, buf);
>> LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
>> store_unsigned_integer (dest, dest size, val);
>> 
>> should insulate it from the current problems.
> 
> 
> But won't we want this absolutely every time we extract a CORE_ADDR? 
> And for that matter, I'm talking about getting a target memory address
> out of a register; is store_*signed_integer right for that?  Is there
> an extract_pointer or so?

In *-nat.c?  Now I'm confused :-)

Doesn't the *-nat.c file just copy raw register bytes between the 
regcache and the /proc or ptrace() interface?  The only complication I 
could see is if someone used 32 bit ptrace calls to get the values for a 
regcache that had space for 64 bit registers - the above code snipit 
would handle that.

The reason for suggesting extract/store signed/unsigned integer is that 
they have clear, machine independant, semantics that work on 
uninterpreted (well apart from assuming they are integers :-) bytes.

?
Andrew

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Daniel Jacobowitz]

On Mon, Nov 26, 2001 at 03:34:41PM -0500, Andrew Cagney wrote:
> 
> >>That sounds like overkill.  If you need to be doing sign/zero extension 
> >>stuff then I'd be looking at explicit calls to extract_signed_integer() 
> >>and/or extract_unsigned_integer() in the nat code.
> >>
> >>A sequence like:
> >>
> >>void *buf = alloca (MAX_REGISTER_RAW_SIZE);
> >>regcache_collect (my reg, buf);
> >>LONGEST val = extract_unsigned_integer (buf, REGISTER_RAW_SIZE(my reg));
> >>store_unsigned_integer (dest, dest size, val);
> >>
> >>should insulate it from the current problems.
> >
> >
> >But won't we want this absolutely every time we extract a CORE_ADDR? 
> >And for that matter, I'm talking about getting a target memory address
> >out of a register; is store_*signed_integer right for that?  Is there
> >an extract_pointer or so?
> 
> In *-nat.c?  Now I'm confused :-)
> 
> Doesn't the *-nat.c file just copy raw register bytes between the 
> regcache and the /proc or ptrace() interface?  The only complication I 
> could see is if someone used 32 bit ptrace calls to get the values for a 
> regcache that had space for 64 bit registers - the above code snipit 
> would handle that.
> 
> The reason for suggesting extract/store signed/unsigned integer is that 
> they have clear, machine independant, semantics that work on 
> uninterpreted (well apart from assuming they are integers :-) bytes.

Well, remember that we can't cast things to (CORE_ADDR *) reliably. 
With --enable-64-bit-bfd, that has a tendency to turn into a 'long long *'.
What was happening was reading $sp out of the regcache, and then
passing it to target_read_memory.  If this were MIPS, I think we'd have
to sign extend there, for "correctness".  We'd eventually truncate it
back down with a cast in infptrace.c, though.

I just don't like duplicating that above code sequence everywhere we
get a pointer out of a register into a CORE_ADDR.  It seems like a very
frequent operation, in nat or in tdep.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Fix sparc-*-linux register fetching/storing

[Andrew Cagney]

> Well, remember that we can't cast things to (CORE_ADDR *) reliably. 
> With --enable-64-bit-bfd, that has a tendency to turn into a 'long long *'.
> What was happening was reading $sp out of the regcache, and then
> passing it to target_read_memory.  If this were MIPS, I think we'd have
> to sign extend there, for "correctness".  We'd eventually truncate it
> back down with a cast in infptrace.c, though.
> 
> I just don't like duplicating that above code sequence everywhere we
> get a pointer out of a register into a CORE_ADDR.  It seems like a very
> frequent operation, in nat or in tdep.


[Carefully read code ... Hmm, why is it reading registers from memory 
(stack) .....  Hmm, why were all the registers written direct to the 
stack   ...?  Hmm, nothing does something that stupid except something 
with register windows ....  Er, Oh dear....]

Sorry, yes that code, as it stands, looks like it does need to do an 
extract_XXXX_address() call on the register.

As it stands?  gdbarch_register_read/write and the fetch registers for 
frame function should be mapping those registers onto memory addresses 
instead of passing the problem down to the target fetch register code. 
That way, those stored-in-memory registers are fetched via the memory 
and not the data cache.  That, however, is currently just theory :-)

Andrew