[RFA]: New function to supply only one x87 register

[RFA]: New function to supply only one x87 register

[Eli Zaretskii]

There's a strange asymmetry in i387-nat.c: i387_fill_fsave accepts a
REGNO argument and can fill only one register's value, but
i387_supply_fsave cannot.  I needed this to be symmetrical, but I
didn't want to break existing APIs.  So I added a new function that
handles one register only.  See below.

Okay to commit?


2001-02-07  Eli Zaretskii  <eliz@is.elta.co.il>

	* i387-nat.h: Add prototype for i387_supply_fpreg.

	* i387-nat.c (i387_supply_fpreg): New function.
	(i387_supply_fsave): Move the loop body to i387_supply_fpreg.

--- gdb/i387-nat.c~1	Wed Feb  7 22:27:42 2001
+++ gdb/i387-nat.c	Wed Feb  7 22:31:52 2001
@@ -55,6 +55,38 @@ static int fsave_offset[] =
 #define FSAVE_ADDR(fsave, regnum) (fsave + fsave_offset[regnum - FP0_REGNUM])
 \f
 
+/* Fill one FP register's slot in GDB's register array with the
+   floating-point value of register REGNO from *FSAVE.  This function
+   masks off any of the reserved bits in *FSAVE.  */
+
+void
+i387_supply_fpreg (char *fsave, int regno)
+{
+  /* Most of the FPU control registers occupy only 16 bits in
+	 the fsave area.  Give those a special treatment.  */
+  if (regno >= FIRST_FPU_CTRL_REGNUM
+      && regno != FCOFF_REGNUM && regno != FDOFF_REGNUM)
+    {
+      unsigned int val = *(unsigned short *) (FSAVE_ADDR (fsave, regno));
+
+      if (regno == FOP_REGNUM)
+	{
+	  val &= ((1 << 11) - 1);
+	  /* Feature: restore the 5 bits of the opcode that are
+	     missing in the data supplied by FSAVE/FNSAVE.  This shows
+	     the opcode in its full glory, in case someone wants to
+	     produce a mnemonic from it.  */
+	  if (val)
+	    val |= 0xd800;
+	  supply_register (regno, (char *) &val);
+	}
+      else
+	supply_register (regno, (char *) &val);
+    }
+  else
+    supply_register (regno, FSAVE_ADDR (fsave, regno));
+}
+
 /* Fill GDB's register array with the floating-point register values
    in *FSAVE.  This function masks off any of the reserved
    bits in *FSAVE.  */
@@ -66,29 +98,7 @@ i387_supply_fsave (char *fsave)
 
   for (i = FP0_REGNUM; i <= LAST_FPU_CTRL_REGNUM; i++)
     {
-      /* Most of the FPU control registers occupy only 16 bits in
-	 the fsave area.  Give those a special treatment.  */
-      if (i >= FIRST_FPU_CTRL_REGNUM
-	  && i != FCOFF_REGNUM && i != FDOFF_REGNUM)
-	{
-	  unsigned int val = *(unsigned short *) (FSAVE_ADDR (fsave, i));
-
-	  if (i == FOP_REGNUM)
-	    {
-	      val &= ((1 << 11) - 1);
-	      /* Feature: restore the 5 bits of the opcode that are
-		 missing in the data supplied by FSAVE/FNSAVE.  This
-		 shows the opcode in its full glory, in case someone
-		 wants to produce a mnemonic from it.  */
-	      if (val)
-		val |= 0xd800;
-	      supply_register (i, (char *) &val);
-	    }
-	  else
-	    supply_register (i, (char *) &val);
-	}
-      else
-	supply_register (i, FSAVE_ADDR (fsave, i));
+      i387_supply_fpreg (fsave, i);
     }
 }
 
From eliz@delorie.com Thu Feb 08 10:28:00 2001
From: Eli Zaretskii <eliz@delorie.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA]: Convert the last FP opcode saved by FSAVE into a full opcode
Date: Thu, 08 Feb 2001 10:28:00 -0000
Message-id: <200102081828.NAA25535@indy.delorie.com>
X-SW-Source: 2001-02/msg00139.html
Content-length: 1553

This patch restores the 5 bits of the last FP opcode which are not
delivered by FSAVE/FXSAVE.  This shows the full opcode in "info
float", and may be used to display the opcode as a mnemonic, or look
it up in a list of instructions.

The DJGPP port was doing this for quite some time.  Now I'm converting
go32-nat.c to using i387-nat.c, and I'd hate to lose the feature.

Okay?

2001-02-07  Eli Zaretskii  <eliz@is.elta.co.il>

	* i387-nat.c (i387_supply_fsave): Restore the 5 bits of the FPU's
	opcode that are not provided by FSAVE/FNSAVE.
	(i387_supply_fxsave): Likewise.

--- gdb/i387-nat.c~0	Thu Aug 10 17:54:50 2000
+++ gdb/i387-nat.c	Wed Feb  7 20:40:02 2001
@@ -76,6 +76,12 @@ i387_supply_fsave (char *fsave)
 	  if (i == FOP_REGNUM)
 	    {
 	      val &= ((1 << 11) - 1);
+	      /* Feature: restore the 5 bits of the opcode that are
+		 missing in the data supplied by FSAVE/FNSAVE.  This
+		 shows the opcode in its full glory, in case someone
+		 wants to produce a mnemonic from it.  */
+	      if (val)
+		val |= 0xd800;
 	      supply_register (i, (char *) &val);
 	    }
 	  else
@@ -185,6 +191,12 @@ i387_supply_fxsave (char *fxsave)
 	  if (i == FOP_REGNUM)
 	    {
 	      val &= ((1 << 11) - 1);
+	      /* Feature: restore the 5 bits of the opcode that are
+		 missing in the data supplied by FSAVE/FNSAVE.  This
+		 shows the opcode in its full glory, in case someone
+		 wants to produce a mnemonic from it.  */
+	      if (val)
+		val |= 0xd800;
 	      supply_register (i, (char *) &val);
 	    }
 	  else if (i== FTAG_REGNUM)
From msnyder@cygnus.com Thu Feb 08 12:00:00 2001
From: Michael Snyder <msnyder@cygnus.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: Michael Elizabeth Chastain <chastain@cygnus.com>, "Peter.Schauer" <Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] PATCH: finding a function with address
Date: Thu, 08 Feb 2001 12:00:00 -0000
Message-id: <3A82FA30.6464C26D@cygnus.com>
References: <200102051329.FAA31142@bosch.cygnus.com> <200102051025.LAA26655@reisser.regent.e-technik.tu-muenchen.de> <200102081827.NAA25508@indy.delorie.com>
X-SW-Source: 2001-02/msg00141.html
Content-length: 1899

OK, Eli, I'm not the symtab maintainer, but I wrote that code, 
and I think your change is reasonable.  -- Michael Snyder

Eli Zaretskii wrote:
> 
> I wrote:
> 
> > Here's the scoop: many debug formats, including COFF, put NULL into
> > SYMBOL_BFD_SECTION, see `prim_record_minimal_symbol' and all its
> > callers.  This makes `lookup_minimal_symbol_by_pc_section' reject all
> > symbols it finds, and come up empty handed.
> >
> > Can someone who knows his/her way inside minsyms.c please suggest how
> > to fix this?  It seems that in its current shape,
> > `lookup_minimal_symbol_by_pc_section' is too harsh to quite a few
> > platforms, so I think we'd better change that.  Would an additional
> > test for SYMBOL_BFD_SECTION being non-NULL be okay, for example?
> 
> Well, since no one replied, I'm now turning this into RFA ;-)
> 
> The following patch allows targets where SYMBOL_BFD_SECTION is NULL,
> in particular targets which use COFF debug info, to use "info symbol"
> as advertised.
> 
> Okay to commit?
> 
> 2001-02-07  Eli Zaretskii  <eliz@is.elta.co.il>
> 
>         * minsyms.c (lookup_minimal_symbol_by_pc_section): Don't skip
>         symbols whose SYMBOL_BFD_SECTION is NULL.
> 
> --- gdb/minsyms.c~0     Fri Dec 15 03:01:48 2000
> +++ gdb/minsyms.c       Wed Feb  7 20:29:38 2001
> @@ -482,6 +482,10 @@ lookup_minimal_symbol_by_pc_section (COR
>               /* This is the new code that distinguishes it from the old function */
>               if (section)
>                 while (hi >= 0
> +                      /* Some types of debug info, such as COFF,
> +                         don't fill the bfd_section member, so don't
> +                         throw away symbols on those platforms.  */
> +                      && SYMBOL_BFD_SECTION (&msymbol[hi]) != NULL
>                        && SYMBOL_BFD_SECTION (&msymbol[hi]) != section)
>                   --hi;
>
From msokolov@ivan.Harhan.ORG Thu Feb 08 13:47:00 2001
From: msokolov@ivan.Harhan.ORG (Michael Sokolov)
To: gdb-patches@sources.redhat.com
Subject: Re: [patch] to gdb: portability fix: <sys/file.h>
Date: Thu, 08 Feb 2001 13:47:00 -0000
Message-id: <0102082144.AA24940@ivan.Harhan.ORG>
X-SW-Source: 2001-02/msg00142.html
Content-length: 3590

Eli Zaretskii <eliz@is.elta.co.il> wrote:

> You could replace HAVE_SYS_FILE_H with something 4.3BSD defines, for
> example.

But why? Sure, I could use the BSD4_3 define, but what about some system that
is derived from 4.3BSD and still defines BSD4_3, but doesn't have this
property? Or vice-versa, some other system that also has important definitions
in <sys/file.h> but is not directly related to 4.3BSD genetically and doesn't
define BSD4_3? I thought you wanted to move away from hard-coded system-
specific assumptions toward generic feature tests. In this vein I cannot
understand your reluctance to add the check for <sys/file.h> to configure.

Thinking more about it, I just realised that <sys/file.h> might be needed on
some systems for other file-related definitions besides the access(2) macros.
Therefore, I think it would be best to *always* include this header in defs.h
if it is present. Defining F_OK, etc. when they are not defined anywhere then
becomes a separate matter. The patch is below. Now can I *please* get a review
by a *blanket write priv maintainer*?

-- 
Michael Sokolov
Public Service Agent
International Engineering and Science Task Force

1351 VINE AVE APT 27		Phone: +1-714-738-5409
FULLERTON CA 92833-4291 USA	(home office)

E-mail: msokolov@ivan.Harhan.ORG (ARPA TCP/SMTP)

2001-02-08  Michael Sokolov  <msokolov@ivan.Harhan.ORG>

	* configure.in (AC_CHECK_HEADERS): Add sys/file.h.
	* configure, config.in: Regenerate.
	* defs.h: Include <sys/file.h> if present.
	(F_OK, X_OK, W_OK, R_OK): Define for systems that don't have them.

Index: configure.in
===================================================================
RCS file: /cvs/src/src/gdb/configure.in,v
retrieving revision 1.55
diff -p -r1.55 configure.in
*** configure.in	2001/01/31 02:08:23	1.55
--- configure.in	2001/02/08 21:28:55
***************
*** 1,5 ****
  dnl Autoconf configure script for GDB, the GNU debugger.
! dnl Copyright 1995, 1996, 1997, 1998, 1999, 2000 Free Software Foundation, Inc.
  dnl
  dnl This file is part of GDB.
  dnl 
--- 1,6 ----
  dnl Autoconf configure script for GDB, the GNU debugger.
! dnl Copyright 1995, 1996, 1997, 1998, 1999, 2000,
! dnl 2001 Free Software Foundation, Inc.
  dnl
  dnl This file is part of GDB.
  dnl 
*************** AC_CHECK_HEADERS(ctype.h endian.h link.h
*** 122,128 ****
  	string.h sys/procfs.h sys/ptrace.h sys/reg.h stdint.h \
  	term.h termio.h termios.h unistd.h wait.h sys/wait.h \
  	wchar.h wctype.h asm/debugreg.h sys/debugreg.h sys/select.h \
! 	time.h sys/ioctl.h sys/user.h \
  	dirent.h sys/ndir.h sys/dir.h ndir.h \
  	curses.h ncurses.h \
  	poll.h sys/poll.h)
--- 123,129 ----
  	string.h sys/procfs.h sys/ptrace.h sys/reg.h stdint.h \
  	term.h termio.h termios.h unistd.h wait.h sys/wait.h \
  	wchar.h wctype.h asm/debugreg.h sys/debugreg.h sys/select.h \
! 	time.h sys/file.h sys/ioctl.h sys/user.h \
  	dirent.h sys/ndir.h sys/dir.h ndir.h \
  	curses.h ncurses.h \
  	poll.h sys/poll.h)
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.38
diff -p -r1.38 defs.h
*** defs.h	2001/02/08 06:49:19	1.38
--- defs.h	2001/02/08 21:29:02
***************
*** 38,43 ****
--- 38,60 ----
  #include <unistd.h>
  #endif
  
+ #ifdef HAVE_SYS_FILE_H
+ #include <sys/file.h>
+ #endif
+ 
+ #ifndef F_OK
+ # define F_OK 0
+ #endif
+ #ifndef X_OK
+ # define X_OK 1
+ #endif
+ #ifndef W_OK
+ # define W_OK 2
+ #endif
+ #ifndef R_OK
+ # define R_OK 4
+ #endif
+ 
  /* Just in case they're not defined in stdio.h. */
  
  #ifndef SEEK_SET
From jkingdon@engr.sgi.com Thu Feb 08 17:29:00 2001
From: jkingdon@engr.sgi.com
To: gdb-patches@sources.redhat.com
Subject: help out make TAGS
Date: Thu, 08 Feb 2001 17:29:00 -0000
Message-id: <200102090129.UAA15637@panix2.panix.com>
X-SW-Source: 2001-02/msg00143.html
Content-length: 2348

Seems like "make TAGS" has bitrotted a bit lately.  OK to commit the
enclosed patch?  I don't know whether it fixes everything, but it
works for me and is an improvement.

Jim Kingdon
jkingdon@engr.sgi.com

2001-02-08  Jim Kingdon  <jkingdon@engr.sgi.com>

	Updates to "make TAGS":
	* Makefile.in (ALLDEPFILES): Remove altos-xdep.c arm-convert.s
	arm-xdep.c convex-tdep.c convex-xdep.c pyr-tdep.c pyr-xdep.c
	tahoe-tdep.c.
	(TAGFILES_NO_SRCDIR): Add $(SUBDIR_CLI_SRCS).

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.60
diff -u -r1.60 Makefile.in
--- Makefile.in	2001/02/06 04:17:03	1.60
+++ Makefile.in	2001/02/09 01:02:34
@@ -655,7 +655,7 @@
 # Don't include YYFILES (*.tab.c) because we already include *.y in SFILES,
 # and it's more useful to see it in the .y file.
 TAGFILES_NO_SRCDIR = $(SFILES) $(HFILES_NO_SRCDIR) $(ALLDEPFILES) \
-	$(POSSLIBS)
+	$(POSSLIBS) $(SUBDIR_CLI_SRCS)
 TAGFILES_WITH_SRCDIR = $(HFILES_WITH_SRCDIR)
 
 COMMON_OBS = version.o blockframe.o breakpoint.o findvar.o regcache.o \
@@ -1130,9 +1130,8 @@
 ALLDEPFILES = 29k-share/udi/udip2soc.c 29k-share/udi/udr.c \
 	29k-share/udi/udi2go32.c \
 	a29k-tdep.c a68v-nat.c alpha-nat.c alpha-tdep.c \
-	altos-xdep.c arm-convert.s \
-	arm-linux-nat.c arm-linux-tdep.c arm-tdep.c arm-xdep.c \
-	coff-solib.c convex-tdep.c convex-xdep.c \
+	arm-linux-nat.c arm-linux-tdep.c arm-tdep.c \
+	coff-solib.c \
 	core-sol2.c core-regset.c core-aout.c corelow.c \
 	dcache.c delta68-nat.c dpx2-nat.c dstread.c exec.c fork-child.c \
 	go32-nat.c h8300-tdep.c h8500-tdep.c \
@@ -1156,7 +1155,7 @@
 	ns32k-tdep.c ns32km3-nat.c osfsolib.c \
 	somread.c somsolib.c $(HPREAD_SOURCE) \
 	ppc-linux-nat.c ppc-linux-tdep.c \
-	procfs.c pyr-tdep.c pyr-xdep.c \
+	procfs.c \
 	remote-adapt.c remote-array.c remote-bug.c remote-e7000.c remote-eb.c \
 	remote-es.c remote-hms.c remote-mips.c \
 	remote-mm.c remote-nindy.c remote-os9k.c remote-rdp.c remote-sim.c \
@@ -1167,7 +1166,7 @@
 	sh-tdep.c solib.c sparc-nat.c \
 	sparc-tdep.c sparcl-tdep.c sun3-nat.c sun386-nat.c \
 	symm-tdep.c symm-nat.c \
-	tahoe-tdep.c ultra3-nat.c ultra3-xdep.c umax-xdep.c \
+	ultra3-nat.c ultra3-xdep.c umax-xdep.c \
 	vax-tdep.c \
 	vx-share/xdr_ld.c vx-share/xdr_ptrace.c vx-share/xdr_rdb.c \
 	win32-nat.c \
From ac131313@cygnus.com Thu Feb 08 17:43:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: jkingdon@engr.sgi.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: help out make TAGS
Date: Thu, 08 Feb 2001 17:43:00 -0000
Message-id: <3A834A05.E7DBBC57@cygnus.com>
References: <200102090129.UAA15637@panix2.panix.com>
X-SW-Source: 2001-02/msg00144.html
Content-length: 694

jkingdon@engr.sgi.com wrote:
> 
> Seems like "make TAGS" has bitrotted a bit lately.  OK to commit the
> enclosed patch?  I don't know whether it fixes everything, but it
> works for me and is an improvement.
> 
> Jim Kingdon
> jkingdon@engr.sgi.com
> 
> 2001-02-08  Jim Kingdon  <jkingdon@engr.sgi.com>
> 
>         Updates to "make TAGS":
>         * Makefile.in (ALLDEPFILES): Remove altos-xdep.c arm-convert.s
>         arm-xdep.c convex-tdep.c convex-xdep.c pyr-tdep.c pyr-xdep.c
>         tahoe-tdep.c.
>         (TAGFILES_NO_SRCDIR): Add $(SUBDIR_CLI_SRCS).

Yes, fine and thanks! Other people may want to keep an eye out for other
bitrot caused by me removing several targets.

	Andrew
From ezannoni@cygnus.com Thu Feb 08 18:48:00 2001
From: Elena Zannoni <ezannoni@cygnus.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: "Peter.Schauer" <Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] PATCH: finding a function with address
Date: Thu, 08 Feb 2001 18:48:00 -0000
Message-id: <14979.22938.783125.721722@kwikemart.cygnus.com>
References: <200102051329.FAA31142@bosch.cygnus.com> <200102051025.LAA26655@reisser.regent.e-technik.tu-muenchen.de> <200102081827.NAA25508@indy.delorie.com>
X-SW-Source: 2001-02/msg00145.html
Content-length: 1944

Eli,
Yes, go ahead.

(it took me a while to follow the section vs. bfd_section naming of
formal parameters!) Yes bfd_section can be NULL. Other places in gdb
check it like this: if (SYMBOL_BFD_SECTION (symbol)). I don't care
which way you do it.


Thanks 
Elena


Eli Zaretskii writes:
 > I wrote:
 > 
 > > Here's the scoop: many debug formats, including COFF, put NULL into
 > > SYMBOL_BFD_SECTION, see `prim_record_minimal_symbol' and all its
 > > callers.  This makes `lookup_minimal_symbol_by_pc_section' reject all
 > > symbols it finds, and come up empty handed.
 > > 
 > > Can someone who knows his/her way inside minsyms.c please suggest how
 > > to fix this?  It seems that in its current shape,
 > > `lookup_minimal_symbol_by_pc_section' is too harsh to quite a few
 > > platforms, so I think we'd better change that.  Would an additional
 > > test for SYMBOL_BFD_SECTION being non-NULL be okay, for example?
 > 
 > Well, since no one replied, I'm now turning this into RFA ;-)
 > 
 > The following patch allows targets where SYMBOL_BFD_SECTION is NULL,
 > in particular targets which use COFF debug info, to use "info symbol"
 > as advertised.
 > 
 > Okay to commit?
 > 
 > 2001-02-07  Eli Zaretskii  <eliz@is.elta.co.il>
 > 
 > 	* minsyms.c (lookup_minimal_symbol_by_pc_section): Don't skip
 > 	symbols whose SYMBOL_BFD_SECTION is NULL.
 > 
 > --- gdb/minsyms.c~0	Fri Dec 15 03:01:48 2000
 > +++ gdb/minsyms.c	Wed Feb  7 20:29:38 2001
 > @@ -482,6 +482,10 @@ lookup_minimal_symbol_by_pc_section (COR
 >  	      /* This is the new code that distinguishes it from the old function */
 >  	      if (section)
 >  		while (hi >= 0
 > +		       /* Some types of debug info, such as COFF,
 > +			  don't fill the bfd_section member, so don't
 > +			  throw away symbols on those platforms.  */
 > +		       && SYMBOL_BFD_SECTION (&msymbol[hi]) != NULL
 >  		       && SYMBOL_BFD_SECTION (&msymbol[hi]) != section)
 >  		  --hi;
 >  
 > 

Re: [RFA]: New function to supply only one x87 register

[Mark Kettenis]

Eli Zaretskii <eliz@delorie.com> writes:

> There's a strange asymmetry in i387-nat.c: i387_fill_fsave accepts a
> REGNO argument and can fill only one register's value, but
> i387_supply_fsave cannot.  I needed this to be symmetrical, but I
> didn't want to break existing APIs.  So I added a new function that
> handles one register only.  See below.

Hmm, the asymmetry you're seeing is the same asymmetry that exists
between supply_gregset() and fill_gregset().  Ultimately this
asymmetry stems from the fact that GDB's register cache is a
write-through cache (at least that's what I think).  This means that
it never hurts to supply the full FSAVE/FXSAVE info even if only one
register is requested, and that is what all other targets that use the
stuff from i387-nat.c do.  Is there a compelling reason you cannot do
the same in the go32 target?  I'd really prefer keeping the number of
interfaces as small as possible, and therefore I'd like to avoid
adding the i387_supply_fpreg function.

Mark

PS. Sorry for being so critical about your work.  I really don't want
    to discourage you.  Feel free to show me the code, if you don't
    know what to do with my suggestions.

Re: [RFA]: New function to supply only one x87 register

[Eli Zaretskii]

> From: Mark Kettenis <kettenis@wins.uva.nl>
> Date: 09 Feb 2001 14:47:25 +0100
> 
> Hmm, the asymmetry you're seeing is the same asymmetry that exists
> between supply_gregset() and fill_gregset().  Ultimately this
> asymmetry stems from the fact that GDB's register cache is a
> write-through cache (at least that's what I think).  This means that
> it never hurts to supply the full FSAVE/FXSAVE info even if only one
> register is requested, and that is what all other targets that use the
> stuff from i387-nat.c do.  Is there a compelling reason you cannot do
> the same in the go32 target?

I don't know enough about GDB's register cache to feel safe with such
assumptions.  What I'm trying to do is be consistent with the
documented API.  Since target.h's `to_fetch_registers' method says
"fetch register REGNO, or all regs if regno == -1", I'm trying to do
just that and nothing else.

Isn't it dangerous to have functions in the infrastructure with
built-in implicit assumptions that are not documented anywhere, and
only hold because the current application-level code does what it
does?

> PS. Sorry for being so critical about your work.  I really don't want
>     to discourage you.

No need to apologize.  The whole point of an RFA is to draw as much
``friendly fire'' as possible ;-)

Re: [RFA]: New function to supply only one x87 register

[Mark Kettenis]

   Date: Fri, 9 Feb 2001 09:59:15 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   > From: Mark Kettenis <kettenis@wins.uva.nl>
   > Date: 09 Feb 2001 14:47:25 +0100

   I don't know enough about GDB's register cache to feel safe with such
   assumptions.  What I'm trying to do is be consistent with the
   documented API.  Since target.h's `to_fetch_registers' method says
   "fetch register REGNO, or all regs if regno == -1", I'm trying to do
   just that and nothing else.

   Isn't it dangerous to have functions in the infrastructure with
   built-in implicit assumptions that are not documented anywhere, and
   only hold because the current application-level code does what it
   does?

Of course it is, but in this particular case it is wide-spread.  All
systems with a SVR4-like /proc file system do this, as well as most
systems that have a ptrace request that fetches a whole bunch of
registers in one go.  By supplying all those registers at once, the
cache becomes some sort of read-ahead cache.  This makes sense since
GDB hardly ever needs only one register, and pre-fetching those saves
a few system calls.  This probably doesn't matter much on go32, but I
think you should do it anyway, at least for the FPU.

We probably should document this somewhere.

Mark

Re: [RFA]: New function to supply only one x87 register

[Eli Zaretskii]

> Date: Fri, 9 Feb 2001 17:55:15 +0100 (MET)
> From: Mark Kettenis <kettenis@wins.uva.nl>
> 
>    I don't know enough about GDB's register cache to feel safe with such
>    assumptions.  What I'm trying to do is be consistent with the
>    documented API.  Since target.h's `to_fetch_registers' method says
>    "fetch register REGNO, or all regs if regno == -1", I'm trying to do
>    just that and nothing else.
> 
>    Isn't it dangerous to have functions in the infrastructure with
>    built-in implicit assumptions that are not documented anywhere, and
>    only hold because the current application-level code does what it
>    does?
> 
> Of course it is, but in this particular case it is wide-spread.  All
> systems with a SVR4-like /proc file system do this, as well as most
> systems that have a ptrace request that fetches a whole bunch of
> registers in one go.  By supplying all those registers at once, the
> cache becomes some sort of read-ahead cache.  This makes sense since
> GDB hardly ever needs only one register, and pre-fetching those saves
> a few system calls.  This probably doesn't matter much on go32, but I
> think you should do it anyway, at least for the FPU.

I can certainly do that (under a protest! ;-), but it simply seems
wrong.  I mean, let's take i386bsd-nat.c, for example:

    /* Fetch register REGNO from the inferior.  If REGNO is -1, do this
       for all registers (including the floating point registers).  */

    void
    fetch_inferior_registers (int regno)
    {
      gregset_t gregs;

      if (ptrace (PT_GETREGS, inferior_pid, (PTRACE_ARG3_TYPE) &gregs, 0) == -1)
	perror_with_name ("Couldn't get registers");

      supply_gregset (&gregs);

      if (regno == -1 || regno >= FP0_REGNUM)
	{
	  fpregset_t fpregs;

	  if (ptrace (PT_GETFPREGS, inferior_pid,
		      (PTRACE_ARG3_TYPE) &fpregs, 0) == -1)
	    perror_with_name ("Couldn't get floating point status");

	  supply_fpregset (&fpregs);
	}
    }

I remember staring at this for a few moments and thinking: this one
comlpetely ignores REGNO--isn't that a bug? and why does it call
ptrace for the general registers even if REGNO might be an FP
register--isn't _that_ a bug??

It simply doesn't feel right to do this sort of things, even if it is
somehow more efficient.  IMHO, a minor inefficiency is not worth
circumventing the established API, because it might cost much more in
maintenance down the road.

If we think it's a Good Thing to fetch all registers at once, let's
change the API, or let's have the target end cache them to be more
efficient (the DJGPP port already does that, btw).

> By supplying all those registers at once, the
> cache becomes some sort of read-ahead cache.  This makes sense since
> GDB hardly ever needs only one register, and pre-fetching those saves
> a few system calls.

Well, I thought about this, and I'm still not convinced there's a gain
here, at least not in all possible situations.

First, if GDB needs more than one FP register, it could just pass -1 as
REGNO and get them all.

Also, I can certainly think about a situation when GDB actually needs
only one FP register.  Imagine that I need to put a watchpoint on such
a register, for example, or display it at each stop.

> We probably should document this somewhere.

I'm afraid this aspect is so subtle that no matter where you document
it, it runs a high risk of being overlooked.

Btw, I found only two other platforms which currently use functions
from i387-nat.c, so it doesn't seem too late to make changes.

Re: [RFA]: New function to supply only one x87 register

[Mark Kettenis]

   Date: Sat, 10 Feb 2001 06:42:14 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   > Date: Fri, 9 Feb 2001 17:55:15 +0100 (MET)
   > From: Mark Kettenis <kettenis@wins.uva.nl>
   > 
   >    I don't know enough about GDB's register cache to feel safe with such
   >    assumptions.  What I'm trying to do is be consistent with the
   >    documented API.  Since target.h's `to_fetch_registers' method says
   >    "fetch register REGNO, or all regs if regno == -1", I'm trying to do
   >    just that and nothing else.
   > 
   >    Isn't it dangerous to have functions in the infrastructure with
   >    built-in implicit assumptions that are not documented anywhere, and
   >    only hold because the current application-level code does what it
   >    does?
   > 
   > Of course it is, but in this particular case it is wide-spread.  All
   > systems with a SVR4-like /proc file system do this, as well as most
   > systems that have a ptrace request that fetches a whole bunch of
   > registers in one go.  By supplying all those registers at once, the
   > cache becomes some sort of read-ahead cache.  This makes sense since
   > GDB hardly ever needs only one register, and pre-fetching those saves
   > a few system calls.  This probably doesn't matter much on go32, but I
   > think you should do it anyway, at least for the FPU.

   I can certainly do that (under a protest! ;-), but it simply seems
   wrong.  I mean, let's take i386bsd-nat.c, for example:

   [...]

   I remember staring at this for a few moments and thinking: this one
   comlpetely ignores REGNO--isn't that a bug? and why does it call
   ptrace for the general registers even if REGNO might be an FP
   register--isn't _that_ a bug??

Calling ptrace for the general registers if REGNO is an FP register
surely isn't very efficient.  It shouldn't hurt, since if it mattered
you'd have cache coherency problems.  So I wouldn't call it a bug (but
hey I wrote that code :-)).

   It simply doesn't feel right to do this sort of things, even if it is
   somehow more efficient.  IMHO, a minor inefficiency is not worth
   circumventing the established API, because it might cost much more in
   maintenance down the road.

True, but this circumvention is so widespread that I'm included to
call it the established API.

   If we think it's a Good Thing to fetch all registers at once, let's
   change the API, or let's have the target end cache them to be more
   efficient (the DJGPP port already does that, btw).

I just posted a message to the discussion list on this subject.  And
no, I don't thing that the target end should cache the registers,
since that would be duplicating what the existing register cache is
already capable of.  I can see why the DJGPP port works the way it
does and caches the registers, but IMHO its way of handling things is
the exception to the way a UNIXoid OS handles this stuff.

   > By supplying all those registers at once, the
   > cache becomes some sort of read-ahead cache.  This makes sense since
   > GDB hardly ever needs only one register, and pre-fetching those saves
   > a few system calls.

   Well, I thought about this, and I'm still not convinced there's a gain
   here, at least not in all possible situations.

There hardly ever is :-(.

   First, if GDB needs more than one FP register, it could just pass -1 as
   REGNO and get them all.

Sure, but that would also re-fetch the generic registers, and the
GNU/Linux/x86 target for example doesn't do that if you request a
single FP register.

   Also, I can certainly think about a situation when GDB actually needs
   only one FP register.  Imagine that I need to put a watchpoint on such
   a register, for example, or display it at each stop.

Yep.  Its a trade off.  And in the DJGPP case there's not much to
gain.  But the cost of supplying those extra registers should be
almost negligable.  I really think that keeping the number of
interfaces down helps the long-time maintainablility of GDB and is
worth that cost.

   Btw, I found only two other platforms which currently use functions
   from i387-nat.c, so it doesn't seem too late to make changes.

True, but I'd really like to keep the symmetry between
supply_fpregset() and i387_supply_fsave() and fill_fpregset() and
i387_fill_fsave().

Mark

Re: [RFA]: New function to supply only one x87 register

[Andrew Cagney]

Mark Kettenis wrote:

> I just posted a message to the discussion list on this subject.  And
> no, I don't thing that the target end should cache the registers,
> since that would be duplicating what the existing register cache is
> already capable of.  I can see why the DJGPP port works the way it
> does and caches the registers, but IMHO its way of handling things is
> the exception to the way a UNIXoid OS handles this stuff.

To follow up on my other e-mail.  The method
target->fetch_regisers(RAWNUM) is expected to at supply a value for at
least REGNUM (or failing that, provide a dummy value and mark it as
unavailable).

	enjoy,
		Andrew