This is the mail archive of the
newlib@sources.redhat.com
mailing list for the newlib project.
Re: Bug in vfprintf?
Ok, another patch for you. The previous patch was not robust enough.
The allocation of the buffer cannot occur until we know the size
of the result so we need to do this at the end of the routine instead
of the start. We can use a fixed-length local buffer up to that point
which is reasonable in size.
-- Jeff J.
Jason Tishler wrote:
Jeff,
On Tue, Jan 07, 2003 at 02:56:20PM -0500, J. Johnston wrote:
Jason Tishler wrote:
On Mon, Jan 06, 2003 at 10:05:19PM -0500, Charles Wilson wrote:
J. Johnston wrote:
Try the attached patch with your testsuite.
That fixes it. Thanks.
It also fixes the following too:
http://cygwin.com/ml/cygwin/2003-01/msg00047.html
Good. Patch has been checked in.
Changes to the Python regression test have uncovered another memory
overwrite problem in this area of newlib. The attached C program,
test_string5.c, is a minimal testcase that demonstrates the problem
(albeit, most likely only noticeable in a debugger).
Use the following invocation:
test_string5 '%.50f'
to trigger the problem.
The following is an excerpt from libc/stdio/vfprintf.c:
1022 static char *
1023 cvt(data, value, ndigits, flags, sign, decpt, ch, length)
...
1069 if ((ch != 'g' && ch != 'G') || flags & ALT) { /* Print ...
*>1070 bp = digits + ndigits;
1071 if (ch == 'f') {
1072 if (*digits == '0' && value)
1073 *decpt = -ndigits + 1;
*>1074 bp += *decpt;
1075 }
1076 if (value == 0) /* kludge for __dtoa irregularity */
1077 rve = bp;
1078 while (rve < bp)
1079 *rve++ = '0';
1080 }
I'm not sure of the root cause, but line 1074 pushes bp to past the end
of allocated memory which is 84 bytes in this testcase. The combination
of lines 1070 and 1074 cause the string to be padding to a field width
of 100 instead of the requested 50 which overwrites adjacent memory.
I can "fix" the problem by allocating more memory than necessary in
_ldtoa_r(), but that is probably not the best way to solve this problem.
I hope that this post will enable you to easily find the best solution.
If you are too busy to address this problem, let me know and I will try
to dig deeper and fix it myself.
Thanks,
Jason
------------------------------------------------------------------------
#include <stdio.h>
int
main(int argc, char* argv[])
{
const char* format = argv[1];
double value = 10033002222963781815252827772780694713070670000000.0;
printf(format, value);
printf("\n");
return 0;
}
Index: ldtoa.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdlib/ldtoa.c,v
retrieving revision 1.7
diff -u -r1.7 ldtoa.c
--- ldtoa.c 7 Jan 2003 19:52:27 -0000 1.7
+++ ldtoa.c 1 Feb 2003 00:52:49 -0000
@@ -2707,10 +2707,13 @@
unsigned short e[NI];
char *s, *p;
int i, j, k;
+int orig_ndigits;
LDPARMS rnd;
LDPARMS *ldp = &rnd;
char *outstr;
+char outbuf[NDEC + MAX_EXP_DIGITS + 10];
+orig_ndigits = ndigits;
rnd.rlast = -1;
rnd.rndprc = NBITS;
@@ -2748,22 +2751,13 @@
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;
-etoasc( e, outstr, ndigits, mode, ldp );
-s = outstr;
+etoasc( e, outbuf, ndigits, mode, ldp );
+s = outbuf;
if( eisinf(e) || eisnan(e) )
{
*decpt = 9999;
@@ -2774,7 +2768,7 @@
/* Transform the string returned by etoasc into what the caller wants. */
/* Look for decimal point and delete it from the string. */
-s = outstr;
+s = outbuf;
while( *s != '\0' )
{
if( *s == '.' )
@@ -2795,19 +2789,19 @@
nodecpt:
/* Back up over the exponent field. */
-while( *s != 'E' && s > outstr)
+while( *s != 'E' && s > outbuf)
--s;
*s = '\0';
stripspaces:
/* Strip leading spaces and sign. */
-p = outstr;
+p = outbuf;
while( *p == ' ' || *p == '-')
++p;
/* Find new end of string. */
-s = outstr;
+s = outbuf;
while( (*s++ = *p++) != '\0' )
;
--s;
@@ -2820,20 +2814,38 @@
else
k = ldp->outexpon;
-while( *(s-1) == '0' && ((s - outstr) > k))
+while( *(s-1) == '0' && ((s - outbuf) > k))
*(--s) = '\0';
/* In f format, flush small off-scale values to zero.
Rounding has been taken care of by etoasc. */
if( mode == 3 && ((ndigits + ldp->outexpon) < 0))
{
- s = outstr;
+ s = outbuf;
*s = '\0';
*decpt = 0;
}
+/* reentrancy addition to use mprec storage pool */
+/* we want to have enough space to hold the formatted result */
+
+if (mode == 3) /* f format, account for sign + dec digits + decpt + frac */
+ i = *decpt + orig_ndigits + 3;
+else /* account for sign + max precision digs + E + exp sign + exponent */
+ i = ndigits + MAX_EXP_DIGITS + 4;
+
+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));
+
+/* Copy from internal temporary buffer to permanent buffer. */
+outstr = (char *)_REENT_MP_RESULT(ptr);
+strcpy (outstr, outbuf);
+
if( rve )
- *rve = s;
+ *rve = outstr + (s - outbuf);
+
return outstr;
}