Bug 1517433 - Do not assert on over-long float printf. r=froydnj, a=RyanVM
authorTom Tromey <tom@tromey.com>
Wed, 09 Jan 2019 15:50:27 +0000
changeset 506653 60be2a80ee0e8c213d16a192780a9f70ac6f5d5a
parent 506652 694ef5b65db8c5e3570e6e1a9bee47608b4d3ba9
child 506654 da0b07a8386c07b7b4c54a5c001fbedc64fa79eb
push id10499
push userryanvm@gmail.com
push dateSat, 12 Jan 2019 20:11:30 +0000
treeherdermozilla-beta@da0b07a8386c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, RyanVM
bugs1517433
milestone65.0
Bug 1517433 - Do not assert on over-long float printf. r=froydnj, a=RyanVM 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);