Bug 1388789 - make nsTextFormatter runtime type-safe; r?froydnj draft
authorTom Tromey <tom@tromey.com>
Fri, 01 Sep 2017 14:03:56 -0600
changeset 667048 50886828b56a7044aff1ed152f2635a5df6d4ebd
parent 667047 21aa782f1c6f66c4e59de257b59a107c1acba431
child 667049 89bf07e11e68a7fba6bedfdc05c9a4f1e969b2e8
push id80599
push userbmo:ttromey@mozilla.com
push dateTue, 19 Sep 2017 16:49:35 +0000
reviewersfroydnj
bugs1388789
milestone57.0a1
Bug 1388789 - make nsTextFormatter runtime type-safe; r?froydnj Change nsTextFormatter functions to template functions, box their arguments, and then make the formatter mostly impervious to type mismatches. Most formatting is done according to the type of the actual argument. MozReview-Commit-ID: H8WmyxFCb7s
xpcom/string/nsTextFormatter.cpp
xpcom/string/nsTextFormatter.h
xpcom/tests/gtest/TestTextFormatter.cpp
--- a/xpcom/string/nsTextFormatter.cpp
+++ b/xpcom/string/nsTextFormatter.cpp
@@ -16,95 +16,52 @@
  */
 
 /*
  * Copied from xpcom/ds/nsTextFormatter.cpp r1.22
  *     Changed to use nsMemory and Frozen linkage
  *                  -- Prasad <prasad@medhas.org>
  */
 
-#include <stdarg.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <string.h>
 #include "prdtoa.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Sprintf.h"
 #include "nsCRTGlue.h"
 #include "nsTextFormatter.h"
 #include "nsMemory.h"
 
-/*
-** Note: on some platforms va_list is defined as an array,
-** and requires array notation.
-*/
-
-#ifdef HAVE_VA_COPY
-#define VARARGS_ASSIGN(foo, bar)        VA_COPY(foo,bar)
-#elif defined(HAVE_VA_LIST_AS_ARRAY)
-#define VARARGS_ASSIGN(foo, bar)	foo[0] = bar[0]
-#else
-#define VARARGS_ASSIGN(foo, bar)	(foo) = (bar)
-#endif
-
-struct SprintfStateStr
+struct nsTextFormatter::SprintfStateStr
 {
   int (*stuff)(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen);
 
   char16_t* base;
   char16_t* cur;
   uint32_t maxlen;
 
   void* stuffclosure;
 };
 
-/*
-** Numbered Arguement State
-*/
-struct NumArgState
-{
-  int type;    /* type of the current ap */
-  va_list ap;  /* point to the corresponding position on ap */
-
-  enum Type
-  {
-    INT16,
-    UINT16,
-    INTN,
-    UINTN,
-    INT32,
-    UINT32,
-    INT64,
-    UINT64,
-    STRING,
-    DOUBLE,
-    INTSTR,
-    UNISTRING,
-    UNKNOWN
-  };
-};
-
-#define NAS_DEFAULT_NUM 20  /* default number of NumberedArgumentState array */
-
 #define _LEFT		0x1
 #define _SIGNED		0x2
 #define _SPACED		0x4
 #define _ZEROS		0x8
 #define _NEG		0x10
+#define _UNSIGNED       0x20
 
 #define ELEMENTS_OF(array_) (sizeof(array_) / sizeof(array_[0]))
 
-#define FREE_IF_NECESSARY(nas) if (nas && (nas != nasArray)) { free(nas); }
-
 /*
 ** Fill into the buffer using the data in src
 */
-static int
-fill2(SprintfStateStr* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
-      int aFlags)
+int
+nsTextFormatter::fill2(SprintfStateStr* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
+                       int aFlags)
 {
   char16_t space = ' ';
   int rv;
 
   aWidth -= aSrcLen;
   /* Right adjusting */
   if ((aWidth > 0) && ((aFlags & _LEFT) == 0)) {
     if (aFlags & _ZEROS) {
@@ -134,32 +91,32 @@ fill2(SprintfStateStr* aState, const cha
     }
   }
   return 0;
 }
 
 /*
 ** Fill a number. The order is: optional-sign zero-filling conversion-digits
 */
-static int
-fill_n(SprintfStateStr* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
-       int aPrec, int aType, int aFlags)
+int
+nsTextFormatter::fill_n(nsTextFormatter::SprintfStateStr* aState, const char16_t* aSrc,
+                        int aSrcLen, int aWidth, int aPrec, int aFlags)
 {
   int zerowidth   = 0;
   int precwidth   = 0;
   int signwidth   = 0;
   int leftspaces  = 0;
   int rightspaces = 0;
   int cvtwidth;
   int rv;
   char16_t sign;
   char16_t space = ' ';
   char16_t zero = '0';
 
-  if ((aType & 1) == 0) {
+  if ((aFlags & _UNSIGNED) == 0) {
     if (aFlags & _NEG) {
       sign = '-';
       signwidth = 1;
     } else if (aFlags & _SIGNED) {
       sign = '+';
       signwidth = 1;
     } else if (aFlags & _SPACED) {
       sign = ' ';
@@ -228,105 +185,64 @@ fill_n(SprintfStateStr* aState, const ch
     if (rv < 0) {
       return rv;
     }
   }
   return 0;
 }
 
 /*
-** Convert a long into its printable form
+** Convert a 64-bit integer into its printable form
 */
-static int
-cvt_l(SprintfStateStr* aState, long aNum, int aWidth, int aPrec, int aRadix,
-      int aType, int aFlags, const char16_t* aHexStr)
+int
+nsTextFormatter::cvt_ll(SprintfStateStr* aState, uint64_t aNum, int aWidth, int aPrec, int aRadix,
+                        int aFlags, const char16_t* aHexStr)
 {
   char16_t cvtbuf[100];
   char16_t* cvt;
   int digits;
 
   /* according to the man page this needs to happen */
-  if ((aPrec == 0) && (aNum == 0)) {
-    return 0;
-  }
-
-  /*
-  ** Converting decimal is a little tricky. In the unsigned case we
-  ** need to stop when we hit 10 digits. In the signed case, we can
-  ** stop when the number is zero.
-  */
-  cvt = &cvtbuf[0] + ELEMENTS_OF(cvtbuf);
-  digits = 0;
-  while (aNum) {
-    int digit = (((unsigned long)aNum) % aRadix) & 0xF;
-    *--cvt = aHexStr[digit];
-    digits++;
-    aNum = (long)(((unsigned long)aNum) / aRadix);
-  }
-  if (digits == 0) {
-    *--cvt = '0';
-    digits++;
-  }
-
-  /*
-  ** Now that we have the number converted without its sign, deal with
-  ** the sign and zero padding.
-  */
-  return fill_n(aState, cvt, digits, aWidth, aPrec, aType, aFlags);
-}
-
-/*
-** Convert a 64-bit integer into its printable form
-*/
-static int
-cvt_ll(SprintfStateStr* aState, int64_t aNum, int aWidth, int aPrec, int aRadix,
-       int aType, int aFlags, const char16_t* aHexStr)
-{
-  char16_t cvtbuf[100];
-  char16_t* cvt;
-  int digits;
-  int64_t rad;
-
-  /* according to the man page this needs to happen */
   if (aPrec == 0 && aNum == 0) {
     return 0;
   }
 
   /*
   ** Converting decimal is a little tricky. In the unsigned case we
   ** need to stop when we hit 10 digits. In the signed case, we can
   ** stop when the number is zero.
   */
-  rad = aRadix;
   cvt = &cvtbuf[0] + ELEMENTS_OF(cvtbuf);
   digits = 0;
   while (aNum != 0) {
-    *--cvt = aHexStr[int32_t(aNum % rad) & 0xf];
+    uint64_t quot = aNum / aRadix;
+    uint64_t rem = aNum % aRadix;
+    *--cvt = aHexStr[rem & 0xf];
     digits++;
-    aNum /= rad;
+    aNum = quot;
   }
   if (digits == 0) {
     *--cvt = '0';
     digits++;
   }
 
   /*
   ** Now that we have the number converted without its sign, deal with
   ** the sign and zero padding.
   */
-  return fill_n(aState, cvt, digits, aWidth, aPrec, aType, aFlags);
+  return fill_n(aState, cvt, digits, aWidth, aPrec, aFlags);
 }
 
 /*
 ** Convert a double precision floating point number into its printable
 ** form.
 */
-static int
-cvt_f(SprintfStateStr* aState, double aDouble, int aWidth, int aPrec,
-      const char16_t aType, int aFlags)
+int
+nsTextFormatter::cvt_f(SprintfStateStr* aState, double aDouble, int aWidth, int aPrec,
+                       const char16_t aType, int aFlags)
 {
   int    mode = 2;
   int    decpt;
   int    sign;
   char   buf[256];
   char*  bufp = buf;
   int    bufsz = 256;
   char   num[256];
@@ -401,17 +317,17 @@ cvt_f(SprintfStateStr* aState, double aD
             aPrec--;
           }
           while (aPrec-- > 0) {
             *bufp++ = '0';
           }
         }
         *bufp++ = exp;
 
-        snprintf(bufp, bufsz - (bufp - buf), "%+03d", decpt - 1);
+        ::snprintf(bufp, bufsz - (bufp - buf), "%+03d", decpt - 1);
         break;
 
       case 'f':
 
         if (decpt < 1) {
           *bufp++ = '0';
           if (aPrec > 0) {
             *bufp++ = '.';
@@ -453,17 +369,17 @@ cvt_f(SprintfStateStr* aState, double aD
           numdigits--;
           if (numdigits > 0) {
             *bufp++ = '.';
             while (*nump) {
               *bufp++ = *nump++;
             }
           }
           *bufp++ = exp;
-          snprintf(bufp, bufsz - (bufp - buf), "%+03d", decpt - 1);
+          ::snprintf(bufp, bufsz - (bufp - buf), "%+03d", decpt - 1);
         } else {
           if (decpt < 1) {
             *bufp++ = '0';
             if (aPrec > 0) {
               *bufp++ = '.';
               while (decpt++) {
                 *bufp++ = '0';
               }
@@ -502,19 +418,19 @@ cvt_f(SprintfStateStr* aState, double aD
   return fill2(aState, rbuf, NS_strlen(rbuf), aWidth, aFlags);
 }
 
 /*
 ** Convert a string into its printable form. |aWidth| is the output
 ** width. |aPrec| is the maximum number of characters of |aStr| to output,
 ** where -1 means until NUL.
 */
-static int
-cvt_S(SprintfStateStr* aState, const char16_t* aStr, int aWidth, int aPrec,
-      int aFlags)
+int
+nsTextFormatter::cvt_S(SprintfStateStr* aState, const char16_t* aStr, int aWidth, int aPrec,
+                       int aFlags)
 {
   int slen;
 
   if (aPrec == 0) {
     return 0;
   }
 
   /* Limit string length by precision value */
@@ -529,341 +445,51 @@ cvt_S(SprintfStateStr* aState, const cha
   return fill2(aState, aStr ? aStr : u"(null)", slen, aWidth, aFlags);
 }
 
 /*
 ** Convert a string into its printable form.  |aWidth| is the output
 ** width. |aPrec| is the maximum number of characters of |aStr| to output,
 ** where -1 means until NUL.
 */
-static int
-cvt_s(SprintfStateStr* aState, const char* aStr, int aWidth, int aPrec, int aFlags)
+int
+nsTextFormatter::cvt_s(nsTextFormatter::SprintfStateStr* aState, const char* aStr, int aWidth,
+                       int aPrec, int aFlags)
 {
   NS_ConvertUTF8toUTF16 utf16Val(aStr);
   return cvt_S(aState, utf16Val.get(), aWidth, aPrec, aFlags);
 }
 
 /*
-** BuildArgArray stands for Numbered Argument list Sprintf
-** for example,
-**	fmp = "%4$i, %2$d, %3s, %1d";
-** the number must start from 1, and no gap among them
-*/
-
-static struct NumArgState*
-BuildArgArray(const char16_t* aFmt, va_list aAp, int* aRv,
-              struct NumArgState* aNasArray)
-{
-  int number = 0, cn = 0, i;
-  const char16_t* p;
-  char16_t  c;
-  struct NumArgState* nas;
-
-  /*
-  **	first pass:
-  **	detemine how many legal % I have got, then allocate space
-  */
-  p = aFmt;
-  *aRv = 0;
-  i = 0;
-  while ((c = *p++) != 0) {
-    if (c != '%') {
-      continue;
-    }
-    /* skip %% case */
-    if ((c = *p++) == '%') {
-      continue;
-    }
-
-    while (c != 0) {
-      if (c > '9' || c < '0') {
-        /* numbered argument csae */
-        if (c == '$') {
-          if (i > 0) {
-            *aRv = -1;
-            return nullptr;
-          }
-          number++;
-          break;
-
-        } else {
-          /* non-numbered argument case */
-          if (number > 0) {
-            *aRv = -1;
-            return nullptr;
-          }
-          i = 1;
-          break;
-        }
-      }
-      c = *p++;
-    }
-  }
-
-  if (number == 0) {
-    return nullptr;
-  }
-
-  if (number > NAS_DEFAULT_NUM) {
-    nas = (struct NumArgState*)moz_xmalloc(number * sizeof(struct NumArgState));
-    if (!nas) {
-      *aRv = -1;
-      return nullptr;
-    }
-  } else {
-    nas = aNasArray;
-  }
-
-  for (i = 0; i < number; i++) {
-    nas[i].type = NumArgState::UNKNOWN;
-  }
-
-  /*
-  ** second pass:
-  ** set nas[].type
-  */
-  p = aFmt;
-  while ((c = *p++) != 0) {
-    if (c != '%') {
-      continue;
-    }
-    c = *p++;
-    if (c == '%') {
-      continue;
-    }
-    cn = 0;
-    /* should improve error check later */
-    while (c && c != '$') {
-      cn = cn * 10 + c - '0';
-      c = *p++;
-    }
-
-    if (!c || cn < 1 || cn > number) {
-      *aRv = -1;
-      break;
-    }
-
-    /* nas[cn] starts from 0, and make sure
-             nas[cn].type is not assigned */
-    cn--;
-    if (nas[cn].type != NumArgState::UNKNOWN) {
-      continue;
-    }
-
-    c = *p++;
-
-    /* width */
-    if (c == '*') {
-      /* not supported feature, for the argument is not numbered */
-      *aRv = -1;
-      break;
-    } else {
-      while ((c >= '0') && (c <= '9')) {
-        c = *p++;
-      }
-    }
-
-    /* precision */
-    if (c == '.') {
-      c = *p++;
-      if (c == '*') {
-        /* not supported feature, for the argument is not numbered */
-        *aRv = -1;
-        break;
-      } else {
-        while ((c >= '0') && (c <= '9')) {
-          c = *p++;
-        }
-      }
-    }
-
-    /* size */
-    nas[cn].type = NumArgState::INTN;
-    if (c == 'h') {
-      nas[cn].type = NumArgState::INT16;
-      c = *p++;
-    } else if (c == 'L') {
-      /* XXX not quite sure here */
-      nas[cn].type = NumArgState::INT64;
-      c = *p++;
-    } else if (c == 'l') {
-      nas[cn].type = NumArgState::INT32;
-      c = *p++;
-      if (c == 'l') {
-        nas[cn].type = NumArgState::INT64;
-        c = *p++;
-      }
-    }
-
-    /* format */
-    switch (c) {
-      case 'd':
-      case 'c':
-      case 'i':
-      case 'o':
-      case 'u':
-      case 'x':
-      case 'X':
-        break;
-
-      case 'e':
-      case 'f':
-      case 'g':
-        nas[cn].type = NumArgState::DOUBLE;
-        break;
-
-      case 'p':
-        /* XXX should use cpp */
-        if (sizeof(void*) == sizeof(int32_t)) {
-          nas[cn].type = NumArgState::UINT32;
-        } else if (sizeof(void*) == sizeof(int64_t)) {
-          nas[cn].type = NumArgState::UINT64;
-        } else if (sizeof(void*) == sizeof(int)) {
-          nas[cn].type = NumArgState::UINTN;
-        } else {
-          nas[cn].type = NumArgState::UNKNOWN;
-        }
-        break;
-
-      case 'C':
-        /* XXX not supported I suppose */
-        MOZ_ASSERT(0);
-        nas[cn].type = NumArgState::UNKNOWN;
-        break;
-
-      case 'S':
-        nas[cn].type = NumArgState::UNISTRING;
-        break;
-
-      case 's':
-        nas[cn].type = NumArgState::STRING;
-        break;
-
-      case 'n':
-        nas[cn].type = NumArgState::INTSTR;
-        break;
-
-      default:
-        MOZ_ASSERT(0);
-        nas[cn].type = NumArgState::UNKNOWN;
-        break;
-    }
-
-    /* get a legal para. */
-    if (nas[cn].type == NumArgState::UNKNOWN) {
-      *aRv = -1;
-      break;
-    }
-  }
-
-
-  /*
-  ** third pass
-  ** fill the nas[cn].ap
-  */
-  if (*aRv < 0) {
-    if (nas != aNasArray) {
-      free(nas);
-    }
-    return nullptr;
-  }
-
-  cn = 0;
-  while (cn < number) {
-    if (nas[cn].type == NumArgState::UNKNOWN) {
-      cn++;
-      continue;
-    }
-
-    VARARGS_ASSIGN(nas[cn].ap, aAp);
-
-    switch (nas[cn].type) {
-      case NumArgState::INT16:
-      case NumArgState::UINT16:
-      case NumArgState::INTN:
-      case NumArgState::UINTN:     (void)va_arg(aAp, int);         break;
-
-      case NumArgState::INT32:     (void)va_arg(aAp, int32_t);     break;
-
-      case NumArgState::UINT32:    (void)va_arg(aAp, uint32_t);    break;
-
-      case NumArgState::INT64:     (void)va_arg(aAp, int64_t);     break;
-
-      case NumArgState::UINT64:    (void)va_arg(aAp, uint64_t);    break;
-
-      case NumArgState::STRING:    (void)va_arg(aAp, char*);       break;
-
-      case NumArgState::INTSTR:    (void)va_arg(aAp, int*);        break;
-
-      case NumArgState::DOUBLE:    (void)va_arg(aAp, double);      break;
-
-      case NumArgState::UNISTRING: (void)va_arg(aAp, char16_t*);   break;
-
-      default:
-        if (nas != aNasArray) {
-          free(nas);
-        }
-        *aRv = -1;
-        va_end(aAp);
-        return nullptr;
-    }
-    cn++;
-  }
-  va_end(aAp);
-  return nas;
-}
-
-
-/*
 ** The workhorse sprintf code.
 */
-static int
-dosprintf(SprintfStateStr* aState, const char16_t* aFmt, va_list aAp)
+int
+nsTextFormatter::dosprintf(SprintfStateStr* aState, const char16_t* aFmt,
+                           mozilla::Span<BoxedValue> aValues)
 {
-  char16_t c;
-  int flags, width, prec, radix, type;
-  union
-  {
-    char16_t ch;
-    int i;
-    long l;
-    int64_t ll;
-    double d;
-    const char* s;
-    const char16_t* S;
-    int* ip;
-  } u;
-  char16_t space = ' ';
-
+  static const char16_t space = ' ';
   static const char16_t hex[] = u"0123456789abcdef";
   static char16_t HEX[] = u"0123456789ABCDEF";
+  static const BoxedValue emptyString(u"");
+
+  char16_t c;
+  int flags, width, prec, radix;
 
   const char16_t* hexp;
-  int rv, i;
-  struct NumArgState* nas = nullptr;
-  struct NumArgState  nasArray[NAS_DEFAULT_NUM];
-
+  int rv;
 
-  /*
-  ** build an argument array, IF the aFmt is numbered argument
-  ** list style, to contain the Numbered Argument list pointers
-  */
-  nas = BuildArgArray(aFmt, aAp, &rv, nasArray);
-  if (rv < 0) {
-    /* the aFmt contains error Numbered Argument format, jliu@netscape.com */
-    MOZ_ASSERT(0);
-    return rv;
-  }
+  // Next argument for non-numbered arguments.
+  size_t nextNaturalArg = 0;
+  // True if we ever saw a numbered argument.
+  bool sawNumberedArg = false;
 
   while ((c = *aFmt++) != 0) {
     if (c != '%') {
       rv = (*aState->stuff)(aState, aFmt - 1, 1);
       if (rv < 0) {
-        va_end(aAp);
-        FREE_IF_NECESSARY(nas);
         return rv;
       }
       continue;
     }
 
     // Save the location of the "%" in case we decide it isn't a
     // format and want to just emit the text from the format string.
     const char16_t* percentPointer = aFmt - 1;
@@ -873,325 +499,349 @@ dosprintf(SprintfStateStr* aState, const
     ** of the strange cases!
     */
     flags = 0;
     c = *aFmt++;
     if (c == '%') {
       /* quoting a % with %% */
       rv = (*aState->stuff)(aState, aFmt - 1, 1);
       if (rv < 0) {
-        va_end(aAp);
-        FREE_IF_NECESSARY(nas);
         return rv;
       }
       continue;
     }
 
-    if (nas) {
-      /* the aFmt contains the Numbered Arguments feature */
-      i = 0;
-      /* should improve error check later */
-      while (c && c != '$') {
-        i = (i * 10) + (c - '0');
+    // Check for a numbered argument.
+    bool sawWidth = false;
+    const BoxedValue* thisArg = nullptr;
+    if (c >= '0' && c <= '9') {
+      size_t argNumber = 0;
+      while (c && c >= '0' && c <= '9') {
+        argNumber = (argNumber * 10) + (c - '0');
         c = *aFmt++;
       }
 
-      if (nas[i - 1].type == NumArgState::UNKNOWN) {
-        if (nas != nasArray) {
-          free(nas);
+      if (c == '$') {
+        // Mixing numbered arguments and implicit arguments is
+        // disallowed.
+        if (nextNaturalArg > 0) {
+          return -1;
         }
-        va_end(aAp);
+
+        c = *aFmt++;
+
+        // Numbered arguments start at 1.
+        --argNumber;
+        if (argNumber >= aValues.Length()) {
+          // A correctness issue but not a safety issue.
+          MOZ_ASSERT(false);
+          thisArg = &emptyString;
+        } else {
+          thisArg = &aValues[argNumber];
+        }
+        sawNumberedArg = true;
+      } else {
+        width = argNumber;
+        sawWidth = true;
+      }
+    }
+
+    if (thisArg == nullptr) {
+      // Mixing numbered arguments and implicit arguments is
+      // disallowed.
+      if (sawNumberedArg) {
         return -1;
       }
 
-      VARARGS_ASSIGN(aAp, nas[i - 1].ap);
-      c = *aFmt++;
+      if (nextNaturalArg >= aValues.Length()) {
+        // A correctness issue but not a safety issue.
+        MOZ_ASSERT(false);
+        thisArg = &emptyString;
+      } else {
+        thisArg = &aValues[nextNaturalArg++];
+      }
     }
 
-    /*
-     * Examine optional flags.  Note that we do not implement the
-     * '#' flag of sprintf().  The ANSI C spec. of the '#' flag is
-     * somewhat ambiguous and not ideal, which is perhaps why
-     * the various sprintf() implementations are inconsistent
-     * on this feature.
-     */
-    while ((c == '-') || (c == '+') || (c == ' ') || (c == '0')) {
-      if (c == '-') {
-        flags |= _LEFT;
+    if (!sawWidth) {
+      /*
+       * Examine optional flags.  Note that we do not implement the
+       * '#' flag of sprintf().  The ANSI C spec. of the '#' flag is
+       * somewhat ambiguous and not ideal, which is perhaps why
+       * the various sprintf() implementations are inconsistent
+       * on this feature.
+       */
+      while ((c == '-') || (c == '+') || (c == ' ') || (c == '0')) {
+        if (c == '-') {
+          flags |= _LEFT;
+        }
+        if (c == '+') {
+          flags |= _SIGNED;
+        }
+        if (c == ' ') {
+          flags |= _SPACED;
+        }
+        if (c == '0') {
+          flags |= _ZEROS;
+        }
+        c = *aFmt++;
       }
-      if (c == '+') {
-        flags |= _SIGNED;
-      }
-      if (c == ' ') {
-        flags |= _SPACED;
+      if (flags & _SIGNED) {
+        flags &= ~_SPACED;
       }
-      if (c == '0') {
-        flags |= _ZEROS;
+      if (flags & _LEFT) {
+        flags &= ~_ZEROS;
       }
-      c = *aFmt++;
-    }
-    if (flags & _SIGNED) {
-      flags &= ~_SPACED;
-    }
-    if (flags & _LEFT) {
-      flags &= ~_ZEROS;
-    }
+
+      /* width */
+      if (c == '*') {
+        // Not supported with numbered arguments.
+        if (sawNumberedArg) {
+          return -1;
+        }
 
-    /* width */
-    if (c == '*') {
-      c = *aFmt++;
-      width = va_arg(aAp, int);
-    } else {
-      width = 0;
-      while ((c >= '0') && (c <= '9')) {
-        width = (width * 10) + (c - '0');
+        if (nextNaturalArg >= aValues.Length() || !aValues[nextNaturalArg].IntCompatible()) {
+          // A correctness issue but not a safety issue.
+          MOZ_ASSERT(false);
+          width = 0;
+        } else {
+          width = aValues[nextNaturalArg++].mValue.mInt;
+        }
         c = *aFmt++;
+      } else {
+        width = 0;
+        while ((c >= '0') && (c <= '9')) {
+          width = (width * 10) + (c - '0');
+          c = *aFmt++;
+        }
       }
     }
 
     /* precision */
     prec = -1;
     if (c == '.') {
       c = *aFmt++;
       if (c == '*') {
+        // Not supported with numbered arguments.
+        if (sawNumberedArg) {
+          return -1;
+        }
+
+        if (nextNaturalArg >= aValues.Length() || !aValues[nextNaturalArg].IntCompatible()) {
+          // A correctness issue but not a safety issue.
+          MOZ_ASSERT(false);
+        } else {
+          prec = aValues[nextNaturalArg++].mValue.mInt;
+        }
         c = *aFmt++;
-        prec = va_arg(aAp, int);
       } else {
         prec = 0;
         while ((c >= '0') && (c <= '9')) {
           prec = (prec * 10) + (c - '0');
           c = *aFmt++;
         }
       }
     }
 
-    /* size */
-    type = NumArgState::INTN;
+    /* Size.  Defaults to 32 bits.  */
+    uint64_t mask = UINT32_MAX;
     if (c == 'h') {
-      type = NumArgState::INT16;
       c = *aFmt++;
+      mask = UINT16_MAX;
     } else if (c == 'L') {
-      /* XXX not quite sure here */
-      type = NumArgState::INT64;
       c = *aFmt++;
+      mask = UINT64_MAX;
     } else if (c == 'l') {
-      type = NumArgState::INT32;
       c = *aFmt++;
       if (c == 'l') {
-        type = NumArgState::INT64;
         c = *aFmt++;
+        mask = UINT64_MAX;
+      } else {
+        mask = UINT32_MAX;
       }
     }
 
     /* format */
     hexp = hex;
+    radix = 10;
+    // Several `MOZ_ASSERT`s below check for argument compatibility
+    // with the format specifier.  These are only debug assertions,
+    // not release assertions, and exist to catch problems in C++
+    // callers of `nsTextFormatter`, as we do not have compile-time
+    // checking of format strings.  In release mode, these assertions
+    // will be no-ops, and we will fall through to printing the
+    // argument based on the known type of the argument.
     switch (c) {
       case 'd':
       case 'i':                               /* decimal/integer */
-        radix = 10;
-        goto fetch_and_convert;
+        MOZ_ASSERT(thisArg->IntCompatible());
+        break;
 
       case 'o':                               /* octal */
+        MOZ_ASSERT(thisArg->IntCompatible());
         radix = 8;
-        type |= 1;
-        goto fetch_and_convert;
+        flags |= _UNSIGNED;
+        break;
 
       case 'u':                               /* unsigned decimal */
+        MOZ_ASSERT(thisArg->IntCompatible());
         radix = 10;
-        type |= 1;
-        goto fetch_and_convert;
+        flags |= _UNSIGNED;
+        break;
 
       case 'x':                               /* unsigned hex */
+        MOZ_ASSERT(thisArg->IntCompatible());
         radix = 16;
-        type |= 1;
-        goto fetch_and_convert;
+        flags |= _UNSIGNED;
+        break;
 
       case 'X':                               /* unsigned HEX */
+        MOZ_ASSERT(thisArg->IntCompatible());
         radix = 16;
         hexp = HEX;
-        type |= 1;
-        goto fetch_and_convert;
-
-        fetch_and_convert:
-        switch (type) {
-          case NumArgState::INT16:
-            u.l = va_arg(aAp, int);
-            if (u.l < 0) {
-              u.l = -u.l;
-              flags |= _NEG;
-            }
-            goto do_long;
-          case NumArgState::UINT16:
-            u.l = va_arg(aAp, int) & 0xffff;
-            goto do_long;
-          case NumArgState::INTN:
-            u.l = va_arg(aAp, int);
-            if (u.l < 0) {
-              u.l = -u.l;
-              flags |= _NEG;
-            }
-            goto do_long;
-          case NumArgState::UINTN:
-            u.l = (long)va_arg(aAp, unsigned int);
-            goto do_long;
-
-          case NumArgState::INT32:
-            u.l = va_arg(aAp, int32_t);
-            if (u.l < 0) {
-              u.l = -u.l;
-              flags |= _NEG;
-            }
-            goto do_long;
-          case NumArgState::UINT32:
-            u.l = (long)va_arg(aAp, uint32_t);
-          do_long:
-            rv = cvt_l(aState, u.l, width, prec, radix, type, flags, hexp);
-            if (rv < 0) {
-              va_end(aAp);
-              FREE_IF_NECESSARY(nas);
-              return rv;
-            }
-            break;
-
-          case NumArgState::INT64:
-            u.ll = va_arg(aAp, int64_t);
-            if (u.ll < 0) {
-              u.ll = -u.ll;
-              flags |= _NEG;
-            }
-            goto do_longlong;
-          case NumArgState::UINT64:
-            u.ll = va_arg(aAp, uint64_t);
-          do_longlong:
-            rv = cvt_ll(aState, u.ll, width, prec, radix, type, flags, hexp);
-            if (rv < 0) {
-              va_end(aAp);
-              FREE_IF_NECESSARY(nas);
-              return rv;
-            }
-            break;
-        }
+        flags |= _UNSIGNED;
         break;
 
       case 'e':
       case 'E':
       case 'f':
       case 'g':
       case 'G':
-        u.d = va_arg(aAp, double);
-        rv = cvt_f(aState, u.d, width, prec, c, flags);
-        if (rv < 0) {
-          return rv;
-        }
+        MOZ_ASSERT(thisArg->mKind == DOUBLE);
+        // Type-based printing below.
+        break;
+
+      case 'S':
+        MOZ_ASSERT(thisArg->mKind == STRING16);
+        // Type-based printing below.
+        break;
+
+      case 's':
+        MOZ_ASSERT(thisArg->mKind == STRING);
+        // Type-based printing below.
         break;
 
-      case 'c':
-        u.ch = va_arg(aAp, int);
-        if ((flags & _LEFT) == 0) {
-          while (width-- > 1) {
-            rv = (*aState->stuff)(aState, &space, 1);
-            if (rv < 0) {
-              va_end(aAp);
-              FREE_IF_NECESSARY(nas);
-              return rv;
+      case 'c': {
+          if (!thisArg->IntCompatible()) {
+            MOZ_ASSERT(false);
+            // Type-based printing below.
+            break;
+          }
+
+          if ((flags & _LEFT) == 0) {
+            while (width-- > 1) {
+              rv = (*aState->stuff)(aState, &space, 1);
+              if (rv < 0) {
+                return rv;
+              }
             }
           }
-        }
-        rv = (*aState->stuff)(aState, &u.ch, 1);
-        if (rv < 0) {
-          va_end(aAp);
-          FREE_IF_NECESSARY(nas);
-          return rv;
-        }
-        if (flags & _LEFT) {
-          while (width-- > 1) {
-            rv = (*aState->stuff)(aState, &space, 1);
-            if (rv < 0) {
-              va_end(aAp);
-              FREE_IF_NECESSARY(nas);
-              return rv;
+          char16_t ch = thisArg->mValue.mInt;
+          rv = (*aState->stuff)(aState, &ch, 1);
+          if (rv < 0) {
+            return rv;
+          }
+          if (flags & _LEFT) {
+            while (width-- > 1) {
+              rv = (*aState->stuff)(aState, &space, 1);
+              if (rv < 0) {
+                return rv;
+              }
             }
           }
         }
-        break;
+        continue;
 
       case 'p':
-        if (sizeof(void*) == sizeof(int32_t)) {
-          type = NumArgState::UINT32;
-        } else if (sizeof(void*) == sizeof(int64_t)) {
-          type = NumArgState::UINT64;
-        } else if (sizeof(void*) == sizeof(int)) {
-          type = NumArgState::UINTN;
-        } else {
-          MOZ_ASSERT(0);
-          break;
+        if (!thisArg->PointerCompatible()) {
+            MOZ_ASSERT(false);
+            break;
         }
-        radix = 16;
-        goto fetch_and_convert;
-
-#if 0
-      case 'C':
-        /* XXX not supported I suppose */
-        MOZ_ASSERT(0);
-        break;
-#endif
-
-      case 'S':
-        u.S = va_arg(aAp, const char16_t*);
-        rv = cvt_S(aState, u.S, width, prec, flags);
+        static_assert(sizeof(uint64_t) >= sizeof(void*), "pointers are larger than 64 bits");
+        rv = cvt_ll(aState, uintptr_t(thisArg->mValue.mPtr), width, prec, 16, flags | _UNSIGNED,
+                    hexp);
         if (rv < 0) {
-          va_end(aAp);
-          FREE_IF_NECESSARY(nas);
           return rv;
         }
-        break;
-
-      case 's':
-        u.s = va_arg(aAp, const char*);
-        rv = cvt_s(aState, u.s, width, prec, flags);
-        if (rv < 0) {
-          va_end(aAp);
-          FREE_IF_NECESSARY(nas);
-          return rv;
-        }
-        break;
+        continue;
 
       case 'n':
-        u.ip = va_arg(aAp, int*);
-        if (u.ip) {
-          *u.ip = aState->cur - aState->base;
+        if (thisArg->mKind != INTPOINTER) {
+          return -1;
         }
-        break;
+
+        if (thisArg->mValue.mIntPtr != nullptr) {
+          *thisArg->mValue.mIntPtr = aState->cur - aState->base;
+        }
+        continue;
 
       default:
         /* Not a % token after all... skip it */
         rv = (*aState->stuff)(aState, percentPointer, aFmt - percentPointer);
         if (rv < 0) {
-          va_end(aAp);
-          FREE_IF_NECESSARY(nas);
           return rv;
         }
+        continue;
+    }
+
+    // If we get here, we want to handle the argument according to its
+    // actual type; modified by the flags as appropriate.
+    switch (thisArg->mKind) {
+      case INT:
+      case UINT: {
+          int64_t val = thisArg->mValue.mInt;
+          if ((flags & _UNSIGNED) == 0 && val < 0) {
+            val = -val;
+            flags |= _NEG;
+          }
+          rv = cvt_ll(aState, uint64_t(val) & mask, width, prec, radix, flags, hexp);
+        }
+        break;
+      case INTPOINTER:
+      case POINTER:
+        // Always treat these as unsigned hex, no matter the format.
+        static_assert(sizeof(uint64_t) >= sizeof(void*), "pointers are larger than 64 bits");
+        rv = cvt_ll(aState, uintptr_t(thisArg->mValue.mPtr), width, prec, 16, flags | _UNSIGNED,
+                    hexp);
+        break;
+      case DOUBLE:
+        if (c != 'f' && c != 'E' && c != 'e' && c != 'G' && c != 'g') {
+          // Pick some default.
+          c = 'g';
+        }
+        rv = cvt_f(aState, thisArg->mValue.mDouble, width, prec, c, flags);
+        break;
+      case STRING:
+        rv = cvt_s(aState, thisArg->mValue.mString, width, prec, flags);
+        break;
+      case STRING16:
+        rv = cvt_S(aState, thisArg->mValue.mString16, width, prec, flags);
+        break;
+      default:
+        // Can't happen.
+        MOZ_ASSERT(0);
+    }
+
+    if (rv < 0) {
+      return rv;
     }
   }
 
   /* Stuff trailing NUL */
   char16_t null = '\0';
 
   rv = (*aState->stuff)(aState, &null, 1);
 
-  va_end(aAp);
-  FREE_IF_NECESSARY(nas);
-
   return rv;
 }
 
 /************************************************************************/
 
-static int
-StringStuff(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen)
+int
+nsTextFormatter::StringStuff(nsTextFormatter::SprintfStateStr* aState, const char16_t* aStr,
+                             uint32_t aLen)
 {
   if (*aStr == '\0') {
     return 0;
   }
 
   ptrdiff_t off = aState->cur - aState->base;
 
   nsAString* str = static_cast<nsAString*>(aState->stuffclosure);
@@ -1199,102 +849,70 @@ StringStuff(SprintfStateStr* aState, con
 
   aState->base = str->BeginWriting();
   aState->cur = aState->base + off;
 
   return 0;
 }
 
 void
-nsTextFormatter::ssprintf(nsAString& aOut, const char16_t* aFmt, ...)
-{
-  va_list ap;
-
-  va_start(ap, aFmt);
-  nsTextFormatter::vssprintf(aOut, aFmt, ap);
-  va_end(ap);
-}
-
-void
-nsTextFormatter::vssprintf(nsAString& aOut, const char16_t* aFmt, va_list aAp)
+nsTextFormatter::vssprintf(nsAString& aOut, const char16_t* aFmt,
+                           mozilla::Span<BoxedValue> aValues)
 {
   SprintfStateStr ss;
   ss.stuff = StringStuff;
   ss.base = 0;
   ss.cur = 0;
   ss.maxlen = 0;
   ss.stuffclosure = &aOut;
 
   aOut.Truncate();
-  dosprintf(&ss, aFmt, aAp);
+  dosprintf(&ss, aFmt, aValues);
 }
 
 /*
 ** Stuff routine that discards overflow data
 */
-static int
-LimitStuff(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen)
+int
+nsTextFormatter::LimitStuff(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen)
 {
   uint32_t limit = aState->maxlen - (aState->cur - aState->base);
 
   if (aLen > limit) {
     aLen = limit;
   }
   while (aLen) {
     --aLen;
     *aState->cur++ = *aStr++;
   }
   return 0;
 }
 
-/*
-** sprintf into a fixed size buffer. Make sure there is a NUL at the end
-** when finished.
-*/
-uint32_t
-nsTextFormatter::snprintf(char16_t* aOut, uint32_t aOutLen,
-                          const char16_t* aFmt, ...)
-{
-  va_list ap;
-  uint32_t rv;
-
-  MOZ_ASSERT((int32_t)aOutLen > 0);
-  if ((int32_t)aOutLen <= 0) {
-    return 0;
-  }
-
-  va_start(ap, aFmt);
-  rv = nsTextFormatter::vsnprintf(aOut, aOutLen, aFmt, ap);
-  va_end(ap);
-  return rv;
-}
-
 uint32_t
 nsTextFormatter::vsnprintf(char16_t* aOut, uint32_t aOutLen,
-                           const char16_t* aFmt, va_list aAp)
+                           const char16_t* aFmt, mozilla::Span<BoxedValue> aValues)
 {
   SprintfStateStr ss;
   uint32_t n;
 
   MOZ_ASSERT((int32_t)aOutLen > 0);
   if ((int32_t)aOutLen <= 0) {
     return 0;
   }
 
   ss.stuff = LimitStuff;
   ss.base = aOut;
   ss.cur = aOut;
   ss.maxlen = aOutLen;
-  int result = dosprintf(&ss, aFmt, aAp);
+  int result = dosprintf(&ss, aFmt, aValues);
 
   /* If we added chars, and we didn't append a null, do it now. */
   if ((ss.cur != ss.base) && (*(ss.cur - 1) != '\0')) {
     *(--ss.cur) = '\0';
   }
 
   if (result < 0) {
     return -1;
   }
 
   n = ss.cur - ss.base;
   return n ? n - 1 : n;
 }
-
--- a/xpcom/string/nsTextFormatter.h
+++ b/xpcom/string/nsTextFormatter.h
@@ -29,42 +29,175 @@
  **	%p - pointer (deals with machine dependent pointer size)
  **	%f - float
  **	%g - float
  */
 #include <stdio.h>
 #include <stdarg.h>
 #include "nscore.h"
 #include "nsStringGlue.h"
+#include "mozilla/Span.h"
+#include "mozilla/TypeTraits.h"
 
 #ifdef XPCOM_GLUE
 #error "nsTextFormatter is not available in the standalone glue due to NSPR dependencies."
 #endif
 
 class nsTextFormatter
 {
 public:
 
   /*
    * sprintf into a fixed size buffer. Guarantees that the buffer is null
    * terminated. Returns the length of the written output, NOT including the
    * null terminator, or (uint32_t)-1 if an error occurs.
    */
-  static uint32_t snprintf(char16_t* aOut, uint32_t aOutLen,
-                           const char16_t* aFmt, ...);
+  template<typename ...T>
+  static uint32_t snprintf(char16_t* aOut, uint32_t aOutLen, const char16_t* aFmt, T... aArgs) {
+    BoxedValue values[] = { BoxedValue(aArgs)... };
+    return vsnprintf(aOut, aOutLen, aFmt, mozilla::MakeSpan(values, sizeof...(aArgs)));
+  }
 
   /*
    * sprintf into an existing nsAString, overwriting any contents it already
    * has. Infallible.
    */
-  static void ssprintf(nsAString& aOut, const char16_t* aFmt, ...);
+  template<typename ...T>
+  static void ssprintf(nsAString& aOut, const char16_t* aFmt, T... aArgs) {
+    BoxedValue values[] = { BoxedValue(aArgs)... };
+    vssprintf(aOut, aFmt, mozilla::MakeSpan(values, sizeof...(aArgs)));
+  }
 
 private:
 
-  /*
-   * va_list forms of the above.
-   */
+  enum ArgumentKind {
+    INT,
+    UINT,
+    POINTER,
+    DOUBLE,
+    STRING,
+    STRING16,
+    INTPOINTER,
+  };
+
+  union ValueUnion {
+    int64_t mInt;
+    uint64_t mUInt;
+    void const* mPtr;
+    double mDouble;
+    char const* mString;
+    char16_t const* mString16;
+    int* mIntPtr;
+  };
+
+  struct BoxedValue {
+    ArgumentKind mKind;
+    ValueUnion mValue;
+
+    explicit BoxedValue(int aArg)
+      : mKind(INT)
+    {
+      mValue.mInt = aArg;
+    }
+
+    explicit BoxedValue(unsigned int aArg)
+      : mKind(UINT)
+    {
+      mValue.mUInt = aArg;
+    }
+
+    explicit BoxedValue(long aArg)
+      : mKind(INT)
+    {
+      mValue.mInt = aArg;
+    }
+
+    explicit BoxedValue(unsigned long aArg)
+      : mKind(UINT)
+    {
+      mValue.mUInt = aArg;
+    }
+
+    explicit BoxedValue(long long aArg)
+      : mKind(INT)
+    {
+      mValue.mInt = aArg;
+    }
+
+    explicit BoxedValue(unsigned long long aArg)
+      : mKind(UINT)
+    {
+      mValue.mUInt = aArg;
+    }
+
+    explicit BoxedValue(const void* aArg)
+      : mKind(POINTER)
+    {
+      mValue.mPtr = aArg;
+    }
+
+    explicit BoxedValue(double aArg)
+      : mKind(DOUBLE)
+    {
+      mValue.mDouble = aArg;
+    }
+
+    explicit BoxedValue(const char* aArg)
+      : mKind(STRING)
+    {
+      mValue.mString = aArg;
+    }
+
+    explicit BoxedValue(const char16_t* aArg)
+      : mKind(STRING16)
+    {
+      mValue.mString16 = aArg;
+    }
+
+#if defined(MOZ_USE_CHAR16_WRAPPER)
+    explicit BoxedValue(const char16ptr_t aArg)
+      : mKind(STRING16)
+    {
+      mValue.mString16 = aArg;
+    }
+
+#endif
+
+    explicit BoxedValue(int* aArg)
+      : mKind(INTPOINTER)
+    {
+      mValue.mIntPtr = aArg;
+    }
+
+    bool IntCompatible() const {
+      return mKind == INT || mKind == UINT;
+    }
+
+    bool PointerCompatible() const {
+      return mKind == POINTER || mKind == STRING || mKind == STRING16 || mKind == INTPOINTER;
+    }
+  };
+
+  struct SprintfStateStr;
+
+  static int fill2(SprintfStateStr* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
+                   int aFlags);
+  static int fill_n(SprintfStateStr* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
+                    int aPrec, int aFlags);
+  static int cvt_ll(SprintfStateStr* aState, uint64_t aNum, int aWidth, int aPrec, int aRadix,
+                    int aFlags, const char16_t* aHexStr);
+  static int cvt_f(SprintfStateStr* aState, double aDouble, int aWidth, int aPrec,
+                   const char16_t aType, int aFlags);
+  static int cvt_S(SprintfStateStr* aState, const char16_t* aStr, int aWidth, int aPrec,
+                   int aFlags);
+  static int cvt_s(SprintfStateStr* aState, const char* aStr, int aWidth, int aPrec,
+                   int aFlags);
+  static int dosprintf(SprintfStateStr* aState, const char16_t* aFmt,
+                       mozilla::Span<BoxedValue> aValues);
+  static int StringStuff(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen);
+  static int LimitStuff(SprintfStateStr* aState, const char16_t* aStr, uint32_t aLen);
   static uint32_t vsnprintf(char16_t* aOut, uint32_t aOutLen, const char16_t* aFmt,
-                            va_list aAp);
-  static void vssprintf(nsAString& aOut, const char16_t* aFmt, va_list aAp);
+                            mozilla::Span<BoxedValue> aValues);
+  static void vssprintf(nsAString& aOut, const char16_t* aFmt,
+                        mozilla::Span<BoxedValue> aValues);
 };
 
 #endif /* nsTextFormatter_h___ */
--- a/xpcom/tests/gtest/TestTextFormatter.cpp
+++ b/xpcom/tests/gtest/TestTextFormatter.cpp
@@ -30,10 +30,189 @@ TEST(TextFormatter, Tests)
   for (uint32_t i=0; i<out.Length(); i++) {
     ASSERT_EQ(uout[i], expected[i]);
   }
 
   // Test that an unrecognized escape is passed through.
   nsString out2;
   nsTextFormatter::ssprintf(out2, u"%1m!", 23);
   EXPECT_STREQ("%1m!", NS_ConvertUTF16toUTF8(out2).get());
+
+  // Treat NULL the same in both %s cases.
+  nsTextFormatter::ssprintf(out2, u"%s %S", (char*) nullptr, (char16_t*) nullptr);
+  EXPECT_STREQ("(null) (null)", NS_ConvertUTF16toUTF8(out2).get());
+
+  nsTextFormatter::ssprintf(out2, u"%lld", INT64_MIN);
+  EXPECT_STREQ("-9223372036854775808", NS_ConvertUTF16toUTF8(out2).get());
+}
+
+/*
+ * Check misordered parameters
+ */
+
+TEST(TextFormatterOrdering, orders)
+{
+  nsString out;
+
+  // plain list
+  nsTextFormatter::ssprintf(out, u"%S %S %S", u"1", u"2", u"3");
+  EXPECT_STREQ("1 2 3", NS_ConvertUTF16toUTF8(out).get());
+
+  // ordered list
+  nsTextFormatter::ssprintf(out, u"%2$S %3$S %1$S", u"1", u"2", u"3");
+  EXPECT_STREQ("2 3 1", NS_ConvertUTF16toUTF8(out).get());
+
+  // Mixed ordered list and non-ordered does not work.  This shouldn't
+  // crash (hence the calls to ssprintf) but should fail for for
+  // snprintf.
+  nsTextFormatter::ssprintf(out, u"%2S %S %1$S", u"1", u"2", u"3");
+  nsTextFormatter::ssprintf(out, u"%S %2$S", u"1", u"2");
+  char16_t buffer[1024];            // plenty big
+  EXPECT_EQ(nsTextFormatter::snprintf(buffer, sizeof(buffer), u"%2S %S %1$S", u"1", u"2", u"3"),
+            uint32_t(-1));
+  EXPECT_EQ(nsTextFormatter::snprintf(buffer, sizeof(buffer), u"%S %2$S", u"1", u"2"),
+            uint32_t(-1));
+
+  // Referencing an extra param returns empty strings in release.
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u" %2$S ", u"1");
+  EXPECT_STREQ("  ", NS_ConvertUTF16toUTF8(out).get());
+#endif
+
+  // Double referencing existing argument works
+  nsTextFormatter::ssprintf(out, u"%1$S %1$S", u"1");
+  EXPECT_STREQ("1 1", NS_ConvertUTF16toUTF8(out).get());
+
+  // Dropping trailing argument works
+  nsTextFormatter::ssprintf(out, u" %1$S ", u"1", u"2");
+  EXPECT_STREQ(" 1 ", NS_ConvertUTF16toUTF8(out).get());
+
+  // Dropping leading arguments works
+  nsTextFormatter::ssprintf(out, u" %2$S ", u"1", u"2");
+  EXPECT_STREQ(" 2 ", NS_ConvertUTF16toUTF8(out).get());
+
+  // Dropping middle arguments works
+  nsTextFormatter::ssprintf(out, u" %3$S %1$S ", u"1", u"2", u"3");
+  EXPECT_STREQ(" 3 1 ", NS_ConvertUTF16toUTF8(out).get());
+}
+
+/*
+ * Tests to validate that horrible things don't happen if the passed-in
+ * variable and the formatter don't match.
+ */
+TEST(TextFormatterTestMismatch, format_d)
+{
+  nsString out;
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%d", int(-1));
+  EXPECT_STREQ("-1", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%d", uint32_t(-1));
+  EXPECT_STREQ("4294967295", NS_ConvertUTF16toUTF8(out).get());
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%d", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%d", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%d", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
 }
 
+TEST(TextFormatterTestMismatch, format_u)
+{
+  nsString out;
+  nsTextFormatter::ssprintf(out, u"%u", int(-1));
+  EXPECT_STREQ("4294967295", NS_ConvertUTF16toUTF8(out).get());
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%u", uint32_t(-1));
+  EXPECT_STREQ("4294967295", NS_ConvertUTF16toUTF8(out).get());
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%u", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%u", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%u", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
+}
+
+TEST(TextFormatterTestMismatch, format_x)
+{
+  nsString out;
+  nsTextFormatter::ssprintf(out, u"%x", int32_t(-1));
+  EXPECT_STREQ("ffffffff", NS_ConvertUTF16toUTF8(out).get());
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%x", uint32_t(-1));
+  EXPECT_STREQ("ffffffff", NS_ConvertUTF16toUTF8(out).get());
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%x", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%x", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%x", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
+}
+
+TEST(TextFormatterTestMismatch, format_s)
+{
+  nsString out;
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%s", int(-1));
+  EXPECT_STREQ("-1", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%s", uint32_t(-1));
+  EXPECT_STREQ("4294967295", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%s", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+#endif
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%s", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%s", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
+}
+
+TEST(TextFormatterTestMismatch, format_S)
+{
+  nsString out;
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%S", int32_t(-1));
+  EXPECT_STREQ("-1", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%S", uint32_t(-1));
+  EXPECT_STREQ("4294967295", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%S", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%S", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%S", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+}
+
+TEST(TextFormatterTestMismatch, format_c)
+{
+  nsString out;
+  nsTextFormatter::ssprintf(out, u"%c", int32_t(-1));
+  EXPECT_EQ(1u, out.Length());
+  EXPECT_EQ((uint16_t)-1, out.CharAt(0));  // not useful for humans :-/
+  nsTextFormatter::ssprintf(out, u"%c", uint32_t(-1));
+  EXPECT_EQ(1u, out.Length());
+  EXPECT_EQ((uint16_t)-1, out.CharAt(0));  // not useful for humans :-/
+#ifndef DEBUG
+  nsTextFormatter::ssprintf(out, u"%c", float(3.5));
+  EXPECT_STREQ("3.5", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%c", "foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+  nsTextFormatter::ssprintf(out, u"%c", u"foo");
+  EXPECT_STREQ("foo", NS_ConvertUTF16toUTF8(out).get());
+#endif
+
+  // just for completeness, this is our format, and works
+  nsTextFormatter::ssprintf(out, u"%c", 'c');
+  EXPECT_EQ(1u, out.Length());
+  EXPECT_EQ(u'c', out.CharAt(0));
+  nsTextFormatter::ssprintf(out, u"%c", u'c');
+  EXPECT_EQ(1u, out.Length());
+  EXPECT_EQ(u'c', out.CharAt(0));
+}