[gcc r12-6115] Fortran: simplify library code for integer-to-decimal conversion

[gcc r12-6115] Fortran: simplify library code for integer-to-decimal conversion

[Franथईois-Xavier Coudert]

https://gcc.gnu.org/g:4ae906e46c4c173b79c9c60c6157ff68e05f7464

commit r12-6115-g4ae906e46c4c173b79c9c60c6157ff68e05f7464
Author: Francois-Xavier Coudert <fxcoudert@gmail.com>
Date:   Sat Dec 25 15:07:12 2021 +0100

    Fortran: simplify library code for integer-to-decimal conversion
    
    libgfortran/ChangeLog:
    
            PR libfortran/81986
            PR libfortran/99191
    
            * libgfortran.h: Remove gfc_xtoa(), adjust gfc_itoa() and
            GFC_ITOA_BUF_SIZE.
            * io/write.c (write_decimal): conversion parameter is always
            gfc_itoa(), so remove it. Protect from overflow.
            (xtoa): Move gfc_xtoa and update its name.
            (xtoa_big): Renamed from ztoa_big for consistency.
            (write_z): Adjust to new function names.
            (write_i, write_integer): Remove last arg of write_decimal.
            * runtime/backtrace.c (error_callback): Comment on the use of
            gfc_itoa().
            * runtime/error.c (gfc_xtoa): Move to io/write.c.
            * runtime/string.c (gfc_itoa): Take an unsigned argument,
            remove the handling of negative values.

Diff:
---
 libgfortran/io/write.c          | 70 ++++++++++++++++++++++++++++-------------
 libgfortran/libgfortran.h       |  7 ++---
 libgfortran/runtime/backtrace.c |  1 +
 libgfortran/runtime/error.c     | 31 ------------------
 libgfortran/runtime/string.c    | 39 +++++++++--------------
 5 files changed, 66 insertions(+), 82 deletions(-)

diff --git a/libgfortran/io/write.c b/libgfortran/io/write.c
index b9e92845bcf..41c9162ea4c 100644
--- a/libgfortran/io/write.c
+++ b/libgfortran/io/write.c
@@ -795,10 +795,10 @@ write_boz (st_parameter_dt *dtp, const fnode *f, const char *q, int n, int len)
 
 static void
 write_decimal (st_parameter_dt *dtp, const fnode *f, const char *source,
-	       int len,
-               const char *(*conv) (GFC_INTEGER_LARGEST, char *, size_t))
+	       int len)
 {
   GFC_INTEGER_LARGEST n = 0;
+  GFC_UINTEGER_LARGEST absn;
   int w, m, digits, nsign, nzero, nblank;
   char *p;
   const char *q;
@@ -831,18 +831,14 @@ write_decimal (st_parameter_dt *dtp, const fnode *f, const char *source,
 
   sign = calculate_sign (dtp, n < 0);
   if (n < 0)
-    n = -n;
+    /* Use unsigned to protect from overflow. */
+    absn = -(GFC_UINTEGER_LARGEST) n;
+  else
+    absn = n;
   nsign = sign == S_NONE ? 0 : 1;
 
-  /* conv calls itoa which sets the negative sign needed
-     by write_integer. The sign '+' or '-' is set below based on sign
-     calculated above, so we just point past the sign in the string
-     before proceeding to avoid double signs in corner cases.
-     (see PR38504)  */
-  q = conv (n, itoa_buf, sizeof (itoa_buf));
-  if (*q == '-')
-    q++;
-
+  /* gfc_itoa() converts the nonnegative value to decimal representation.  */
+  q = gfc_itoa (absn, itoa_buf, sizeof (itoa_buf));
   digits = strlen (q);
 
   /* Select a width if none was specified.  The idea here is to always
@@ -945,7 +941,37 @@ write_decimal (st_parameter_dt *dtp, const fnode *f, const char *source,
 }
 
 
-/* Convert unsigned octal to ascii.  */
+/* Convert hexadecimal to ASCII.  */
+
+static const char *
+xtoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
+{
+  int digit;
+  char *p;
+
+  assert (len >= GFC_XTOA_BUF_SIZE);
+
+  if (n == 0)
+    return "0";
+
+  p = buffer + GFC_XTOA_BUF_SIZE - 1;
+  *p = '\0';
+
+  while (n != 0)
+    {
+      digit = n & 0xF;
+      if (digit > 9)
+	digit += 'A' - '0' - 10;
+
+      *--p = '0' + digit;
+      n >>= 4;
+    }
+
+  return p;
+}
+
+
+/* Convert unsigned octal to ASCII.  */
 
 static const char *
 otoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
@@ -970,7 +996,7 @@ otoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
 }
 
 
-/* Convert unsigned binary to ascii.  */
+/* Convert unsigned binary to ASCII.  */
 
 static const char *
 btoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
@@ -994,7 +1020,7 @@ btoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
   return p;
 }
 
-/* The following three functions, btoa_big, otoa_big, and ztoa_big, are needed
+/* The following three functions, btoa_big, otoa_big, and xtoa_big, are needed
    to convert large reals with kind sizes that exceed the largest integer type
    available on certain platforms.  In these cases, byte by byte conversion is
    performed. Endianess is taken into account.  */
@@ -1132,10 +1158,10 @@ otoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
   return q;
 }
 
-/* Conversion to hexidecimal.  */
+/* Conversion to hexadecimal.  */
 
 static const char *
-ztoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
+xtoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
 {
   static char a[16] = {'0', '1', '2', '3', '4', '5', '6', '7',
     '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
@@ -1177,7 +1203,7 @@ ztoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
 	}
     }
 
-  /* write_z, which calls ztoa_big, is called from transfer.c,
+  /* write_z, which calls xtoa_big, is called from transfer.c,
      formatted_transfer_scalar_write.  There it is passed the kind as
      argument, which means a maximum of 16.  The buffer is large
      enough, but the compiler does not know that, so shut up the
@@ -1201,7 +1227,7 @@ ztoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
 void
 write_i (st_parameter_dt *dtp, const fnode *f, const char *p, int len)
 {
-  write_decimal (dtp, f, p, len, (void *) gfc_itoa);
+  write_decimal (dtp, f, p, len);
 }
 
 
@@ -1258,13 +1284,13 @@ write_z (st_parameter_dt *dtp, const fnode *f, const char *source, int len)
 
   if (len > (int) sizeof (GFC_UINTEGER_LARGEST))
     {
-      p = ztoa_big (source, itoa_buf, len, &n);
+      p = xtoa_big (source, itoa_buf, len, &n);
       write_boz (dtp, f, p, n, len);
     }
   else
     {
       n = extract_uint (source, len);
-      p = gfc_xtoa (n, itoa_buf, sizeof (itoa_buf));
+      p = xtoa (n, itoa_buf, sizeof (itoa_buf));
       write_boz (dtp, f, p, n, len);
     }
 }
@@ -1365,7 +1391,7 @@ write_integer (st_parameter_dt *dtp, const char *source, int kind)
   f.u.integer.w = width;
   f.u.integer.m = -1;
   f.format = FMT_NONE;
-  write_decimal (dtp, &f, source, kind, (void *) gfc_itoa);
+  write_decimal (dtp, &f, source, kind);
 }
 
 
diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h
index 93e3591b21f..6d60a9ca314 100644
--- a/libgfortran/libgfortran.h
+++ b/libgfortran/libgfortran.h
@@ -710,7 +710,7 @@ internal_proto(show_backtrace);
 #define GFC_LARGEST_BUF (sizeof (GFC_INTEGER_LARGEST))
 #endif
 
-#define GFC_ITOA_BUF_SIZE (sizeof (GFC_INTEGER_LARGEST) * 3 + 2)
+#define GFC_ITOA_BUF_SIZE (sizeof (GFC_INTEGER_LARGEST) * 3 + 1)
 #define GFC_XTOA_BUF_SIZE (GFC_LARGEST_BUF * 2 + 1)
 #define GFC_OTOA_BUF_SIZE (GFC_LARGEST_BUF * 3 + 1)
 #define GFC_BTOA_BUF_SIZE (GFC_LARGEST_BUF * 8 + 1)
@@ -738,9 +738,6 @@ extern int st_printf (const char *, ...)
   __attribute__((format (gfc_printf, 1, 2)));
 internal_proto(st_printf);
 
-extern const char *gfc_xtoa (GFC_UINTEGER_LARGEST, char *, size_t);
-internal_proto(gfc_xtoa);
-
 extern _Noreturn void os_error (const char *);
 iexport_proto(os_error);
 
@@ -896,7 +893,7 @@ internal_proto(fc_strdup);
 extern char *fc_strdup_notrim(const char *, gfc_charlen_type);
 internal_proto(fc_strdup_notrim);
 
-extern const char *gfc_itoa(GFC_INTEGER_LARGEST, char *, size_t);
+extern const char *gfc_itoa(GFC_UINTEGER_LARGEST, char *, size_t);
 internal_proto(gfc_itoa);
 
 /* io/intrinsics.c */
diff --git a/libgfortran/runtime/backtrace.c b/libgfortran/runtime/backtrace.c
index 5ac08316f71..403c7c37f18 100644
--- a/libgfortran/runtime/backtrace.c
+++ b/libgfortran/runtime/backtrace.c
@@ -97,6 +97,7 @@ error_callback (void *data, const char *msg, int errnum)
 	  iov[1].iov_len = strlen (msg);
 	  iov[2].iov_base = (char*) ", errno: ";
 	  iov[2].iov_len = strlen (iov[2].iov_base);
+	  /* Async-signal-safe function, errnum must be positive.  */
 	  const char *p = gfc_itoa (errnum, errbuf, sizeof (errbuf));
 	  iov[3].iov_base = (char*) p;
 	  iov[3].iov_len = strlen (p);
diff --git a/libgfortran/runtime/error.c b/libgfortran/runtime/error.c
index b9c75742690..ba6ff866a01 100644
--- a/libgfortran/runtime/error.c
+++ b/libgfortran/runtime/error.c
@@ -219,37 +219,6 @@ exit_error (int status)
 }
 
 
-
-/* gfc_xtoa()-- Integer to hexadecimal conversion.  */
-
-const char *
-gfc_xtoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
-{
-  int digit;
-  char *p;
-
-  assert (len >= GFC_XTOA_BUF_SIZE);
-
-  if (n == 0)
-    return "0";
-
-  p = buffer + GFC_XTOA_BUF_SIZE - 1;
-  *p = '\0';
-
-  while (n != 0)
-    {
-      digit = n & 0xF;
-      if (digit > 9)
-	digit += 'A' - '0' - 10;
-
-      *--p = '0' + digit;
-      n >>= 4;
-    }
-
-  return p;
-}
-
-
 /* Hopefully thread-safe wrapper for a strerror() style function.  */
 
 char *
diff --git a/libgfortran/runtime/string.c b/libgfortran/runtime/string.c
index 536a9cd3f2b..835027a7cd6 100644
--- a/libgfortran/runtime/string.c
+++ b/libgfortran/runtime/string.c
@@ -169,21 +169,22 @@ find_option (st_parameter_common *cmp, const char *s1, gfc_charlen_type s1_len,
 }
 
 
-/* gfc_itoa()-- Integer to decimal conversion.
-   The itoa function is a widespread non-standard extension to
-   standard C, often declared in <stdlib.h>.  Even though the itoa
-   defined here is a static function we take care not to conflict with
-   any prior non-static declaration.  Hence the 'gfc_' prefix, which
-   is normally reserved for functions with external linkage.  Notably,
-   in contrast to the *printf() family of functions, this ought to be
-   async-signal-safe.  */
+/* Integer to decimal conversion.
+
+   This function is much more restricted than the widespread (but
+   non-standard) itoa() function.  This version has the following
+   characteristics:
+
+     - it takes only non-negative arguments
+     - it is async-signal-safe (we use it runtime/backtrace.c)
+     - it works in base 10 (see xtoa, otoa, btoa functions
+       in io/write.c for other radices)
+ */
 
 const char *
-gfc_itoa (GFC_INTEGER_LARGEST n, char *buffer, size_t len)
+gfc_itoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
 {
-  int negative;
   char *p;
-  GFC_UINTEGER_LARGEST t;
 
   if (len < GFC_ITOA_BUF_SIZE)
     sys_abort ();
@@ -191,24 +192,14 @@ gfc_itoa (GFC_INTEGER_LARGEST n, char *buffer, size_t len)
   if (n == 0)
     return "0";
 
-  negative = 0;
-  t = n;
-  if (n < 0)
-    {
-      negative = 1;
-      t = -(GFC_UINTEGER_LARGEST) n;  /* Must use unsigned to protect from overflow. */
-    }
-
   p = buffer + GFC_ITOA_BUF_SIZE - 1;
   *p = '\0';
 
-  while (t != 0)
+  while (n != 0)
     {
-      *--p = '0' + (t % 10);
-      t /= 10;
+      *--p = '0' + (n % 10);
+      n /= 10;
     }
 
-  if (negative)
-    *--p = '-';
   return p;
 }