Bug 1517433 - do not assert on over-long float printf; r=froydnj
authorTom Tromey <tom@tromey.com>
Wed, 09 Jan 2019 15:50:27 +0000
changeset 510239 be5403f2da75a7a61c2b67870096a3dbda885e3e
parent 510238 ef80c62a30709cfffa64abfa38eb2ad880a906e3
child 510240 2d4af102ece3083438ad8da72fa33fe286c14919
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1517433
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1517433 - do not assert on over-long float printf; r=froydnj mozilla::PrintfTarget::cvt_f release asserts that the desired printf fit into a statically-sized buffer. However, this may not be the case if the user requested a larger width or precision. Handle this unusual case by allocating a temporary buffer. MozReview-Commit-ID: 2WicecHDzDR Differential Revision: https://phabricator.services.mozilla.com/D15989
mozglue/misc/Printf.cpp
mozglue/tests/TestPrintf.cpp
--- a/mozglue/misc/Printf.cpp
+++ b/mozglue/misc/Printf.cpp
@@ -6,16 +6,17 @@
 
 /*
  * Portable safe sprintf code.
  *
  * Author: Kipp E.B. Hickman
  */
 
 #include "mozilla/AllocPolicy.h"
+#include "mozilla/Likely.h"
 #include "mozilla/Printf.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/UniquePtrExtensions.h"
 #include "mozilla/Vector.h"
 
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -253,19 +254,37 @@ bool mozilla::PrintfTarget::cvt_f(double
     const char* p = fin;
     while (*p) {
       MOZ_ASSERT(*p != 'L');
       p++;
     }
   }
 #endif
   size_t len = SprintfLiteral(fout, fin, d);
-  MOZ_RELEASE_ASSERT(len <= sizeof(fout));
+  // Note that SprintfLiteral will always write a \0 at the end, so a
+  // "<=" check here would be incorrect -- the buffer size passed to
+  // snprintf includes the trailing \0, but the returned length does
+  // not.
+  if (MOZ_LIKELY(len < sizeof(fout))) {
+    return emit(fout, len);
+  }
 
-  return emit(fout, len);
+  // Maybe the user used "%500.500f" or something like that.
+  size_t buf_size = len + 1;
+  UniqueFreePtr<char> buf((char*)malloc(buf_size));
+  if (!buf) {
+    return false;
+  }
+  len = snprintf(buf.get(), buf_size, fin, d);
+  // If this assert fails, then SprintfLiteral has a bug -- and in
+  // this case we would like to learn of it, which is why there is a
+  // release assert.
+  MOZ_RELEASE_ASSERT(len < buf_size);
+
+  return emit(buf.get(), len);
 }
 
 /*
  * Convert a string into its printable form.  "width" is the output
  * width. "prec" is the maximum number of characters of "s" to output,
  * where -1 means until NUL.
  */
 bool mozilla::PrintfTarget::cvt_s(const char* s, int width, int prec,
--- a/mozglue/tests/TestPrintf.cpp
+++ b/mozglue/tests/TestPrintf.cpp
@@ -117,16 +117,21 @@ static void TestPrintfFormats() {
   MOZ_RELEASE_ASSERT(print_one("q  ", "%-3c", 'q'));
   MOZ_RELEASE_ASSERT(print_one("  q", "%*c", 3, 'q'));
   MOZ_RELEASE_ASSERT(print_one("q  ", "%*c", -3, 'q'));
 
   // Regression test for bug#1350097.  The bug was an assertion
   // failure caused by printing a very long floating point value.
   print_one("ignore", "%lf", DBL_MAX);
 
+  // Regression test for bug#1517433.  The bug was an assertion
+  // failure caused by printing a floating point value with a large
+  // precision and/or width.
+  print_one("ignore", "%500.500lf", DBL_MAX);
+
   MOZ_RELEASE_ASSERT(print_one("2727", "%" PRIu32, (uint32_t)2727));
   MOZ_RELEASE_ASSERT(print_one("aa7", "%" PRIx32, (uint32_t)2727));
   MOZ_RELEASE_ASSERT(print_one("2727", "%" PRIu64, (uint64_t)2727));
   MOZ_RELEASE_ASSERT(print_one("aa7", "%" PRIx64, (uint64_t)2727));
 
   int n1, n2;
   MOZ_RELEASE_ASSERT(print_one(" hi ", "%n hi %n", &n1, &n2));
   MOZ_RELEASE_ASSERT(n1 == 0);