public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ldtoa: Fix insufficient valid output digits for "%f" format.
@ 2021-11-25 12:02 Takashi Yano
  2021-11-25 14:09 ` Corinna Vinschen
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Yano @ 2021-11-25 12:02 UTC (permalink / raw)
  To: newlib

- If the number has large integer part and small fraction part is
  specified in output format, e.g. printf("%.3f", sqrt(2)*1e60);,
  valid output digits were insufficient. This patch fixes the issue.
---
 newlib/libc/stdlib/ldtoa.c | 41 +++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/newlib/libc/stdlib/ldtoa.c b/newlib/libc/stdlib/ldtoa.c
index 1b32e801c..95347e017 100644
--- a/newlib/libc/stdlib/ldtoa.c
+++ b/newlib/libc/stdlib/ldtoa.c
@@ -83,7 +83,7 @@ static void eclear (register short unsigned int *x);
 static void einfin (register short unsigned int *x, register LDPARMS * ldp);
 static void efloor (short unsigned int *x, short unsigned int *y,
 		    LDPARMS * ldp);
-static void etoasc (short unsigned int *x, char *string, int ndigs,
+static void etoasc (short unsigned int *x, char *string, int ndec, int ndigs,
 		    int outformat, LDPARMS * ldp);
 
 union uconv
@@ -217,7 +217,7 @@ static const char *const ermsg[7] = {
  *	e24toasc( &f, str, n )	single to ASCII string, n digits after decimal
  *	e53toasc( &d, str, n )	double to ASCII string, n digits after decimal
  *	e64toasc( &d, str, n )	long double to ASCII string
- *	etoasc(e,str,n,fmt,ldp)e to ASCII string, n digits after decimal
+ *	etoasc(e,str,ndec,n,fmt,ldp)e to ASCII string, n digits after decimal
  *	etoe24( e, &f )		convert e type to IEEE single precision
  *	etoe53( e, &d )		convert e type to IEEE double precision
  *	etoe64( e, &d )		convert e type to IEEE long double precision
@@ -2839,23 +2839,33 @@ _ldtoa_r (struct _reent *ptr, long double d, int mode, int ndigits,
 
 /* This sanity limit must agree with the corresponding one in etoasc, to
    keep straight the returned value of outexpon.  Note that we use a dynamic
-   limit now, either NDEC or NDEC_SML, depending on ndigits.  See the usage
-   of "my_NDEC" in etoasc. */
-  if (ndigits > NDEC)
-    ndigits = NDEC;
+   limit now, either ndec (<= NDEC) or NDEC_SML, depending on ndigits. */
+  int ndec;
+  if (mode == 3) /* %f */
+    {
+      int expon = (e[NE - 1] & 0x7fff) - (EXONE - 1); /* exponent part */
+      /* log2(10) approximately 485/146 */
+      ndec = expon * 146 / 485 + ndigits;
+    }
+  else /* %g/%e */
+    ndec = ndigits;
+  if (ndec < 0)
+    ndec = 0;
+  if (ndec > NDEC)
+    ndec = NDEC;
 
   /* Allocate buffer if more than NDEC_SML digits are requested. */
-  if (ndigits > NDEC_SML)
+  if (ndec > NDEC_SML)
     {
-      outbuf = (char *) _malloc_r (ptr, NDEC + MAX_EXP_DIGITS + 10);
+      outbuf = (char *) _malloc_r (ptr, ndec + MAX_EXP_DIGITS + 10);
       if (!outbuf)
 	{
-	  ndigits = NDEC_SML;
+	  ndec = NDEC_SML;
 	  outbuf = outbuf_sml;
 	}
     }
 
-  etoasc (e, outbuf, ndigits, mode, ldp);
+  etoasc (e, outbuf, ndec, ndigits, mode, ldp);
   s = outbuf;
   if (eisinf (e) || eisnan (e))
     {
@@ -2992,8 +3002,8 @@ _ldcheck (long double *d)
 }				/* _ldcheck */
 
 static void
-etoasc (short unsigned int *x, char *string, int ndigits, int outformat,
-	LDPARMS * ldp)
+etoasc (short unsigned int *x, char *string, int ndec, int ndigits,
+	int outformat, LDPARMS * ldp)
 {
   long digit;
   unsigned short y[NI], t[NI], u[NI], w[NI];
@@ -3003,7 +3013,6 @@ etoasc (short unsigned int *x, char *string, int ndigits, int outformat,
   char *s, *ss;
   unsigned short m;
   unsigned short *equot = ldp->equot;
-  int my_NDEC = (ndigits > NDEC_SML) ? NDEC : NDEC_SML;
 
   ndigs = ndigits;
   rndsav = ldp->rndprc;
@@ -3129,7 +3138,7 @@ tnzro:
       else
 	{
 	  emovi (y, w);
-	  for (i = 0; i < my_NDEC + 1; i++)
+	  for (i = 0; i < ndec + 1; i++)
 	    {
 	      if ((w[NI - 1] & 0x7) != 0)
 		break;
@@ -3205,8 +3214,8 @@ isone:
 else if( ndigs < 0 )
         ndigs = 0;
 */
-  if (ndigs > my_NDEC)
-    ndigs = my_NDEC;
+  if (ndigs > ndec)
+    ndigs = ndec;
   if (digit == 10)
     {
       *s++ = '1';
-- 
2.33.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ldtoa: Fix insufficient valid output digits for "%f" format.
  2021-11-25 12:02 [PATCH] ldtoa: Fix insufficient valid output digits for "%f" format Takashi Yano
@ 2021-11-25 14:09 ` Corinna Vinschen
  2021-11-25 21:51   ` Takashi Yano
  0 siblings, 1 reply; 3+ messages in thread
From: Corinna Vinschen @ 2021-11-25 14:09 UTC (permalink / raw)
  To: newlib

Hi Takashi,

looks good, but I see a potential problem at one point:

On Nov 25 21:02, Takashi Yano wrote:
> +   limit now, either ndec (<= NDEC) or NDEC_SML, depending on ndigits. */
> +  int ndec;
> +  if (mode == 3) /* %f */
> +    {
> +      int expon = (e[NE - 1] & 0x7fff) - (EXONE - 1); /* exponent part */
> +      /* log2(10) approximately 485/146 */
> +      ndec = expon * 146 / 485 + ndigits;

We have targets with sizeof(int) == 2.  If expon is any number up to
0x7fff, multiplying it with 146 will overflow.  Using __int32_t for
expon should be safer.


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ldtoa: Fix insufficient valid output digits for "%f" format.
  2021-11-25 14:09 ` Corinna Vinschen
@ 2021-11-25 21:51   ` Takashi Yano
  0 siblings, 0 replies; 3+ messages in thread
From: Takashi Yano @ 2021-11-25 21:51 UTC (permalink / raw)
  To: newlib

On Thu, 25 Nov 2021 15:09:33 +0100
Corinna Vinschen wrote:
> looks good, but I see a potential problem at one point:
> 
> On Nov 25 21:02, Takashi Yano wrote:
> > +   limit now, either ndec (<= NDEC) or NDEC_SML, depending on ndigits. */
> > +  int ndec;
> > +  if (mode == 3) /* %f */
> > +    {
> > +      int expon = (e[NE - 1] & 0x7fff) - (EXONE - 1); /* exponent part */
> > +      /* log2(10) approximately 485/146 */
> > +      ndec = expon * 146 / 485 + ndigits;
> 
> We have targets with sizeof(int) == 2.  If expon is any number up to
> 0x7fff, multiplying it with 146 will overflow.  Using __int32_t for
> expon should be safer.

Thanks for checking. I have just submitted v2 patch fixed.


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-25 21:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 12:02 [PATCH] ldtoa: Fix insufficient valid output digits for "%f" format Takashi Yano
2021-11-25 14:09 ` Corinna Vinschen
2021-11-25 21:51   ` Takashi Yano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).