Subject: Re: bug#8668: * editfns.c (Fformat): Fix several integer overflow problems

From: Paul Eggert <[email-masked]>
Date: Sun, 15 May 2011 23:56:59 -0700

Further inspection has found several other problems with Fformat,
having to do with large strings and integer overflow.  Here's a
revised patch to address the problems that I've found.  This
supersedes the earlier patch.  I'll do more testing on this one
but thought I'd give people a heads-up.  This patch assumes other
fixes I've sent in recently.

* editfns.c: Rework Fformat to avoid integer overflow issues.
Don't include <float.h>; no longer needed.
(MAX_10_EXP, CONVERTED_BYTE_SIZE): Remove; no longer needed.
(format_overflow): New function.
(pWIDE, pWIDElen, signed_wide, unsigned_wide): New defns.
(Fformat): Avoid the prepass trying to compute sizes; it was
only approximate and thus did not catch overflow reliably.
Instead, walk through the format just once, formatting and
computing sizes as we go, checking for integer overflow at
every step, and allocating a larger buffer as needed.
Keep track separately whether the format is multibyte.
Keep only the most-recently calculated precision, rather than them all.
Record whether each argument has been converted to string.
Use EMACS_INT, not int, for byte and char and arg counts.
Support field widths and precisions larger than INT_MAX, for
strings, since we format them ourselves.  Fix bug with
strchr succeeding on '\0' when looking for flags.  Be more careful
about formatting out-of-range floating point numbers with int
formats.  (Bug#8668)
=== modified file 'src/editfns.c'
--- src/editfns.c	2011-05-15 17:17:44 +0000
+++ src/editfns.c	2011-05-16 06:40:19 +0000
@@ -57,13 +57,6 @@
 #include "window.h"
 #include "blockinput.h"

-#ifdef STDC_HEADERS
-#include <float.h>
-#define MAX_10_EXP	DBL_MAX_10_EXP
-#else
-#define MAX_10_EXP	310
-#endif
-
 #ifndef NULL
 #define NULL 0
 #endif
@@ -3526,13 +3519,30 @@
 }

-/* Number of bytes that STRING will occupy when put into the result.
-   MULTIBYTE is nonzero if the result should be multibyte.  */
+/* Report the failure of a format conversion of a single number, due to
+   size overflow.  Snprintf's API has an INT_MAX-byte limit.  */
+static void format_overflow (void) NO_RETURN;
+static void
+format_overflow (void)
+{
+  error ("Maximum format conversion size exceeded");
+}

-#define CONVERTED_BYTE_SIZE(MULTIBYTE, STRING)				\
-  (((MULTIBYTE) && ! STRING_MULTIBYTE (STRING))				\
-   ? count_size_as_multibyte (SDATA (STRING), SBYTES (STRING))		\
-   : SBYTES (STRING))
+/* A conversion for printing large decimal integers (possibly including
+   trailing "d" that is ignored), along with its length, signed and
+   unsigned types for printing them, and their bounds.  Use widest integers
+   if available, EMACS_INT otherwise.  */
+#ifdef PRIdMAX
+# define pWIDE PRIdMAX
+enum { pWIDElen = sizeof PRIdMAX - 2 }; /* Don't count trailing "d".  */
+typedef intmax_t signed_wide;
+typedef uintmax_t unsigned_wide;
+#else
+# define pWIDE pI
+enum { pWIDElen = sizeof pI - 1 };
+typedef EMACS_INT signed_wide;
+typedef EMACS_UINT unsigned_wide;
+#endif

 DEFUN ("format", Fformat, Sformat, 1, MANY, 0,
        doc: /* Format a string out of a format-string and arguments.
@@ -3583,11 +3593,14 @@
 usage: (format STRING &rest OBJECTS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  register size_t n;		/* The number of the next arg to substitute */
-  register size_t total;	/* An estimate of the final length */
+  register EMACS_INT n;		/* The number of the next arg to substitute */
   char *buf, *p;
+  EMACS_INT bufsize;
+  EMACS_INT max_bufsize = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX);
   register char *format, *end, *format_start;
-  int nchars;
+  EMACS_INT formatlen, nchars;
+  /* Nonzero if the format is multibyte.  */
+  int multibyte_format = 0;
   /* Nonzero if the output should be a multibyte string,
      which is true if any of the inputs is one.  */
   int multibyte = 0;
@@ -3596,14 +3609,6 @@
      multibyte character of the previous string.  This flag tells if we
      must consider such a situation or not.  */
   int maybe_combine_byte;
-  char *this_format;
-  /* Precision for each spec, or -1, a flag value meaning no precision
-     was given in that spec.  Element 0, corresponding to the format
-     string itself, will not be used.  Element NARGS, corresponding to
-     no argument, *will* be assigned to in the case that a `%' and `.'
-     occur after the final format specifier.  */
-  int *precision = (int *) (alloca ((nargs + 1) * sizeof (int)));
-  int longest_format;
   Lisp_Object val;
   int arg_intervals = 0;
   USE_SAFE_ALLOCA;
@@ -3611,323 +3616,230 @@
   /* discarded[I] is 1 if byte I of the format
      string was not copied into the output.
      It is 2 if byte I was not the first byte of its character.  */
-  char *discarded = 0;
+  char *discarded;

   /* Each element records, for one argument,
      the start and end bytepos in the output string,
+     whether the argument has been converted to string (e.g., due to "%S"),
      and whether the argument is a string with intervals.
      info[0] is unused.  Unused elements have -1 for start.  */
   struct info
   {
-    int start, end, intervals;
+    EMACS_INT start, end;
+    int converted_to_string;
+    int intervals;
   } *info = 0;

   /* It should not be necessary to GCPRO ARGS, because
      the caller in the interpreter should take care of that.  */

+  CHECK_STRING (args[0]);
+
+  /* Allocate a buffer to contain a copy of the result.  The result size is
+     not known yet, so guess it; it will be increased as needed.  */
+  {
+    enum { min_bufsize = 1000, bufsize_multiplier = 4 };
+    formatlen = SBYTES (args[0]);
+    bufsize = formatlen + 1;
+    if (bufsize <= min_bufsize / bufsize_multiplier)
+      bufsize = min_bufsize;
+    else if (bufsize <= max_bufsize / bufsize_multiplier)
+      bufsize *= bufsize_multiplier;
+    else if (bufsize <= max_bufsize)
+      bufsize = max_bufsize;
+    else
+      memory_full ();
+    SAFE_ALLOCA (buf, char *, bufsize);
+  }
+
+  /* Ensure that BUF is big enough to hold N more bytes.  N must be
+     nonnegative.  */
+# define BUF_ROOM(n)							\
+  do									\
+    {									\
+      EMACS_INT additional = (n);					\
+      EMACS_INT used = p - buf;						\
+      if (bufsize - used < additional)					\
+	{								\
+	  char *newbuf;							\
+	  if (INT_ADD_OVERFLOW (used, additional)			\
+	      || max_bufsize < used + additional)			\
+	    string_overflow ();						\
+	  bufsize = used + additional;					\
+	  bufsize = (bufsize < max_bufsize / 3 * 2			\
+		     ? bufsize + bufsize / 2				\
+		     : max_bufsize);					\
+	  SAFE_ALLOCA (newbuf, char *, bufsize);			\
+	  memcpy (newbuf, buf, used);					\
+	  buf = newbuf;							\
+	  p = buf + used;						\
+	}								\
+    }									\
+  while (0);
+
+  /* Allocate the info and discarded tables, and room for a copy of
+     the format.  The copy is needed because we may truncate a format
+     specifier with '\0', or change "%S" to "%s", or insert pWIDE.  */
+  {
+    EMACS_INT i, extralen = 2 * formatlen + pWIDElen + 1;
+    if (SIZE_MAX < extralen
+	|| (SIZE_MAX - extralen) / sizeof (struct info) <= nargs)
+      memory_full ();
+    SAFE_ALLOCA (info, struct info *, (nargs + 1) * sizeof *info + extralen);
+    discarded = (char *) &info[nargs + 1];
+    format_start = discarded + formatlen + pWIDElen;
+    for (i = 0; i < nargs + 1; i++)
+      {
+	info[i].start = -1;
+	info[i].intervals = info[i].converted_to_string = 0;
+      }
+    memset (discarded, 0, formatlen);
+  }
+
   /* Try to determine whether the result should be multibyte.
      This is not always right; sometimes the result needs to be multibyte
      because of an object that we will pass through prin1,
      and in that case, we won't know it here.  */
-  for (n = 0; n < nargs; n++)
-    {
-      if (STRINGP (args[n]) && STRING_MULTIBYTE (args[n]))
-	multibyte = 1;
-      /* Piggyback on this loop to initialize precision[N]. */
-      precision[n] = -1;
-    }
-  precision[nargs] = -1;
-
-  CHECK_STRING (args[0]);
-  /* We may have to change "%S" to "%s". */
-  args[0] = Fcopy_sequence (args[0]);
-
-  /* GC should never happen here, so abort if it does.  */
-  abort_on_gc++;
+  multibyte_format = STRING_MULTIBYTE (args[0]);
+  multibyte = multibyte_format;
+  for (n = 1; !multibyte && n < nargs; n++)
+    if (STRINGP (args[n]) && STRING_MULTIBYTE (args[n]))
+      multibyte = 1;

   /* If we start out planning a unibyte result,
-     then discover it has to be multibyte, we jump back to retry.
-     That can only happen from the first large while loop below.  */
+     then discover it has to be multibyte, we jump back to retry.  */
  retry:

-  format = SSDATA (args[0]);
-  format_start = format;
-  end = format + SBYTES (args[0]);
-  longest_format = 0;
-
-  /* Make room in result for all the non-%-codes in the control string.  */
-  total = 5 + CONVERTED_BYTE_SIZE (multibyte, args[0]) + 1;
-
-  /* Allocate the info and discarded tables.  */
-  {
-    size_t nbytes = (nargs+1) * sizeof *info;
-    size_t i;
-    if (!info)
-      info = (struct info *) alloca (nbytes);
-    memset (info, 0, nbytes);
-    for (i = 0; i < nargs + 1; i++)
-      info[i].start = -1;
-    if (!discarded)
-      SAFE_ALLOCA (discarded, char *, SBYTES (args[0]));
-    memset (discarded, 0, SBYTES (args[0]));
-  }
-
-  /* Add to TOTAL enough space to hold the converted arguments.  */
-
-  n = 0;
-  while (format != end)
-    if (*format++ == '%')
-      {
-	EMACS_INT thissize = 0;
-	EMACS_INT actual_width = 0;
-	char *this_format_start = format - 1;
-	int field_width = 0;
-
-	/* General format specifications look like
-
-	   '%' [flags] [field-width] [precision] format
-
-	   where
-
-	   flags	::= [-+ #0]+
-	   field-width	::= [0-9]+
-	   precision	::= '.' [0-9]*
-
-	   If a field-width is specified, it specifies to which width
-	   the output should be padded with blanks, if the output
-	   string is shorter than field-width.
-
-	   If precision is specified, it specifies the number of
-	   digits to print after the '.' for floats, or the max.
-	   number of chars to print from a string.  */
-
-	while (format != end
-	       && (*format == '-' || *format == '0' || *format == '#'
-		   || * format == ' ' || *format == '+'))
-	  ++format;
-
-	if (*format >= '0' && *format <= '9')
-	  {
-	    for (field_width = 0; *format >= '0' && *format <= '9'; ++format)
-	      field_width = 10 * field_width + *format - '0';
-	  }
-
-	/* N is not incremented for another few lines below, so refer to
-	   element N+1 (which might be precision[NARGS]). */
-	if (*format == '.')
-	  {
-	    ++format;
-	    for (precision[n+1] = 0; *format >= '0' && *format <= '9'; ++format)
-	      precision[n+1] = 10 * precision[n+1] + *format - '0';
-	  }
-
-	/* Extra +1 for 'l' that we may need to insert into the
-	   format.  */
-	if (format - this_format_start + 2 > longest_format)
-	  longest_format = format - this_format_start + 2;
-
-	if (format == end)
-	  error ("Format string ends in middle of format specifier");
-	if (*format == '%')
-	  format++;
-	else if (++n >= nargs)
-	  error ("Not enough arguments for format string");
-	else if (*format == 'S')
-	  {
-	    /* For `S', prin1 the argument and then treat like a string.  */
-	    register Lisp_Object tem;
-	    tem = Fprin1_to_string (args[n], Qnil);
-	    if (STRING_MULTIBYTE (tem) && ! multibyte)
-	      {
-		multibyte = 1;
-		goto retry;
-	      }
-	    args[n] = tem;
-	    /* If we restart the loop, we should not come here again
-	       because args[n] is now a string and calling
-	       Fprin1_to_string on it produces superflous double
-	       quotes.  So, change "%S" to "%s" now.  */
-	    *format = 's';
-	    goto string;
-	  }
-	else if (SYMBOLP (args[n]))
-	  {
-	    args[n] = SYMBOL_NAME (args[n]);
-	    if (STRING_MULTIBYTE (args[n]) && ! multibyte)
-	      {
-		multibyte = 1;
-		goto retry;
-	      }
-	    goto string;
-	  }
-	else if (STRINGP (args[n]))
-	  {
-	  string:
-	    if (*format != 's' && *format != 'S')
-	      error ("Format specifier doesn't match argument type");
-	    /* In the case (PRECISION[N] > 0), THISSIZE may not need
-	       to be as large as is calculated here.  Easy check for
-	       the case PRECISION = 0. */
-	    thissize = precision[n] ? CONVERTED_BYTE_SIZE (multibyte, args[n]) : 0;
-	    /* The precision also constrains how much of the argument
-	       string will finally appear (Bug#5710). */
-	    actual_width = lisp_string_width (args[n], -1, NULL, NULL);
-	    if (precision[n] != -1)
-	      actual_width = min (actual_width, precision[n]);
-	  }
-	/* Would get MPV otherwise, since Lisp_Int's `point' to low memory.  */
-	else if (INTEGERP (args[n]) && *format != 's')
-	  {
-	    /* The following loop assumes the Lisp type indicates
-	       the proper way to pass the argument.
-	       So make sure we have a flonum if the argument should
-	       be a double.  */
-	    if (*format == 'e' || *format == 'f' || *format == 'g')
-	      args[n] = Ffloat (args[n]);
-	    else
-	      if (*format != 'd' && *format != 'o' && *format != 'x'
-		  && *format != 'i' && *format != 'X' && *format != 'c')
-		error ("Invalid format operation %%%c", *format);
-
-	    thissize = 30 + (precision[n] > 0 ? precision[n] : 0);
-	    if (*format == 'c')
-	      {
-		if (! ASCII_CHAR_P (XINT (args[n]))
-		    /* Note: No one can remember why we have to treat
-		       the character 0 as a multibyte character here.
-		       But, until it causes a real problem, let's
-		       don't change it.  */
-		    || XINT (args[n]) == 0)
-		  {
-		    if (! multibyte)
-		      {
-			multibyte = 1;
-			goto retry;
-		      }
-		    args[n] = Fchar_to_string (args[n]);
-		    thissize = SBYTES (args[n]);
-		  }
-	      }
-	  }
-	else if (FLOATP (args[n]) && *format != 's')
-	  {
-	    if (! (*format == 'e' || *format == 'f' || *format == 'g'))
-	      {
-		if (*format != 'd' && *format != 'o' && *format != 'x'
-		    && *format != 'i' && *format != 'X' && *format != 'c')
-		  error ("Invalid format operation %%%c", *format);
-		/* This fails unnecessarily if args[n] is bigger than
-		   most-positive-fixnum but smaller than MAXINT.
-		   These cases are important because we sometimes use floats
-		   to represent such integer values (typically such values
-		   come from UIDs or PIDs).  */
-		/* args[n] = Ftruncate (args[n], Qnil); */
-	      }
-
-	    /* Note that we're using sprintf to print floats,
-	       so we have to take into account what that function
-	       prints.  */
-	    /* Filter out flag value of -1.  */
-	    thissize = (MAX_10_EXP + 100
-			+ (precision[n] > 0 ? precision[n] : 0));
-	  }
-	else
-	  {
-	    /* Anything but a string, convert to a string using princ.  */
-	    register Lisp_Object tem;
-	    tem = Fprin1_to_string (args[n], Qt);
-	    if (STRING_MULTIBYTE (tem) && ! multibyte)
-	      {
-		multibyte = 1;
-		goto retry;
-	      }
-	    args[n] = tem;
-	    goto string;
-	  }
-
-	thissize += max (0, field_width - actual_width);
-	total += thissize + 4;
-      }
-
-  abort_on_gc--;
-
-  /* Now we can no longer jump to retry.
-     TOTAL and LONGEST_FORMAT are known for certain.  */
-
-  this_format = (char *) alloca (longest_format + 1);
-
-  /* Allocate the space for the result.
-     Note that TOTAL is an overestimate.  */
-  SAFE_ALLOCA (buf, char *, total);
-
   p = buf;
   nchars = 0;
   n = 0;

   /* Scan the format and store result in BUF.  */
-  format = SSDATA (args[0]);
-  format_start = format;
-  end = format + SBYTES (args[0]);
+  memcpy (format_start, SSDATA (args[0]), formatlen + 1);
+  format = format_start;
+  end = format + formatlen;
   maybe_combine_byte = 0;
   while (format != end)
     {
       if (*format == '%')
 	{
-	  int minlen;
+	  /* General format specifications look like
+
+	     '%' [flags] [field-width] [precision] format
+
+	     where
+
+	     flags ::= [-+0# ]+
+	     field-width ::= [0-9]+
+	     precision ::= '.' [0-9]*
+
+	     If a field-width is specified, it specifies to which width
+	     the output should be padded with blanks, if the output
+	     string is shorter than field-width.
+
+	     If precision is specified, it specifies the number of
+	     digits to print after the '.' for floats, or the max.
+	     number of chars to print from a string.  */
+
 	  int negative = 0;
-	  char *this_format_start = format;
+	  char *convspec = format; /* The conversion specification.  */
+	  uintmax_t field_width;
+	  uintmax_t precision = UINTMAX_MAX;
+	  char *num_end;
+	  char conversion;

-	  discarded[format - format_start] = 1;
 	  format++;

-	  while (strchr ("-+0# ", *format))
+	  while (*format == '-' || *format == ' ' || *format == '#'
+		 || *format == '+' || *format == '0')
 	    {
 	      if (*format == '-')
 		{
 		  negative = 1;
 		}
-	      discarded[format - format_start] = 1;
 	      ++format;
 	    }

-	  minlen = atoi (format);
-
-	  while ((*format >= '0' && *format <= '9') || *format == '.')
-	    {
-	      discarded[format - format_start] = 1;
-	      format++;
-	    }
-
-	  if (*format++ == '%')
-	    {
-	      *p++ = '%';
-	      nchars++;
-	      continue;
-	    }
+	  field_width = strtoumax (format, &num_end, 10);
+	  if (*num_end == '.')
+	    precision = strtoumax (num_end + 1, &num_end, 10);
+	  format = num_end;
+
+	  if (format == end)
+	    error ("Format string ends in middle of format specifier");
+
+	  memset (&discarded[convspec - format_start], 1, format - convspec);
+	  conversion = *format;
+	  if (conversion == '%')
+	    goto copy_char;
+	  discarded[format - format_start] = 1;
+	  format++;

 	  ++n;
+	  if (! (n < nargs))
+	    error ("Not enough arguments for format string");

-	  discarded[format - format_start - 1] = 1;
 	  info[n].start = nchars;

+	  /* For 'S', prin1 the argument, and then treat like 's'.
+	     For 's', princ any argument that is not a string or
+	     symbol.  But don't do this conversion twice, which might
+	     happen after retrying.  */
+	  if ((conversion == 'S'
+	       || (conversion == 's'
+		   && ! STRINGP (args[n]) && ! SYMBOLP (args[n]))))
+	    {
+	      if (! info[n].converted_to_string)
+		{
+		  Lisp_Object noescape = conversion == 'S' ? Qnil : Qt;
+		  args[n] = Fprin1_to_string (args[n], noescape);
+		  info[n].converted_to_string = 1;
+		  if (STRING_MULTIBYTE (args[n]) && ! multibyte)
+		    {
+		      multibyte = 1;
+		      goto retry;
+		    }
+		}
+	      conversion = 's';
+	      goto string;
+	    }
+
+	  if (SYMBOLP (args[n]))
+	    {
+	      args[n] = SYMBOL_NAME (args[n]);
+	      if (STRING_MULTIBYTE (args[n]) && ! multibyte)
+		{
+		  multibyte = 1;
+		  goto retry;
+		}
+	      goto string;
+	    }
+
 	  if (STRINGP (args[n]))
+	  string:
 	    {
 	      /* handle case (precision[n] >= 0) */

-	      int width, padding;
+	      EMACS_INT width, padding, outbytes;
 	      EMACS_INT nbytes, start;
 	      EMACS_INT nchars_string;

+	      if (conversion != 's')
+		error ("Format specifier doesn't match argument type");
+	      if (max_bufsize <= field_width
+		  || (precision != UINTMAX_MAX && max_bufsize <= precision))
+		string_overflow ();
+
 	      /* lisp_string_width ignores a precision of 0, but GNU
 		 libc functions print 0 characters when the precision
 		 is 0.  Imitate libc behavior here.  Changing
 		 lisp_string_width is the right thing, and will be
 		 done, but meanwhile we work with it. */

-	      if (precision[n] == 0)
+	      if (precision == 0)
 		width = nchars_string = nbytes = 0;
-	      else if (precision[n] > 0)
-		width = lisp_string_width (args[n], precision[n],
+	      else if (precision != UINTMAX_MAX)
+		width = lisp_string_width (args[n], precision,
 					   &nchars_string, &nbytes);
 	      else
 		{		/* no precision spec given for this argument */
@@ -3936,14 +3848,23 @@
 		  nchars_string = SCHARS (args[n]);
 		}

+	      outbytes = nbytes;
+	      if (outbytes && multibyte && ! STRING_MULTIBYTE (args[n]))
+		outbytes = count_size_as_multibyte (SDATA (args[n]), nbytes);
+
+	      padding = width < field_width ? field_width - width : 0;
+
+	      if (INT_ADD_OVERFLOW (outbytes, padding))
+		string_overflow ();
+	      BUF_ROOM (outbytes + padding);
+
 	      /* If spec requires it, pad on right with spaces.  */
-	      padding = minlen - width;
 	      if (! negative)
-		while (padding-- > 0)
-		  {
-		    *p++ = ' ';
-		    ++nchars;
-		  }
+		{
+		  memset (p, ' ', padding);
+		  p += padding;
+		  nchars += padding;
+		}

 	      info[n].start = start = nchars;
 	      nchars += nchars_string;
@@ -3961,108 +3882,216 @@

 	      info[n].end = nchars;

-	      if (negative)
-		while (padding-- > 0)
-		  {
-		    *p++ = ' ';
-		    nchars++;
-		  }
+	      if (negative && 0 < padding)
+		{
+		  memset (p, ' ', padding);
+		  p += padding;
+		  nchars += padding;
+		}

 	      /* If this argument has text properties, record where
 		 in the result string it appears.  */
 	      if (STRING_INTERVALS (args[n]))
 		info[n].intervals = arg_intervals = 1;
 	    }
+	  else if (! (conversion == 'c' || conversion == 'd'
+		      || conversion == 'e' || conversion == 'f'
+		      || conversion == 'g' || conversion == 'i'
+		      || conversion == 'o' || conversion == 'x'
+		      || conversion == 'X'))
+	    error ("Invalid format operation %%%c", conversion);
 	  else if (INTEGERP (args[n]) || FLOATP (args[n]))
 	    {
 	      int this_nchars;

-	      memcpy (this_format, this_format_start,
-		      format - this_format_start);
-	      this_format[format - this_format_start] = 0;
-
-	      if (format[-1] == 'e' || format[-1] == 'f' || format[-1] == 'g')
-		sprintf (p, this_format, XFLOAT_DATA (args[n]));
-	      else
-		{
-		  if (sizeof (EMACS_INT) > sizeof (int)
-		      && format[-1] != 'c')
-		    {
-		      /* Insert 'l' before format spec.  */
-		      this_format[format - this_format_start]
-			= this_format[format - this_format_start - 1];
-		      this_format[format - this_format_start - 1] = 'l';
-		      this_format[format - this_format_start + 1] = 0;
-		    }
-
-		  if (INTEGERP (args[n]))
-		    {
-		      if (format[-1] == 'c')
-			sprintf (p, this_format, (int) XINT (args[n]));
-		      else if (format[-1] == 'd')
-			sprintf (p, this_format, XINT (args[n]));
+	      /* Null-terminate the conversion specification temporarily.  */
+	      char c = *format;
+	      *format = 0;
+
+	      /* Don't allow field width or precision greater than INT_MAX,
+		 as that could make snprintf go kaflooey.  */
+	      if (INT_MAX < field_width
+		  || (INT_MAX < precision && precision < UINTMAX_MAX))
+		format_overflow ();
+
+	      /* Insert pWIDE before an integer conversion specifier.  */
+	      if (pWIDElen
+		  && (conversion == 'd' || conversion == 'i'
+		      || conversion == 'o' || conversion == 'x'
+		      || conversion == 'X'))
+		{
+		  memmove (convspec - pWIDElen, convspec,
+			   format - convspec - 1);
+		  memcpy (format - pWIDElen - 1, pWIDE, pWIDElen);
+		  convspec -= pWIDElen;
+		}
+
+	      /* Append a single value to the output buffer.  */
+
+	      while (1)
+		{
+		  /* Don't pass a buffer size greater than INT_MAX to
+		     snprintf, as this causes snprintf to fail.  */
+		  int prsize = min (INT_MAX, buf + bufsize - p);
+
+		  /* There are four types of conversion: double, unsigned
+		     char (passed as int), wide signed int, and wide
+		     unsigned int.  Treat them separately because the
+		     snprintf ABI is sensitive to which type is passed.  Be
+		     careful about integer overflow, NaNs, infinities, and
+		     conversions; for example, the min and max macros are
+		     not suitable here.  */
+		  if (conversion == 'e' || conversion == 'f'
+		      || conversion == 'g')
+		    {
+		      double x = (INTEGERP (args[n])
+				  ? XINT (args[n])
+				  : XFLOAT_DATA (args[n]));
+		      this_nchars = snprintf (p, prsize, convspec, x);
+		    }
+		  else if (conversion == 'c')
+		    {
+		      unsigned char x;
+		      if (INTEGERP (args[n]))
+			x = XINT (args[n]);
+		      else
+			{
+			  double d = XFLOAT_DATA (args[n]);
+			  if (d < 0)
+			    {
+			      intmax_t i = INTMAX_MIN;
+			      if (i < d)
+				i = d;
+			      x = i;
+			    }
+			  else
+			    {
+			      uintmax_t u = UINTMAX_MAX;
+			      if (d < u)
+				u = d;
+			      x = u;
+			    }
+			}
+		      this_nchars = snprintf (p, prsize, convspec, x);
+		    }
+		  else if (conversion == 'd')
+		    {
+		      /* For float, maybe we should use "%1.0f"
+			 instead so it also works for values outside
+			 the integer range.  */
+		      signed_wide x;
+		      if (INTEGERP (args[n]))
+			x = XINT (args[n]);
+		      else
+			{
+			  double d = XFLOAT_DATA (args[n]);
+			  if (d < 0)
+			    {
+			      x = TYPE_MINIMUM (signed_wide);
+			      if (x < d)
+				x = d;
+			    }
+			  else
+			    {
+			      x = TYPE_MAXIMUM (signed_wide);
+			      if (d < x)
+				x = d;
+			    }
+			}
+		      this_nchars = snprintf (p, prsize, convspec, x);
+		    }
+		  else
+		    {
 		      /* Don't sign-extend for octal or hex printing.  */
+		      unsigned_wide x;
+		      if (INTEGERP (args[n]))
+			x = XUINT (args[n]);
 		      else
-			sprintf (p, this_format, XUINT (args[n]));
+			{
+			  double d = XFLOAT_DATA (args[n]);
+			  if (d < 0)
+			    x = 0;
+			  else
+			    {
+			      x = TYPE_MAXIMUM (unsigned_wide);
+			      if (d < x)
+				x = d;
+			    }
+			}
+		      this_nchars = snprintf (p, prsize, convspec, x);
 		    }
-		  else if (format[-1] == 'c')
-		    sprintf (p, this_format, (int) XFLOAT_DATA (args[n]));
-		  else if (format[-1] == 'd')
-		    /* Maybe we should use "%1.0f" instead so it also works
-		       for values larger than MAXINT.  */
-		    sprintf (p, this_format, (EMACS_INT) XFLOAT_DATA (args[n]));
-		  else
-		    /* Don't sign-extend for octal or hex printing.  */
-		    sprintf (p, this_format, (EMACS_UINT) XFLOAT_DATA (args[n]));
+
+		  if (this_nchars < 0)
+		    format_overflow ();
+		  if (this_nchars <= buf + bufsize - p)
+		    break;
+		  BUF_ROOM (this_nchars);
 		}

+	      *format = c;
+
 	      if (p > buf
 		  && multibyte
 		  && !ASCII_BYTE_P (*((unsigned char *) p - 1))
 		  && !CHAR_HEAD_P (*((unsigned char *) p)))
 		maybe_combine_byte = 1;
-	      this_nchars = strlen (p);
+
 	      if (multibyte)
-		p += str_to_multibyte ((unsigned char *) p,
-				       buf + total - 1 - p, this_nchars);
+		{
+		  EMACS_INT outbytes =
+		    parse_str_to_multibyte ((unsigned char *) p, this_nchars);
+		  BUF_ROOM (outbytes);
+		  p += str_to_multibyte ((unsigned char *) p, outbytes,
+					 this_nchars);
+		}
 	      else
 		p += this_nchars;
 	      nchars += this_nchars;
 	      info[n].end = nchars;
 	    }
-
-	}
-      else if (STRING_MULTIBYTE (args[0]))
-	{
-	  /* Copy a whole multibyte character.  */
-	  if (p > buf
-	      && multibyte
-	      && !ASCII_BYTE_P (*((unsigned char *) p - 1))
-	      && !CHAR_HEAD_P (*format))
-	    maybe_combine_byte = 1;
-	  *p++ = *format++;
-	  while (! CHAR_HEAD_P (*format))
-	    {
-	      discarded[format - format_start] = 2;
-	      *p++ = *format++;
-	    }
-	  nchars++;
-	}
-      else if (multibyte)
-	{
-	  /* Convert a single-byte character to multibyte.  */
-	  int len = copy_text ((unsigned char *) format, (unsigned char *) p,
-			       1, 0, 1);
-
+	}
+      else
+      copy_char:
+	{
+	  char *src = format;
+	  int len;
+
+	  if (multibyte_format)
+	    {
+	      /* Copy a whole multibyte character.  */
+	      if (p > buf
+		  && !ASCII_BYTE_P (*((unsigned char *) p - 1))
+		  && !CHAR_HEAD_P (*format))
+		maybe_combine_byte = 1;
+
+	      do
+		format++;
+	      while (! CHAR_HEAD_P (*format));
+
+	      len = format - src;
+	      memset (&discarded[src + 1 - format_start], 2, len - 1);
+	    }
+	  else
+	    {
+	      if (multibyte)
+		{
+		  /* Convert a single-byte character to multibyte.  */
+		  len = copy_text ((unsigned char *) format, (unsigned char *) p,
+				   1, 0, 1);
+		}
+	      else
+		len = 1;
+	      format++;
+	    }
+
+	  BUF_ROOM (len);
+	  memcpy (p, src, len);
 	  p += len;
-	  format++;
 	  nchars++;
 	}
-      else
-	*p++ = *format++, nchars++;
     }

-  if (p > buf + total)
+  if (bufsize < p - buf)
     abort ();

   if (maybe_combine_byte)
@@ -4089,7 +4118,7 @@
       if (CONSP (props))
 	{
 	  EMACS_INT bytepos = 0, position = 0, translated = 0;
-	  int argn = 1;
+	  EMACS_INT argn = 1;
 	  Lisp_Object list;

 	  /* Adjust the bounds of each text property