This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, libiberty] Fix segfault in floatformat.c:get_field on 64-bit hosts


Hi,

Ian Lance Taylor wrote:
Please compile the file as a standalone program with -DIEEE_DEBUG to
make sure those tests still work.  Ideally on both a big- and
little-endian system, if possible.

I've done this (with a new put_field too), though the IEEE_DEBUG test needs explicitly changing to use floatformat_ieee_double_big not floatformat_ieee_double_little on a big-endian host, which wasn't immediately obvious...


This patch is OK with that change, assuming the tests pass.

Bonus points if you rewrite put_field.

"put_field" indeed crashed too on x86_64 with IEEE_DEBUG defined, so I've rewritten it in a similar style. Just to be sure, is this still OK (for gcc and/or binutils)?


Cheers,

Julian

ChangeLog (libiberty):

    * floatformat.c (get_field): Fix segfault with little-endian word
    order on 64-bit hosts.
    (put_field): Likewise.
    (min): Move definition.
Index: floatformat.c
===================================================================
RCS file: /cvs/src/src/libiberty/floatformat.c,v
retrieving revision 1.20
diff -c -p -r1.20 floatformat.c
*** floatformat.c	24 Apr 2006 21:34:41 -0000	1.20
--- floatformat.c	15 Jun 2006 22:02:54 -0000
*************** const struct floatformat floatformat_ia6
*** 249,301 ****
    floatformat_always_valid
  };
  
  /* Extract a field which starts at START and is LEN bits long.  DATA and
     TOTAL_LEN are the thing we are extracting it from, in byteorder ORDER.  */
  static unsigned long
  get_field (const unsigned char *data, enum floatformat_byteorders order,
             unsigned int total_len, unsigned int start, unsigned int len)
  {
!   unsigned long result;
    unsigned int cur_byte;
!   int cur_bitshift;
  
    /* Start at the least significant part of the field.  */
-   cur_byte = (start + len) / FLOATFORMAT_CHAR_BIT;
    if (order == floatformat_little)
!     cur_byte = (total_len / FLOATFORMAT_CHAR_BIT) - cur_byte - 1;
!   cur_bitshift =
!     ((start + len) % FLOATFORMAT_CHAR_BIT) - FLOATFORMAT_CHAR_BIT;
!   result = *(data + cur_byte) >> (-cur_bitshift);
!   cur_bitshift += FLOATFORMAT_CHAR_BIT;
!   if (order == floatformat_little)
!     ++cur_byte;
    else
!     --cur_byte;
  
!   /* Move towards the most significant part of the field.  */
!   while ((unsigned int) cur_bitshift < len)
      {
!       if (len - cur_bitshift < FLOATFORMAT_CHAR_BIT)
! 	/* This is the last byte; zero out the bits which are not part of
! 	   this field.  */
! 	result |=
! 	  (*(data + cur_byte) & ((1 << (len - cur_bitshift)) - 1))
! 	    << cur_bitshift;
!       else
! 	result |= *(data + cur_byte) << cur_bitshift;
!       cur_bitshift += FLOATFORMAT_CHAR_BIT;
!       if (order == floatformat_little)
! 	++cur_byte;
!       else
! 	--cur_byte;
      }
    return result;
  }
    
- #ifndef min
- #define min(a, b) ((a) < (b) ? (a) : (b))
- #endif
- 
  /* Convert from FMT to a double.
     FROM is the address of the extended float.
     Store the double in *TO.  */
--- 249,299 ----
    floatformat_always_valid
  };
  
+ 
+ #ifndef min
+ #define min(a, b) ((a) < (b) ? (a) : (b))
+ #endif
+ 
  /* Extract a field which starts at START and is LEN bits long.  DATA and
     TOTAL_LEN are the thing we are extracting it from, in byteorder ORDER.  */
  static unsigned long
  get_field (const unsigned char *data, enum floatformat_byteorders order,
             unsigned int total_len, unsigned int start, unsigned int len)
  {
!   unsigned long result = 0;
    unsigned int cur_byte;
!   int lo_bit, hi_bit, cur_bitshift = 0;
!   int nextbyte = (order == floatformat_little) ? 1 : -1;
! 
!   /* Start is in big-endian bit order!  Fix that first.  */
!   start = total_len - (start + len);
  
    /* Start at the least significant part of the field.  */
    if (order == floatformat_little)
!     cur_byte = start / FLOATFORMAT_CHAR_BIT;
    else
!     cur_byte = (total_len - start - 1) / FLOATFORMAT_CHAR_BIT;
  
!   lo_bit = start % FLOATFORMAT_CHAR_BIT;
!   hi_bit = min (lo_bit + len, FLOATFORMAT_CHAR_BIT);
!   
!   do
      {
!       unsigned int shifted = *(data + cur_byte) >> lo_bit;
!       unsigned int bits = hi_bit - lo_bit;
!       unsigned int mask = (1 << bits) - 1;
!       result |= (shifted & mask) << cur_bitshift;
!       len -= bits;
!       cur_bitshift += bits;
!       cur_byte += nextbyte;
!       lo_bit = 0;
!       hi_bit = min (len, FLOATFORMAT_CHAR_BIT);
      }
+   while (len != 0);
+ 
    return result;
  }
    
  /* Convert from FMT to a double.
     FROM is the address of the extended float.
     Store the double in *TO.  */
*************** put_field (unsigned char *data, enum flo
*** 428,470 ****
             unsigned long stuff_to_put)
  {
    unsigned int cur_byte;
!   int cur_bitshift;
  
    /* Start at the least significant part of the field.  */
-   cur_byte = (start + len) / FLOATFORMAT_CHAR_BIT;
-   if (order == floatformat_little)
-     cur_byte = (total_len / FLOATFORMAT_CHAR_BIT) - cur_byte - 1;
-   cur_bitshift =
-     ((start + len) % FLOATFORMAT_CHAR_BIT) - FLOATFORMAT_CHAR_BIT;
-   *(data + cur_byte) &=
-     ~(((1 << ((start + len) % FLOATFORMAT_CHAR_BIT)) - 1) << (-cur_bitshift));
-   *(data + cur_byte) |=
-     (stuff_to_put & ((1 << FLOATFORMAT_CHAR_BIT) - 1)) << (-cur_bitshift);
-   cur_bitshift += FLOATFORMAT_CHAR_BIT;
    if (order == floatformat_little)
!     ++cur_byte;
    else
!     --cur_byte;
  
!   /* Move towards the most significant part of the field.  */
!   while ((unsigned int) cur_bitshift < len)
      {
!       if (len - cur_bitshift < FLOATFORMAT_CHAR_BIT)
! 	{
! 	  /* This is the last byte.  */
! 	  *(data + cur_byte) &=
! 	    ~((1 << (len - cur_bitshift)) - 1);
! 	  *(data + cur_byte) |= (stuff_to_put >> cur_bitshift);
! 	}
!       else
! 	*(data + cur_byte) = ((stuff_to_put >> cur_bitshift)
! 			      & ((1 << FLOATFORMAT_CHAR_BIT) - 1));
!       cur_bitshift += FLOATFORMAT_CHAR_BIT;
!       if (order == floatformat_little)
! 	++cur_byte;
!       else
! 	--cur_byte;
      }
  }
  
  /* The converse: convert the double *FROM to an extended float
--- 426,459 ----
             unsigned long stuff_to_put)
  {
    unsigned int cur_byte;
!   int lo_bit, hi_bit;
!   int nextbyte = (order == floatformat_little) ? 1 : -1;
! 
!   /* Start is in big-endian bit order!  Fix that first.  */
!   start = total_len - (start + len);
  
    /* Start at the least significant part of the field.  */
    if (order == floatformat_little)
!     cur_byte = start / FLOATFORMAT_CHAR_BIT;
    else
!     cur_byte = (total_len - start - 1) / FLOATFORMAT_CHAR_BIT;
  
!   lo_bit = start % FLOATFORMAT_CHAR_BIT;
!   hi_bit = min (lo_bit + len, FLOATFORMAT_CHAR_BIT);
!   
!   do
      {
!       unsigned char *byte_ptr = data + cur_byte;
!       unsigned int bits = hi_bit - lo_bit;
!       unsigned int mask = ((1 << bits) - 1) << lo_bit;
!       *byte_ptr = (*byte_ptr & ~mask) | ((stuff_to_put << lo_bit) & mask);
!       stuff_to_put >>= bits;
!       len -= bits;
!       cur_byte += nextbyte;
!       lo_bit = 0;
!       hi_bit = min (len, FLOATFORMAT_CHAR_BIT);
      }
+   while (len != 0);
  }
  
  /* The converse: convert the double *FROM to an extended float

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