This is the mail archive of the newlib@sources.redhat.com mailing list for the newlib 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: Bug in vfprintf?


Try the attached patch with your testsuite.

-- Jeff J.

Charles Wilson wrote:
It appears that there is a bug in _vfprintf_r in stdio/vfprintf.c (or cvt() in that same file, or _ldtoa_r in stdlib/ltdoa.c, depending on your point of view).

I stumbled on this bug while running the selftests for glib. In glib's string-test, it calls vfprintf() with a fmt string:
"%s|%0100d|%s|%s|%0*d|%*.*f|%10000.10000f"

The culprit here is the "%10000.10000f" format specifier. vfprintf() calls cvt() for floating point, which calls _ldtoa_r(). However, _ldtoa_r overrides ndigits (e.g. 'prec') if it is greater than NDEC (42 on cygwin). However, cvt() and vfprintf() don't know that _ldtoa_r used "42" instead of "10000" -- and worse, they don't realize that _ldtoa_r allocated only slightly more than 42 bytes (52bytes on cygwin) for the digits string.

Thus, this code in fprintf.c cvt() generates a segfault:

1168 digits = _ldtoa_r(data, value, mode, ndigits, decpt, &dsgn, &rve);
1169 #endif /* !_NO_LONGDBL */
1170
1171 if ((ch != 'g' && ch != 'G') || flags & ALT)
1172 bp = digits + ndigits;

we set bp to something WAY past the end of digits. Since rve points to the actual end of digits (where the '\0' is)...

1173 if (ch == 'f') {
1174 if (*digits == '0' && value)
1175 *decpt = -ndigits + 1;
1176 bp += *decpt;
1177 }
1178 if (value == 0)
1179 rve = bp;
1180 while (rve < bp)
1181 *rve++ = '0';

this while loop (lines 1180-1181) attempts to put 0's into digits[43...10000]. But, there were only 24 + (2^3-1)*4 == 52 bytes allocated for digits.

I'm not sure what the correct fix is...should ndigits/prec be passed by reference to _ldtoa_r, so that cvt "knows" that it got changed? (Ditto pass-by-reference to cvt so that vfprintf_r knows about the override as well). This changes the signature of these two (admittedly internal) routines, but I'm not sure of the "ripple effects" such a change might cause.

Or is this a case of "doctor, it hurts when I do this?" "Don't call printf with prec specifiers greater than 42, then." That can't be good; memory corruption just because an internal newlib routine doesn't like the a given format spec?

This has been discussed somewhat in the following thread on the cygwin mailing list:
http://cygwin.com/ml/cygwin/2003-01/msg00023.html

--Chuck


Index: libc/stdlib/ldtoa.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdlib/ldtoa.c,v
retrieving revision 1.6
diff -u -r1.6 ldtoa.c
--- libc/stdlib/ldtoa.c	12 Nov 2002 21:47:53 -0000	1.6
+++ libc/stdlib/ldtoa.c	6 Jan 2003 23:19:59 -0000
@@ -41,6 +41,9 @@
  /* The exponent of 1.0 */
  #define EXONE (0x3fff)
 
+ /* Maximum exponent digits - base 10 */
+ #define MAX_EXP_DIGITS 5
+
 /* Control structure for long doublue conversion including rounding precision values.
  * rndprc can be set to 80 (if NE=6), 64, 56, 53, or 24 bits.
  */
@@ -2703,7 +2706,7 @@
 {
 unsigned short e[NI];
 char *s, *p;
-int k;
+int i, j, k;
 LDPARMS rnd;
 LDPARMS *ldp = &rnd;
 char *outstr;
@@ -2744,15 +2747,20 @@
    For now, just ask for 20 digits which is enough but sometimes too many.  */
 if( mode == 0 )
         ndigits = 20;
+
+/* reentrancy addition to use mprec storage pool */
+/* we want to have enough space to hold the formatted result */
+i = ndigits + (mode == 3 ? (MAX_EXP_DIGITS + 1) : 1);
+j = sizeof (__ULong);
+for (_REENT_MP_RESULT_K(ptr) = 0; sizeof (_Bigint) - sizeof (__ULong) + j <= i; j <<= 1)
+  _REENT_MP_RESULT_K(ptr)++;
+_REENT_MP_RESULT(ptr) = Balloc (ptr, _REENT_MP_RESULT_K(ptr));
+outstr = (char *)_REENT_MP_RESULT(ptr);
+
 /* This sanity limit must agree with the corresponding one in etoasc, to
    keep straight the returned value of outexpon.  */
 if( ndigits > NDEC )
         ndigits = NDEC;
-
-/* reentrancy addition to use mprec storage pool */
-_REENT_MP_RESULT(ptr) = Balloc (ptr, 3);
-_REENT_MP_RESULT_K(ptr) = 3;
-outstr = (char *)_REENT_MP_RESULT(ptr);
 
 etoasc( e, outstr, ndigits, mode, ldp );
 s =  outstr;

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