[Emacs-diffs] master b3e1b38: Improve integer overflow handling a bit

[Paul Eggert]

branch: master
commit b3e1b382456b0f7d108c57d6f902bbddfdd97b2a
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Improve integer overflow handling a bit
    
    * src/charset.c (read_hex): Use INT_LEFT_SHIFT_OVERFLOW for clarity.
    The machine code is the same on my platform.
    * src/doprnt.c (doprnt):
    * src/emacs-module.c (module_funcall):
    * src/font.c (font_intern_prop):
    * src/keyboard.c (Frecursion_depth):
    * src/lread.c (read1):
    Use WRAPV macros instead of checking overflow by hand.
    * src/editfns.c (hi_time, time_arith, decode_time_components):
    * src/emacs-module.c (Fmodule_load):
    Simplify by using FIXNUM_OVERFLOW_P.
    * src/emacs-module.c: Include intprops.h.
    * src/xdisp.c (percent99): New function.
    (decode_mode_spec): Use it to simplify overflow avoidance and
    formatting of %p and %P.
---
 src/charset.c      |    2 +-
 src/doprnt.c       |   25 ++++++++++----------
 src/editfns.c      |   15 +++---------
 src/emacs-module.c |   13 +++++++----
 src/font.c         |    7 +++---
 src/keyboard.c     |    8 +++----
 src/lread.c        |   10 ++++----
 src/xdisp.c        |   64 ++++++++++++++++++++--------------------------------
 8 files changed, 58 insertions(+), 86 deletions(-)

diff --git a/src/charset.c b/src/charset.c
index 0c831f1..cdbfe11 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -435,7 +435,7 @@ read_hex (FILE *fp, bool *eof, bool *overflow)
   n = 0;
   while (c_isxdigit (c = getc (fp)))
     {
-      if (UINT_MAX >> 4 < n)
+      if (INT_LEFT_SHIFT_OVERFLOW (n, 4))
        *overflow = 1;
       n = ((n << 4)
           | (c - ('0' <= c && c <= '9' ? '0'
diff --git a/src/doprnt.c b/src/doprnt.c
index 9d8b783..de2b89e 100644
--- a/src/doprnt.c
+++ b/src/doprnt.c
@@ -133,8 +133,11 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char 
*format,
   const char *fmt = format;    /* Pointer into format string.  */
   char *bufptr = buffer;       /* Pointer into output buffer.  */
 
+  /* Enough to handle floating point formats with large numbers.  */
+  enum { SIZE_BOUND_EXTRA = DBL_MAX_10_EXP + 50 };
+
   /* Use this for sprintf unless we need something really big.  */
-  char tembuf[DBL_MAX_10_EXP + 100];
+  char tembuf[SIZE_BOUND_EXTRA + 50];
 
   /* Size of sprintf_buffer.  */
   ptrdiff_t size_allocated = sizeof (tembuf);
@@ -196,21 +199,19 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char 
*format,
                     This might be a field width or a precision; e.g.
                     %1.1000f and %1000.1f both might need 1000+ bytes.
                     Parse the width or precision, checking for overflow.  */
-                 ptrdiff_t n = *fmt - '0';
+                 int n = *fmt - '0';
+                 bool overflow = false;
                  while (fmt + 1 < format_end
                         && '0' <= fmt[1] && fmt[1] <= '9')
                    {
-                     /* Avoid ptrdiff_t, size_t, and int overflow, as
-                        many sprintfs mishandle widths greater than INT_MAX.
-                        This test is simple but slightly conservative: e.g.,
-                        (INT_MAX - INT_MAX % 10) is reported as an overflow
-                        even when it's not.  */
-                     if (n >= min (INT_MAX, min (PTRDIFF_MAX, SIZE_MAX)) / 10)
-                       error ("Format width or precision too large");
-                     n = n * 10 + fmt[1] - '0';
+                     overflow |= INT_MULTIPLY_WRAPV (n, 10, &n);
+                     overflow |= INT_ADD_WRAPV (n, fmt[1] - '0', &n);
                      *string++ = *++fmt;
                    }
 
+                 if (overflow
+                     || min (PTRDIFF_MAX, SIZE_MAX) - SIZE_BOUND_EXTRA < n)
+                   error ("Format width or precision too large");
                  if (size_bound < n)
                    size_bound = n;
                }
@@ -244,9 +245,7 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char *format,
 
          /* Make the size bound large enough to handle floating point formats
             with large numbers.  */
-         if (size_bound > min (PTRDIFF_MAX, SIZE_MAX) - DBL_MAX_10_EXP - 50)
-           error ("Format width or precision too large");
-         size_bound += DBL_MAX_10_EXP + 50;
+         size_bound += SIZE_BOUND_EXTRA;
 
          /* Make sure we have that much.  */
          if (size_bound > size_allocated)
diff --git a/src/editfns.c b/src/editfns.c
index 835e432..c5b177e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -1523,17 +1523,8 @@ static EMACS_INT
 hi_time (time_t t)
 {
   time_t hi = t >> LO_TIME_BITS;
-
-  /* Check for overflow, helping the compiler for common cases where
-     no runtime check is needed, and taking care not to convert
-     negative numbers to unsigned before comparing them.  */
-  if (! ((! TYPE_SIGNED (time_t)
-         || MOST_NEGATIVE_FIXNUM <= TIME_T_MIN >> LO_TIME_BITS
-         || MOST_NEGATIVE_FIXNUM <= hi)
-        && (TIME_T_MAX >> LO_TIME_BITS <= MOST_POSITIVE_FIXNUM
-            || hi <= MOST_POSITIVE_FIXNUM)))
+  if (FIXNUM_OVERFLOW_P (hi))
     time_overflow ();
-
   return hi;
 }
 
@@ -1595,7 +1586,7 @@ time_arith (Lisp_Object a, Lisp_Object b,
   struct lisp_time ta = lisp_time_struct (a, &alen);
   struct lisp_time tb = lisp_time_struct (b, &blen);
   struct lisp_time t = op (ta, tb);
-  if (! (MOST_NEGATIVE_FIXNUM <= t.hi && t.hi <= MOST_POSITIVE_FIXNUM))
+  if (FIXNUM_OVERFLOW_P (t.hi))
     time_overflow ();
   Lisp_Object val = Qnil;
 
@@ -1853,7 +1844,7 @@ decode_time_components (Lisp_Object high, Lisp_Object 
low, Lisp_Object usec,
 
   if (result)
     {
-      if (! (MOST_NEGATIVE_FIXNUM <= hi && hi <= MOST_POSITIVE_FIXNUM))
+      if (FIXNUM_OVERFLOW_P (hi))
        return -1;
       result->hi = hi;
       result->lo = lo;
diff --git a/src/emacs-module.c b/src/emacs-module.c
index 724d24a..0e755ef 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -30,7 +30,9 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 #include "lisp.h"
 #include "dynlib.h"
 #include "coding.h"
-#include "verify.h"
+
+#include <intprops.h>
+#include <verify.h>
 
 
 /* Feature tests.  */
@@ -424,13 +426,14 @@ module_funcall (emacs_env *env, emacs_value fun, 
ptrdiff_t nargs,
      first arg, because that's what Ffuncall takes.  */
   Lisp_Object *newargs;
   USE_SAFE_ALLOCA;
-  if (nargs == PTRDIFF_MAX)
+  ptrdiff_t nargs1;
+  if (INT_ADD_WRAPV (nargs, 1, &nargs1))
     xsignal0 (Qoverflow_error);
-  SAFE_ALLOCA_LISP (newargs, nargs + 1);
+  SAFE_ALLOCA_LISP (newargs, nargs1);
   newargs[0] = value_to_lisp (fun);
   for (ptrdiff_t i = 0; i < nargs; i++)
     newargs[1 + i] = value_to_lisp (args[i]);
-  emacs_value result = lisp_to_value (Ffuncall (nargs + 1, newargs));
+  emacs_value result = lisp_to_value (Ffuncall (nargs1, newargs));
   SAFE_FREE ();
   return result;
 }
@@ -665,7 +668,7 @@ DEFUN ("module-load", Fmodule_load, Smodule_load, 1, 1, 0,
 
   if (r != 0)
     {
-      if (! (MOST_NEGATIVE_FIXNUM <= r && r <= MOST_POSITIVE_FIXNUM))
+      if (FIXNUM_OVERFLOW_P (r))
         xsignal0 (Qoverflow_error);
       xsignal2 (Qmodule_load_failed, file, make_number (r));
     }
diff --git a/src/font.c b/src/font.c
index 144ba07..f280063 100644
--- a/src/font.c
+++ b/src/font.c
@@ -264,14 +264,13 @@ font_intern_prop (const char *str, ptrdiff_t len, bool 
force_symbol)
          break;
       if (i == len)
        {
-         EMACS_INT n;
-
          i = 0;
-         for (n = 0; (n += str[i++] - '0') <= MOST_POSITIVE_FIXNUM; n *= 10)
+         for (EMACS_INT n = 0;
+              (n += str[i++] - '0') <= MOST_POSITIVE_FIXNUM; )
            {
              if (i == len)
                return make_number (n);
-             if (MOST_POSITIVE_FIXNUM / 10 < n)
+             if (INT_MULTIPLY_WRAPV (n, 10, &n))
                break;
            }
 
diff --git a/src/keyboard.c b/src/keyboard.c
index b8bc361..ca40c6e 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -10058,11 +10058,9 @@ DEFUN ("recursion-depth", Frecursion_depth, 
Srecursion_depth, 0, 0, 0,
        doc: /* Return the current depth in recursive edits.  */)
   (void)
 {
-  Lisp_Object temp;
-  /* Wrap around reliably on integer overflow.  */
-  EMACS_INT sum = (command_loop_level & INTMASK) + (minibuf_level & INTMASK);
-  XSETINT (temp, sum);
-  return temp;
+  EMACS_INT sum;
+  INT_ADD_WRAPV (command_loop_level, minibuf_level, &sum);
+  return make_number (sum);
 }
 
 DEFUN ("open-dribble-file", Fopen_dribble_file, Sopen_dribble_file, 1, 1,
diff --git a/src/lread.c b/src/lread.c
index dc7c00b..d3413d1 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2894,19 +2894,17 @@ read1 (Lisp_Object readcharfun, int *pch, bool 
first_in_list)
        {
          EMACS_INT n = 0;
          Lisp_Object tem;
+         bool overflow = false;
 
          /* Read a non-negative integer.  */
          while (c >= '0' && c <= '9')
            {
-             if (MOST_POSITIVE_FIXNUM / 10 < n
-                 || MOST_POSITIVE_FIXNUM < n * 10 + c - '0')
-               n = MOST_POSITIVE_FIXNUM + 1;
-             else
-               n = n * 10 + c - '0';
+             overflow |= INT_MULTIPLY_WRAPV (n, 10, &n);
+             overflow |= INT_ADD_WRAPV (n, c - '0', &n);
              c = READCHAR;
            }
 
-         if (n <= MOST_POSITIVE_FIXNUM)
+         if (!overflow && n <= MOST_POSITIVE_FIXNUM)
            {
              if (c == 'r' || c == 'R')
                return read_integer (readcharfun, n);
diff --git a/src/xdisp.c b/src/xdisp.c
index 4bf1470..13af87f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -23448,6 +23448,16 @@ decode_mode_spec_coding (Lisp_Object coding_system, 
char *buf, bool eol_flag)
   return buf;
 }
 
+/* Return the approximate percentage N is of D (rounding upward), or 99,
+   whichever is less.  Assume 0 < D and 0 <= N <= D * INT_MAX / 100.  */
+
+static int
+percent99 (ptrdiff_t n, ptrdiff_t d)
+{
+  int percent = (d - 1 + 100.0 * n) / d;
+  return min (percent, 99);
+}
+
 /* Return a string for the output of a mode line %-spec for window W,
    generated by character C.  FIELD_WIDTH > 0 means pad the string
    returned with spaces to that value.  Return a Lisp string in
@@ -23735,29 +23745,17 @@ decode_mode_spec (struct window *w, register int c, 
int field_width,
     case 'p':
       {
        ptrdiff_t pos = marker_position (w->start);
-       ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b);
+       ptrdiff_t begv = BUF_BEGV (b);
+       ptrdiff_t zv = BUF_ZV (b);
 
-       if (w->window_end_pos <= BUF_Z (b) - BUF_ZV (b))
-         {
-           if (pos <= BUF_BEGV (b))
-             return "All";
-           else
-             return "Bottom";
-         }
-       else if (pos <= BUF_BEGV (b))
+       if (w->window_end_pos <= BUF_Z (b) - zv)
+         return pos <= begv ? "All" : "Bottom";
+       else if (pos <= begv)
          return "Top";
        else
          {
-           if (total > 1000000)
-             /* Do it differently for a large value, to avoid overflow.  */
-             total = ((pos - BUF_BEGV (b)) + (total / 100) - 1) / (total / 
100);
-           else
-             total = ((pos - BUF_BEGV (b)) * 100 + total - 1) / total;
-           /* We can't normally display a 3-digit number,
-              so get us a 2-digit number that is close.  */
-           if (total == 100)
-             total = 99;
-           sprintf (decode_mode_spec_buf, "%2"pD"d%%", total);
+           sprintf (decode_mode_spec_buf, "%2d%%",
+                    percent99 (pos - begv, zv - begv));
            return decode_mode_spec_buf;
          }
       }
@@ -23767,30 +23765,16 @@ decode_mode_spec (struct window *w, register int c, 
int field_width,
       {
        ptrdiff_t toppos = marker_position (w->start);
        ptrdiff_t botpos = BUF_Z (b) - w->window_end_pos;
-       ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b);
+       ptrdiff_t begv = BUF_BEGV (b);
+       ptrdiff_t zv = BUF_ZV (b);
 
-       if (botpos >= BUF_ZV (b))
-         {
-           if (toppos <= BUF_BEGV (b))
-             return "All";
-           else
-             return "Bottom";
-         }
+       if (zv <= botpos)
+         return toppos <= begv ? "All" : "Bottom";
        else
          {
-           if (total > 1000000)
-             /* Do it differently for a large value, to avoid overflow.  */
-             total = ((botpos - BUF_BEGV (b)) + (total / 100) - 1) / (total / 
100);
-           else
-             total = ((botpos - BUF_BEGV (b)) * 100 + total - 1) / total;
-           /* We can't normally display a 3-digit number,
-              so get us a 2-digit number that is close.  */
-           if (total == 100)
-             total = 99;
-           if (toppos <= BUF_BEGV (b))
-             sprintf (decode_mode_spec_buf, "Top%2"pD"d%%", total);
-           else
-             sprintf (decode_mode_spec_buf, "%2"pD"d%%", total);
+           sprintf (decode_mode_spec_buf,
+                    &"Top%2d%%"[begv < toppos ? sizeof "Top" - 1 : 0],
+                    percent99 (botpos - begv, zv - begv));
            return decode_mode_spec_buf;
          }
       }