Date: Wed, 16 Jun 1999 13:50:06 +0300 (IDT)
From: Eli Zaretskii <eliz AT is DOT elta DOT co DOT il>
X-Sender: eliz AT is
To: pavenis AT lanet DOT lv, Robert Hoehne <robert DOT hoehne AT gmx DOT net>
cc: djgpp-workers AT delorie DOT com, muller AT cerbere DOT u-strasbg DOT fr
Subject: Re: save_npx and FSDB/Edebug32
In-Reply-To: <B0000090419@stargate.astr.lu.lv>
Message-ID: <Pine.SUN.3.91.990616133452.27724C-100000@is>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from QUOTED-PRINTABLE to 8bit by delorie.com id GAA26445
Reply-To: djgpp-workers AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk


On Mon, 14 Jun 1999 pavenis AT lanet DOT lv wrote:

> > > However I think that
> > > having 2 FPU states (one for debugger, second for debugee) which are 
> > > swapped before and after running child (as suggested in one of earlier
> > > posts to this mailing list) would be better.  
> > 
> > I agree.  But this should probably wait for a later DJGPP version.
> > 
> 
> Here is my patch. I tested it yet with FSDB only.

Thanks, I checked this in.

However, this was not enough to make debuggers work correctly with FP
programs.  I also needed to remove all the calls to save_npx and
load_npx in EDEBUG32.  For some reason, these calls were removed from
FSDB, but not from EDEBUG.  Robert, is there any reason for this?

And while at that, I removed old, pre-v2 code in FSDB that worked
around the lack of support in sprintf for long doubles.

I also found another bug in dbgcom.c: the extended version of load_npx
there was overwriting the values in npx.reg[] with the values from
either npx.st[] or npx.mmx[], depending on the FPU mode.  This breaks
the debuggers' features that let you change the values in FP
registers, since no debugger updates the shadow values in npx.mmx[]
and npx.st[].  To see the effect, try "set $st0 = 0.666" in GDB and
then "nexti": you will see that the assignment of 0.666 had no effect.
Similar effects are seen in FSDB if you go to the NPX pane and put a 
number into some FP register.

I disabled the code that restores values from st[] and mmx[] members, 
because I think it's a mistake to force the debugger to update all
three views of the FP registers: these alternate views exist to ease
the task of displaying the same registers in 3 different formats, so
they should be read-only.

(Btw, if some debugger uses the mmx[] view of the FP registers, it 
will probably be confused, since FNSAVE copies the FP registers in the
TOP-relative order, whereas the MMX registers are always addressed in
a fixed physical order.)

I attach below all of the changes that I checked in today.

Index: djgpp/src/debug/common/dbgcom.c
===================================================================
RCS file: /cvs/djgpp/djgpp/src/debug/common/dbgcom.c,v
retrieving revision 1.9
diff -c -r1.9 dbgcom.c
*** dbgcom.c	1999/06/06 07:50:38	1.9
--- dbgcom.c	1999/06/16 09:32:30
***************
*** 86,91 ****
--- 86,92 ----
  #endif
  
  NPX npx;
+ NPX debugger_npx;
  
  /* ------------------------------------------------------------------------- */
  /* Store the contents of the NPX in the global variable `npx'.  */
***************
*** 145,153 ****
     {
      npx.mmx[i]= * (long double *) &(npx.reg[i]);
     }
!    
!   /* asm("frstor %0" : :"m" (npx)); */
!   /* if we do this in mmx mode the fpu will be unusable */
  #endif
  }
  /* ------------------------------------------------------------------------- */
--- 146,153 ----
     {
      npx.mmx[i]= * (long double *) &(npx.reg[i]);
     }
!  /* Restore debugger's FPU state.  */
!  asm("frstor %0" : :"m" (debugger_npx));
  #endif
  }
  /* ------------------------------------------------------------------------- */
***************
*** 155,165 ****
  
  void load_npx (void)
  {
-   int i;
    if ((__dpmi_get_coprocessor_status() & FPU_PRESENT) == 0)
      return;
    if (npx.in_mmx_mode)
     {
      /* change reg to mmx */
      for (i=0;i<8;i++)
       if (npx.mmx[i]!= * (long double *) &(npx.reg[i]))
--- 155,177 ----
  
  void load_npx (void)
  {
    if ((__dpmi_get_coprocessor_status() & FPU_PRESENT) == 0)
      return;
+   /* Save debugger's FPU state.  */
+   asm ("fnsave %0" : :"m" (debugger_npx));
+ #if 0
+   /* This code is disabled because npx.mmx[] and npx.st[] are supposed
+      to be read-only, they exist to make it easier for a debugger to
+      display the FP registers either as long doubles or as 64-bit MMX
+      registers.  If the debugger wants to *change* the values, it
+      should always change in npx.reg[].  Otherwise, we will need a
+      whole slew of flags to know which one of the different views
+      should be used to restore child's FPU state, or else the debugger
+      will be forced to handle the extra burden of copying the same
+      value into each one of the three views of the same registers.  */
    if (npx.in_mmx_mode)
     {
+      int i;
      /* change reg to mmx */
      for (i=0;i<8;i++)
       if (npx.mmx[i]!= * (long double *) &(npx.reg[i]))
***************
*** 169,174 ****
--- 181,187 ----
     }
    else
     {
+      int i;
      /* change reg to st */
      for (i=0;i<8;i++)
        {
***************
*** 178,183 ****
--- 191,197 ----
           }
        }
     }
+ #endif
    asm ("frstor %0" : "=m" (npx));
  }
  
***************
*** 1255,1262 ****
    app_ds = a_tss.tss_ds;
    app_cs = a_tss.tss_cs;
    edi.app_base = 0;
    memset(&npx,0,sizeof(npx));
!   save_npx();	/* FIXME!! */
    /* Save all the changed signal handlers */
    oldTRAP = signal(SIGTRAP, dbgsig);
    oldSEGV = signal(SIGSEGV, dbgsig);
--- 1269,1280 ----
    app_ds = a_tss.tss_ds;
    app_cs = a_tss.tss_cs;
    edi.app_base = 0;
+   /* Save debugger's FPU state.  */
+   asm ("fnsave %0" : :"m" (debugger_npx));
+   /* Fill the debuggee's FPU state with the default values, taken from
+      the equivalent of FNINIT performed by FNSAVE above.  */
    memset(&npx,0,sizeof(npx));
!   save_npx();
    /* Save all the changed signal handlers */
    oldTRAP = signal(SIGTRAP, dbgsig);
    oldSEGV = signal(SIGSEGV, dbgsig);
Index: djgpp/src/debug/fsdb/fullscr.c
===================================================================
RCS file: /cvs/djgpp/djgpp/src/debug/fsdb/fullscr.c,v
retrieving revision 1.4
diff -c -r1.4 fullscr.c
*** fullscr.c	1999/03/22 09:30:24	1.4
--- fullscr.c	1999/06/16 09:33:03
***************
*** 1615,1622 ****
  	    if (abs (exp) < 1000)
  	      {
  		d = *((long double*)(npx.reg + i));
! 		/* sprintf does not (djgpp 1.11m3) handle long doubles.	 */
! 		sprintf(dstr,"%+.16g", (double) d);
  	      }
  	    else
  	      sprintf (dstr, "Valid, %s, and %s",
--- 1615,1621 ----
  	    if (abs (exp) < 1000)
  	      {
  		d = *((long double*)(npx.reg + i));
! 		sprintf(dstr,"%+.19Lg", (double) d);
  	      }
  	    else
  	      sprintf (dstr, "Valid, %s, and %s",
***************
*** 3013,3019 ****
  	if (regp)
  	  {
  	    char s[2], *endp, *p;
! 	    double d;
  
  	    s[0] = key; s[1] = '\0';
  	    if (!read_string (key == '=' ? "" : s) && read_buffer[0] != '\0')
--- 3012,3018 ----
  	if (regp)
  	  {
  	    char s[2], *endp, *p;
! 	    long double d;
  
  	    s[0] = key; s[1] = '\0';
  	    if (!read_string (key == '=' ? "" : s) && read_buffer[0] != '\0')
***************
*** 3038,3050 ****
  		  }
  		else
  		  {
! 		    d = strtod (p, &endp);
  		    if (*p != '\0' && *endp)
  		      message (CL_Error, "Expression not understood");
  		    else
  		      {
  			tag = (d == 0.0);
! 			*((long double *)(npx.reg + reg)) = (long double) d;
  			npx.reg[reg].sign = (*p == '-'); /* for -Zero */
  		      }
  		  }
--- 3037,3049 ----
  		  }
  		else
  		  {
! 		    d = _strtold (p, &endp);
  		    if (*p != '\0' && *endp)
  		      message (CL_Error, "Expression not understood");
  		    else
  		      {
  			tag = (d == 0.0);
! 			*((long double *)(npx.reg + reg)) = d;
  			npx.reg[reg].sign = (*p == '-'); /* for -Zero */
  		      }
  		  }
Index: djgpp/src/debug/edebug/debug.c
===================================================================
RCS file: /cvs/djgpp/djgpp/src/debug/edebug/debug.c,v
retrieving revision 1.4
diff -c -r1.4 debug.c
*** debug.c	1999/06/03 17:27:33	1.4
--- debug.c	1999/06/16 09:33:31
***************
*** 24,30 ****
  #include "debug.h"
  #include "unassmbl.h"
  #include <debug/syms.h>
- #include <debug/dbgcom.h>
  
  static char char32spc[] = "xxxúxxxúxxxúxxxùxxxúxxxúxxxúxxx ";
  
--- 24,29 ----
***************
*** 214,220 ****
        printf("Keyboard interrupt\n");
      else if (i == 0x75)
      {
-       save_npx();
        printf("Numeric Exception (");
        if ((npx.status & 0x0241) == 0x0241)
          printf("stack overflow");
--- 213,218 ----
***************
*** 234,240 ****
          printf("loss of precision");
        printf(") at eip=0x%08lx\n", npx.eip);
        unassemble(npx.eip, 0);
-       load_npx();
      }
      else
      {
--- 232,237 ----
***************
*** 742,748 ****
          }
          break;
        case XNPX:
-         save_npx();
          printf("Control: 0x%04lx  Status: 0x%04lx  Tag: 0x%04lx\n",
                 npx.control & 0xffff, npx.status & 0xffff, npx.tag & 0xffff);
          for (i=0; i<8; i++)
--- 739,744 ----
***************
*** 791,797 ****
                break;
            }
          }
-         load_npx();
          break;
  
        case QUIT:
--- 787,792 ----