Bug 1484943 - Don't assert when trying to formatToParts a NaN value whose sign bit is set. r=anba
authorJeff Walden <jwalden@mit.edu>
Tue, 21 Aug 2018 14:34:50 -0500
changeset 488010 2de8069003dc62198d7744d029454aeb9774fe29
parent 488009 87509a363c9ee2a38998a2e4dacc16e577a877ec
child 488011 03ad53dbb0aecb497e06e463a034d32a87488242
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1484943
milestone63.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 1484943 - Don't assert when trying to formatToParts a NaN value whose sign bit is set. r=anba
js/src/builtin/intl/NumberFormat.cpp
js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js
--- a/js/src/builtin/intl/NumberFormat.cpp
+++ b/js/src/builtin/intl/NumberFormat.cpp
@@ -30,16 +30,17 @@
 #include "vm/JSObject-inl.h"
 
 using namespace js;
 
 using mozilla::AssertedCast;
 using mozilla::IsFinite;
 using mozilla::IsNaN;
 using mozilla::IsNegative;
+using mozilla::SpecificNaN;
 
 using js::intl::CallICU;
 using js::intl::DateTimeFormatOptions;
 using js::intl::GetAvailableLocales;
 using js::intl::IcuLocale;
 
 using JS::AutoStableStringChars;
 
@@ -371,30 +372,36 @@ NewUNumberFormat(JSContext* cx, Handle<N
     }
     unum_setAttribute(nf, UNUM_GROUPING_USED, uUseGrouping);
     unum_setAttribute(nf, UNUM_ROUNDING_MODE, UNUM_ROUND_HALFUP);
 
     return toClose.forget();
 }
 
 static JSString*
-PartitionNumberPattern(JSContext* cx, UNumberFormat* nf, double x,
+PartitionNumberPattern(JSContext* cx, UNumberFormat* nf, double* x,
                        UFieldPositionIterator* fpositer)
 {
-    return CallICU(cx, [nf, x, fpositer](UChar* chars, int32_t size, UErrorCode* status) {
-        return unum_formatDoubleForFields(nf, x, chars, size, fpositer, status);
+    // ICU incorrectly formats NaN values with the sign bit set, as if they
+    // were negative.  Replace all NaNs with a single pattern with sign bit
+    // unset ("positive", that is) until ICU is fixed.
+    if (MOZ_UNLIKELY(IsNaN(*x)))
+        *x = SpecificNaN<double>(0, 1);
+
+    return CallICU(cx, [nf, d = *x, fpositer](UChar* chars, int32_t size, UErrorCode* status) {
+        return unum_formatDoubleForFields(nf, d, chars, size, fpositer, status);
     });
 }
 
 static bool
 intl_FormatNumber(JSContext* cx, UNumberFormat* nf, double x, MutableHandleValue result)
 {
     // Passing null for |fpositer| will just not compute partition information,
     // letting us common up all ICU number-formatting code.
-    JSString* str = PartitionNumberPattern(cx, nf, x, nullptr);
+    JSString* str = PartitionNumberPattern(cx, nf, &x, nullptr);
     if (!str)
         return false;
 
     result.setString(str);
     return true;
 }
 
 using FieldType = ImmutablePropertyNamePtr JSAtomState::*;
@@ -423,16 +430,21 @@ GetFieldTypeForNumberField(UNumberFormat
 
       case UNUM_FRACTION_FIELD:
         return &JSAtomState::fraction;
 
       case UNUM_SIGN_FIELD: {
         // Manual trawling through the ICU call graph appears to indicate that
         // the basic formatting we request will never include a positive sign.
         // But this analysis may be mistaken, so don't absolutely trust it.
+        MOZ_ASSERT(!IsNaN(d),
+                   "ICU appearing not to produce positive-sign among fields, "
+                   "plus our coercing all NaNs to one with sign bit unset "
+                   "(i.e. \"positive\"), means we shouldn't reach here with a "
+                   "NaN value");
         return IsNegative(d) ? &JSAtomState::minusSign : &JSAtomState::plusSign;
       }
 
       case UNUM_PERCENT_FIELD:
         return &JSAtomState::percentSign;
 
       case UNUM_CURRENCY_FIELD:
         return &JSAtomState::currency;
@@ -473,17 +485,17 @@ intl_FormatNumberToParts(JSContext* cx, 
     if (U_FAILURE(status)) {
         intl::ReportInternalError(cx);
         return false;
     }
 
     MOZ_ASSERT(fpositer);
     ScopedICUObject<UFieldPositionIterator, ufieldpositer_close> toClose(fpositer);
 
-    RootedString overallResult(cx, PartitionNumberPattern(cx, nf, x, fpositer));
+    RootedString overallResult(cx, PartitionNumberPattern(cx, nf, &x, fpositer));
     if (!overallResult)
         return false;
 
     RootedArrayObject partsArray(cx, NewDenseEmptyArray(cx));
     if (!partsArray)
         return false;
 
     // First, vacuum up fields in the overall formatted string.
new file mode 100644
--- /dev/null
+++ b/js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js
@@ -0,0 +1,35 @@
+// |reftest| skip-if(!this.hasOwnProperty("Intl"))
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1484943;
+var summary = "Don't crash doing format/formatToParts on -NaN";
+
+print(BUGNUMBER + ": " + summary);
+
+//-----------------------------------------------------------------------------
+
+assertEq("formatToParts" in Intl.NumberFormat(), true);
+
+var nf = new Intl.NumberFormat("en-US");
+var parts;
+
+var values = [NaN, -NaN];
+
+for (var v of values)
+{
+  assertEq(nf.format(v), "NaN");
+
+  parts = nf.formatToParts(v);
+  assertEq(parts.length, 1);
+  assertEq(parts[0].type, "nan");
+  assertEq(parts[0].value, "NaN");
+}
+
+//-----------------------------------------------------------------------------
+
+if (typeof reportCompare === "function")
+  reportCompare(0, 0, 'ok');
+
+print("Tests complete");