This is the mail archive of the
newlib@sources.redhat.com
mailing list for the newlib project.
Re: Bug in vfprintf?
- From: "J. Johnston" <jjohnstn at redhat dot com>
- To: Charles Wilson <cwilson at ece dot gatech dot edu>
- Cc: newlib at sources dot redhat dot com
- Date: Mon, 06 Jan 2003 18:22:26 -0500
- Subject: Re: Bug in vfprintf?
- Organization: Red Hat Inc.
- References: <3E13D019.80704@ece.gatech.edu>
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;