[PATCH] Fix VAX crash

[PATCH] Fix VAX crash

[Jason R Thorpe]

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

Richard...

In reference to:

	http://gcc.gnu.org/ml/gcc-bugs/2002-11/msg00244.html

...your REAL_TO_DECIMAL -> real_to_decimal change seems to be
the culprit.  The attached patch prevents the compiler from crashing,
although it's not clear to me why real_to_decimal isn't obeying
the passed-in buffer size..?

	* config/vax/vax.h (PRINT_OPERAND): Increase size of buffer
	provided to real_to_decimal.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: vax-real-patch --]
[-- Type: text/plain, Size: 1534 bytes --]

Index: config/vax/vax.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/vax/vax.h,v
retrieving revision 1.54
diff -c -r1.54 vax.h
*** config/vax/vax.h	22 Oct 2002 23:05:24 -0000	1.54
--- config/vax/vax.h	6 Nov 2002 07:05:01 -0000
***************
*** 1212,1223 ****
    else if (GET_CODE (X) == MEM)						\
      output_address (XEXP (X, 0));					\
    else if (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == SFmode)	\
!     { char dstr[30];							\
        real_to_decimal (dstr, CONST_DOUBLE_REAL_VALUE (X),		\
  		       sizeof (dstr), 0, 1);				\
        fprintf (FILE, "$0f%s", dstr); }					\
    else if (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == DFmode)	\
!     { char dstr[30];							\
        real_to_decimal (dstr, CONST_DOUBLE_REAL_VALUE (X),		\
  		       sizeof (dstr), 0, 1);				\
        fprintf (FILE, "$0%c%s", ASM_DOUBLE_CHAR, dstr); }		\
--- 1212,1223 ----
    else if (GET_CODE (X) == MEM)						\
      output_address (XEXP (X, 0));					\
    else if (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == SFmode)	\
!     { char dstr[50];							\
        real_to_decimal (dstr, CONST_DOUBLE_REAL_VALUE (X),		\
  		       sizeof (dstr), 0, 1);				\
        fprintf (FILE, "$0f%s", dstr); }					\
    else if (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == DFmode)	\
!     { char dstr[50];							\
        real_to_decimal (dstr, CONST_DOUBLE_REAL_VALUE (X),		\
  		       sizeof (dstr), 0, 1);				\
        fprintf (FILE, "$0%c%s", ASM_DOUBLE_CHAR, dstr); }		\

Re: [PATCH] Fix VAX crash

[Graham Stott]

Jason R Thorpe wrote:
> Richard...
> 
> In reference to:
> 
> 	http://gcc.gnu.org/ml/gcc-bugs/2002-11/msg00244.html
> 
> ...your REAL_TO_DECIMAL -> real_to_decimal change seems to be
> the culprit.  The attached patch prevents the compiler from crashing,
> although it's not clear to me why real_to_decimal isn't obeying
> the passed-in buffer size..?
> 
Taking a quick peek at the real_to_decimal routine I see a fair number of
places where it adds to the buffer without checking if there's any room left.


It's probably easier and simpler for the routine to use an internal
buffer that's always big enough to accumulate the result and then at
the end copy it to the output buffer truncating if necessary.

Graham

Re: [PATCH] Fix VAX crash

[Richard Henderson]

On Wed, Nov 06, 2002 at 07:32:30PM +0000, Graham Stott wrote:
> Taking a quick peek at the real_to_decimal routine I see a fair number of
> places where it adds to the buffer without checking if there's any room 
> left.

Such as?


r~

Re: [PATCH] Fix VAX crash

[Graham Stott]

Richard Henderson wrote:
> On Wed, Nov 06, 2002 at 07:32:30PM +0000, Graham Stott wrote:
> 
>>Taking a quick peek at the real_to_decimal routine I see a fair number of
>>places where it adds to the buffer without checking if there's any room 
>>left.
> 
> 
> Such as?
> 
> 
> r~
> 

Will these do?

Index: real.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/real.c,v
retrieving revision 1.103
diff -c -p -r1.103 real.c
*** real.c      27 Oct 2002 14:47:54 -0000      1.103
--- real.c      7 Nov 2002 01:07:49 -0000
*************** real_to_decimal (str, r_orig, buf_size,
*** 1649,1662 ****
     /* ... or overflow.  */
     if (digit == 10)
       {
!       *p++ = '1';
         if (--digits > 0)
         *p++ = '0';
         dec_exp += 1;
       }
     else if (digit > 10)
       abort ();
!   else
       *p++ = digit + '0';

     /* Generate subsequent digits.  */
--- 1649,1663 ----
     /* ... or overflow.  */
     if (digit == 10)
       {
!       if (--digits > 0)
!       *p++ = '1';
         if (--digits > 0)
         *p++ = '0';
         dec_exp += 1;
       }
     else if (digit > 10)
       abort ();
!   else if (--digits > 0)
       *p++ = digit + '0';

     /* Generate subsequent digits.  */

Re: [PATCH] Fix VAX crash

[Richard Henderson]

On Thu, Nov 07, 2002 at 01:12:30AM +0000, Graham Stott wrote:
>     if (digit == 10)
>       {
> !       if (--digits > 0)
> !       *p++ = '1';

digits can't be zero.  That's checked above.


r~

Re: [PATCH] Fix VAX crash

[Graham Stott]

Richard Henderson wrote:
> On Thu, Nov 07, 2002 at 01:12:30AM +0000, Graham Stott wrote:
> 
>>    if (digit == 10)
>>      {
>>!       if (--digits > 0)
>>!       *p++ = '1';
> 
> 
> digits can't be zero.  That's checked above.
> 
> 
> r~
> 

Are you confusing digit with digits?

Graham

Re: [PATCH] Fix VAX crash

[Richard Henderson]

On Thu, Nov 07, 2002 at 06:30:23PM +0000, Graham Stott wrote:
> Are you confusing digit with digits?

No.

  if (digits == 0 || digits > max_digits)
    digits = max_digits;



r~

Re: [PATCH] Fix VAX crash

[Graham Stott]

Richard Henderson wrote:
> On Thu, Nov 07, 2002 at 06:30:23PM +0000, Graham Stott wrote:
> 
>>Are you confusing digit with digits?
> 
> 
> No.
> 
>   if (digits == 0 || digits > max_digits)
>     digits = max_digits;
> 
> 
> 
> r~
> 

Ok but we still have to account for the digits added to the buffer
otherwise we can end up exceeding the buffer later.

Graham

Re: [PATCH] Fix VAX crash

[Richard Henderson]

On Thu, Nov 07, 2002 at 06:41:16PM +0000, Graham Stott wrote:
> Ok but we still have to account for the digits added to the buffer
> otherwise we can end up exceeding the buffer later.

That's why the loop below is a while, not a do/while.


r~

Re: [PATCH] Fix VAX crash

[Graham Stott]

Richard Henderson wrote:
> On Thu, Nov 07, 2002 at 06:41:16PM +0000, Graham Stott wrote:
> 
>>Ok but we still have to account for the digits added to the buffer
>>otherwise we can end up exceeding the buffer later.
> 
> 
> That's why the loop below is a while, not a do/while.
> 
> 
> r~
> 

OK taking a closer look at the code I can now see why it's possible to
over flow the buffer. The problem is in the following code which is
trying to limit digits to the min of either buf_size and representation size.

------------------------------------------------------------------------------
   /* Estimate the decimal exponent, and compute the length of the string it
      will print as.  Be conservative and add one to account for possible
      overflow or rounding error.  */
   dec_exp = r.exp * M_LOG10_2;
   for (max_digits = 1; dec_exp ; max_digits++)
     dec_exp /= 10;

   /* Bound the number of digits printed by the size of the output buffer.  */
   max_digits = buf_size - 1 - 1 - 2 - max_digits - 1;
   if (max_digits > buf_size)
     abort ();
   if (digits > max_digits)
     digits = max_digits;

   /* Bound the number of digits printed by the size of the representation.  */
   max_digits = SIGNIFICAND_BITS * M_LOG10_2;
   if (digits == 0 || digits > max_digits)
     digits = max_digits;
--------------------------------------------------------------------------------

The problem is the last 3 lines which bounds to representation is always
setting digits to the representation size when digits is zero, which totally
ignores the buffer size restriction.

The problem triggered on the vax becuase the buffer size was 30 which was
smaller than the representation size 48.

The solution is to bound to the representation first and then bound to the
buffer size.

The following patch does this  bootstrapped xi686-pc-linux-gnu all languages
noregression. I have also built cc1 for for arm, ip2k, h8300, mn10300, v850,
xscale, ppceabi, s390, sparc, avr, alpha, cris, d30v, ia64, m68k, mmix, mips,
sh and stormy16.

Ok for mainline?

Graham

ChangeLog

        * real.c (real_to_decimal): Fix buffer overrun when buffer size if
        smaller than representation.

-----------------------------------------------------------------------------
Index: real.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/real.c,v
retrieving revision 1.103
diff -c -p -r1.103 real.c
*** real.c      27 Oct 2002 14:47:54 -0000      1.103
--- real.c      8 Nov 2002 18:34:21 -0000
*************** real_to_decimal (str, r_orig, buf_size,
*** 1485,1490 ****
--- 1485,1495 ----
         abort ();
       }

+   /* Bound the number of digits printed by the size of the representation.  */
+   max_digits = SIGNIFICAND_BITS * M_LOG10_2;
+   if (digits == 0 || digits > max_digits)
+     digits = max_digits;
+
     /* Estimate the decimal exponent, and compute the length of the string it
        will print as.  Be conservative and add one to account for possible
        overflow or rounding error.  */
*************** real_to_decimal (str, r_orig, buf_size,
*** 1497,1507 ****
     if (max_digits > buf_size)
       abort ();
     if (digits > max_digits)
-     digits = max_digits;
-
-   /* Bound the number of digits printed by the size of the representation.  */
-   max_digits = SIGNIFICAND_BITS * M_LOG10_2;
-   if (digits == 0 || digits > max_digits)
       digits = max_digits;

     one = real_digit (1);
--- 1502,1507 ----
-----------------------------------------------------------------------------

Re: [PATCH] Fix VAX crash

[Richard Henderson]

On Fri, Nov 08, 2002 at 07:11:26PM +0000, Graham Stott wrote:
>        * real.c (real_to_decimal): Fix buffer overrun when buffer size if
>        smaller than representation.

Ok.


r~