This is the mail archive of the gdb-patches@sourceware.cygnus.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

RFA: fixing partial register reads/writes



Just a fix for a dumb bug.

The old approach tested whether either of the register's endpoints
fell within the region being read/written.  This fails when the region
lies completely within a register --- for example, accessing one
element of a SIMD register.

The new code uses tests which are accurate.

1999-11-22  Jim Blandy  <jimb@cygnus.com>

	* findvar.c (read_register_bytes, write_register_bytes): Correctly
	determine how the region the caller is writing overlaps with each
	register's bytes.

Index: findvar.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/findvar.c,v
retrieving revision 1.108
diff -c -c -b -F'^(' -r1.108 findvar.c
*** findvar.c	1999/11/17 07:31:11	1.108
--- findvar.c	1999/11/22 23:54:43
***************
*** 661,680 ****
      register_valid[i] = 1;
  }
  
! /* read_register_bytes and write_register_bytes are generally a *BAD* idea.
!    They are inefficient because they need to check for partial updates, which
!    can only be done by scanning through all of the registers and seeing if the
!    bytes that are being read/written fall inside of an invalid register.  [The
!    main reason this is necessary is that register sizes can vary, so a simple
!    index won't suffice.]  It is far better to call read_register_gen if you
!    want to get at the raw register contents, as it only takes a regno as an
!    argument, and therefore can't do a partial register update.  It would also
!    be good to have a write_register_gen for similar reasons.
! 
!    Prior to the recent fixes to check for partial updates, both read and
!    write_register_bytes always checked to see if any registers were stale, and
!    then called target_fetch_registers (-1) to update the whole set.  This
!    caused really slowed things down for remote targets.  */
  
  /* Copy INLEN bytes of consecutive data from registers
     starting with the INREGBYTE'th byte of register data
--- 661,682 ----
      register_valid[i] = 1;
  }
  
! /* read_register_bytes and write_register_bytes are generally a *BAD*
!    idea.  They are inefficient because they need to check for partial
!    updates, which can only be done by scanning through all of the
!    registers and seeing if the bytes that are being read/written fall
!    inside of an invalid register.  [The main reason this is necessary
!    is that register sizes can vary, so a simple index won't suffice.]
!    It is far better to call read_register_gen and write_register_gen
!    if you want to get at the raw register contents, as it only takes a
!    regno as an argument, and therefore can't do a partial register
!    update.
! 
!    Prior to the recent fixes to check for partial updates, both read
!    and write_register_bytes always checked to see if any registers
!    were stale, and then called target_fetch_registers (-1) to update
!    the whole set.  This caused really slowed things down for remote
!    targets.  */
  
  /* Copy INLEN bytes of consecutive data from registers
     starting with the INREGBYTE'th byte of register data
***************
*** 701,707 ****
    for (regno = 0; regno < NUM_REGS; regno++)
      {
        int regstart, regend;
-       int startin, endin;
  
        if (register_valid[regno])
  	continue;
--- 703,708 ----
***************
*** 712,726 ****
        regstart = REGISTER_BYTE (regno);
        regend = regstart + REGISTER_RAW_SIZE (regno);
  
!       startin = regstart >= inregbyte && regstart < inregend;
!       endin = regend > inregbyte && regend <= inregend;
! 
!       if (!startin && !endin)
  	continue;
  
        /* We've found an invalid register where at least one byte will be read.
           Update it from the target.  */
- 
        target_fetch_registers (regno);
  
        if (!register_valid[regno])
--- 713,724 ----
        regstart = REGISTER_BYTE (regno);
        regend = regstart + REGISTER_RAW_SIZE (regno);
  
!       if (regend <= inregbyte || inregend <= regstart)
! 	/* The range the user wants to read doesn't overlap with regno.  */
  	continue;
  
        /* We've found an invalid register where at least one byte will be read.
           Update it from the target.  */
        target_fetch_registers (regno);
  
        if (!register_valid[regno])
***************
*** 813,851 ****
    for (regno = 0; regno < NUM_REGS; regno++)
      {
        int regstart, regend;
-       int startin, endin;
-       char regbuf[MAX_REGISTER_RAW_SIZE];
  
        regstart = REGISTER_BYTE (regno);
        regend = regstart + REGISTER_RAW_SIZE (regno);
  
!       startin = regstart >= myregstart && regstart < myregend;
!       endin = regend > myregstart && regend <= myregend;
  
!       if (!startin && !endin)
! 	continue;		/* Register is completely out of range */
  
!       if (startin && endin)	/* register is completely in range */
  	{
! 	  write_register_gen (regno, myaddr + (regstart - myregstart));
! 	  continue;
! 	}
  
!       /* We may be doing a partial update of an invalid register.  Update it
!          from the target before scribbling on it.  */
        read_register_gen (regno, regbuf);
  
-       if (startin)
- 	memcpy (registers + regstart,
- 		myaddr + regstart - myregstart,
- 		myregend - regstart);
-       else			/* endin */
- 	memcpy (registers + myregstart,
- 		myaddr,
- 		regend - myregstart);
        target_store_registers (regno);
      }
  }
  
  /* Return the raw contents of register REGNO, regarding it as an integer.  */
  /* This probably should be returning LONGEST rather than CORE_ADDR.  */
--- 811,850 ----
    for (regno = 0; regno < NUM_REGS; regno++)
      {
        int regstart, regend;
  
        regstart = REGISTER_BYTE (regno);
        regend = regstart + REGISTER_RAW_SIZE (regno);
  
!       /* Is this register completely outside the range the user is writing?  */
!       if (myregend <= regstart || regend <= myregstart)
! 	/* do nothing */ ;		
  
!       /* Is this register completely within the range the user is writing?  */
!       else if (myregstart <= regstart && regend <= myregend)
! 	write_register_gen (regno, myaddr + (regstart - myregstart));
  
!       /* The register partially overlaps the range being written.  */
!       else
  	{
! 	  char regbuf[MAX_REGISTER_RAW_SIZE];
! 	  /* What's the overlap between this register's bytes and
!              those the caller wants to write?  */
! 	  int overlapstart = max (regstart, myregstart);
! 	  int overlapend   = min (regend,   myregend);
  
! 	  /* We may be doing a partial update of an invalid register.
! 	     Update it from the target before scribbling on it.  */
  	  read_register_gen (regno, regbuf);
+ 
+ 	  memcpy (registers + overlapstart,
+ 		  myaddr + (overlapstart - myregstart),
+ 		  overlapend - overlapstart);
  
  	  target_store_registers (regno);
  	}
+     }
  }
+ 
  
  /* Return the raw contents of register REGNO, regarding it as an integer.  */
  /* This probably should be returning LONGEST rather than CORE_ADDR.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]