[rfc]: Replace current_gdbarch in sentinel-frame.c

[rfc]: Replace current_gdbarch in sentinel-frame.c

[Markus Deuling]

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

Hi,

this is a trivial replacement of current_gdbarch in sentinel-frame.c. Tested on x86.

Ok to commit?


ChangeLog:

	* sentinel-frame.c (sentinel_frame_prev_register): Use get_frame_arch
	to get at the current architecture.

Regards, 
Markus


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


[-- Attachment #2: diff-sentinel --]
[-- Type: text/plain, Size: 568 bytes --]

diff -urpN src/gdb/sentinel-frame.c dev/gdb/sentinel-frame.c
--- src/gdb/sentinel-frame.c	2008-01-01 23:53:12.000000000 +0100
+++ dev/gdb/sentinel-frame.c	2008-02-15 10:07:36.000000000 +0100
@@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
      onto the corresponding hardware register.  */
   *optimized = 0;
   *lvalp = lval_register;
-  *addrp = register_offset_hack (current_gdbarch, regnum);
+  *addrp = register_offset_hack (get_frame_arch (next_frame), regnum);
   *realnum = regnum;
 
   /* If needed, find and return the value of the register.  */

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Ulrich Weigand]

Markus Deuling wrote:

> @@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
>       onto the corresponding hardware register.  */
>    *optimized = 0;
>    *lvalp = lval_register;
> -  *addrp = register_offset_hack (current_gdbarch, regnum);
> +  *addrp = register_offset_hack (get_frame_arch (next_frame), regnum);
>    *realnum = regnum;

This is not OK -- as the sentinel frame is always the innermost
frame, next_frame will always be NULL here.

You could use the gdbarch of the sentinel frame's regcache
(cache->regcache).

However, I think setting the "address" for register values is
no longer needed anymore, so the whole call to register_offset_hack
(and the function itself) can be removed.

Can you try whether simply setting *addrp = 0 works?

Bye,
Ulrich

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

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Michael Snyder]

On Fri, 2008-02-15 at 10:24 +0100, Markus Deuling wrote:
> Hi,
> 
> this is a trivial replacement of current_gdbarch in sentinel-frame.c. Tested on x86.
> 
> Ok to commit?

Looks fine to me.

> 
> 
> ChangeLog:
> 
> 	* sentinel-frame.c (sentinel_frame_prev_register): Use get_frame_arch
> 	to get at the current architecture.
> 
> Regards, 
> Markus
> 
> 
> plain text document attachment (diff-sentinel)
> diff -urpN src/gdb/sentinel-frame.c dev/gdb/sentinel-frame.c
> --- src/gdb/sentinel-frame.c	2008-01-01 23:53:12.000000000 +0100
> +++ dev/gdb/sentinel-frame.c	2008-02-15 10:07:36.000000000 +0100
> @@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
>       onto the corresponding hardware register.  */
>    *optimized = 0;
>    *lvalp = lval_register;
> -  *addrp = register_offset_hack (current_gdbarch, regnum);
> +  *addrp = register_offset_hack (get_frame_arch (next_frame), regnum);
>    *realnum = regnum;
>  
>    /* If needed, find and return the value of the register.  */

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Michael Snyder]

On Fri, 2008-02-15 at 09:44 -0800, Michael Snyder wrote:
> On Fri, 2008-02-15 at 10:24 +0100, Markus Deuling wrote:
> > Hi,
> > 
> > this is a trivial replacement of current_gdbarch in sentinel-frame.c. Tested on x86.
> > 
> > Ok to commit?
> 
> Looks fine to me.

And it seems I was premature.
I defer to Ulrich.   ;-)

> 
> > 
> > 
> > ChangeLog:
> > 
> > 	* sentinel-frame.c (sentinel_frame_prev_register): Use get_frame_arch
> > 	to get at the current architecture.
> > 
> > Regards, 
> > Markus
> > 
> > 
> > plain text document attachment (diff-sentinel)
> > diff -urpN src/gdb/sentinel-frame.c dev/gdb/sentinel-frame.c
> > --- src/gdb/sentinel-frame.c	2008-01-01 23:53:12.000000000 +0100
> > +++ dev/gdb/sentinel-frame.c	2008-02-15 10:07:36.000000000 +0100
> > @@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
> >       onto the corresponding hardware register.  */
> >    *optimized = 0;
> >    *lvalp = lval_register;
> > -  *addrp = register_offset_hack (current_gdbarch, regnum);
> > +  *addrp = register_offset_hack (get_frame_arch (next_frame), regnum);
> >    *realnum = regnum;
> >  
> >    /* If needed, find and return the value of the register.  */
> 

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Thiago Jung Bauermann]

On Fri, 2008-02-15 at 18:43 +0100, Ulrich Weigand wrote:
> Markus Deuling wrote:
> 
> > @@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
> >       onto the corresponding hardware register.  */
> >    *optimized = 0;
> >    *lvalp = lval_register;
> > -  *addrp = register_offset_hack (current_gdbarch, regnum);
> > +  *addrp = register_offset_hack (get_frame_arch (next_frame), regnum);
> >    *realnum = regnum;
> 
> This is not OK -- as the sentinel frame is always the innermost
> frame, next_frame will always be NULL here.

Actually, what I observe here when running GDB is that next_frame is the
sentinel frame, probably because sentinel frame's next field points back
to itself.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Ulrich Weigand]

Thiago Jung Bauermann wrote:
> On Fri, 2008-02-15 at 18:43 +0100, Ulrich Weigand wrote:
> > This is not OK -- as the sentinel frame is always the innermost
> > frame, next_frame will always be NULL here.
> 
> Actually, what I observe here when running GDB is that next_frame is the
> sentinel frame, probably because sentinel frame's next field points back
> to itself.

Ah, right, I forgot about that.  Still, it seems best to try to get 
rid of this hack completely ...

Bye,
Ulrich

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

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Markus Deuling]

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

Ulrich Weigand schrieb:
> Thiago Jung Bauermann wrote:
>> On Fri, 2008-02-15 at 18:43 +0100, Ulrich Weigand wrote:
>>> This is not OK -- as the sentinel frame is always the innermost
>>> frame, next_frame will always be NULL here.
>> Actually, what I observe here when running GDB is that next_frame is the
>> sentinel frame, probably because sentinel frame's next field points back
>> to itself.
> 
> Ah, right, I forgot about that.  Still, it seems best to try to get 
> rid of this hack completely ...
> 

I tested *addrp = 0 on x86 and saw no regression. I changed the patch to remove
register_offset_hack completely.

Ok ?


ChangeLog:

	* sentinel-frame.c (sentinel_frame_prev_register): Do not call
	register_offset_hack anymore.

	* regcache.{c,h} (register_offset_hack): Remove.


Regards,
Markus

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


[-- Attachment #2: diff-sentinel --]
[-- Type: text/plain, Size: 2180 bytes --]

diff -urpN src/gdb/regcache.c dev/gdb/regcache.c
--- src/gdb/regcache.c	2008-01-01 23:53:12.000000000 +0100
+++ dev/gdb/regcache.c	2008-02-18 16:19:40.000000000 +0100
@@ -761,18 +761,6 @@ regcache_cooked_write_part (struct regca
 		      regcache_cooked_read, regcache_cooked_write);
 }
 
-/* Hack to keep code that view the register buffer as raw bytes
-   working.  */
-
-int
-register_offset_hack (struct gdbarch *gdbarch, int regnum)
-{
-  struct regcache_descr *descr = regcache_descr (gdbarch);
-  gdb_assert (regnum >= 0 && regnum < descr->nr_cooked_registers);
-  return descr->register_offset[regnum];
-}
-
-
 /* Supply register REGNUM, whose contents are stored in BUF, to REGCACHE.  */
 
 void
diff -urpN src/gdb/regcache.h dev/gdb/regcache.h
--- src/gdb/regcache.h	2008-01-01 23:53:12.000000000 +0100
+++ dev/gdb/regcache.h	2008-02-18 16:21:04.000000000 +0100
@@ -104,18 +104,6 @@ extern void regcache_raw_collect (const 
 				  int regnum, void *buf);
 
 
-/* The register's ``offset''.
-
-   FIXME: cagney/2002-11-07: The frame_register() function, when
-   specifying the real location of a register, does so using that
-   registers offset in the register cache.  That offset is then used
-   by valops.c to determine the location of the register.  The code
-   should instead use the register's number and a location expression
-   to describe a value spread across multiple registers or memory.  */
-
-extern int register_offset_hack (struct gdbarch *gdbarch, int regnum);
-
-
 /* The type of a register.  This function is slightly more efficient
    then its gdbarch vector counterpart since it returns a precomputed
    value stored in a table.  */
diff -urpN src/gdb/sentinel-frame.c dev/gdb/sentinel-frame.c
--- src/gdb/sentinel-frame.c	2008-01-01 23:53:12.000000000 +0100
+++ dev/gdb/sentinel-frame.c	2008-02-18 16:21:39.000000000 +0100
@@ -54,7 +54,7 @@ sentinel_frame_prev_register (struct fra
      onto the corresponding hardware register.  */
   *optimized = 0;
   *lvalp = lval_register;
-  *addrp = register_offset_hack (current_gdbarch, regnum);
+  *addrp = 0;
   *realnum = regnum;
 
   /* If needed, find and return the value of the register.  */

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Ulrich Weigand]

Markus Deuling wrote:

> 	* sentinel-frame.c (sentinel_frame_prev_register): Do not call
> 	register_offset_hack anymore.
> 
> 	* regcache.{c,h} (register_offset_hack): Remove.

This is OK as well.

Thanks,
Ulrich

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

Re: [rfc]: Replace current_gdbarch in sentinel-frame.c

[Markus Deuling]

Ulrich Weigand schrieb:
> Markus Deuling wrote:
> 
>> 	* sentinel-frame.c (sentinel_frame_prev_register): Do not call
>> 	register_offset_hack anymore.
>>
>> 	* regcache.{c,h} (register_offset_hack): Remove.
> 
> This is OK as well.
> 
> Thanks,
> Ulrich
> 

Thank you. Committed as well.


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