Reland "[Temporal] Use double/int32_t instead of int64_t for duration parsing"

This is a reland of commit a165e82e

The reason of revert is  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/objects/js-temporal-objects.cc:3837:22   which is the line
"nanoseconds_mv = std::round((seconds_mv - std::floor(seconds_mv)) * 1e9);"
where seconds_mv is a double and nanoseconds_mv is a int32_t
In this reland, we change the type of nanoseconds_mv to double to avoid the ubsan error.


Original change's description:
> [Temporal] Use double/int32_t instead of int64_t for duration parsing
>
> Use double and int32_t instead of int64_t in duration parsing result
> so we can parse very large duration fields as infinity and throw RangeError in later stages. The three fractional parts can hold up value from 0 to 999,999,999 so we use int32_t to hold it. Other part could be infinity so we use double to hold it. Also rearrange the order of the three int32_t in the struct ParsedISO8601Duration after all the double
>
> Bug: v8:11544
> Change-Id: I7e5b02f7c7bbb60997f1419f016aed61dd3e0d6c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3840761
> Reviewed-by: Shu-yu Guo <syg@chromium.org>
> Commit-Queue: Frank Tang <ftang@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82754}

Bug: v8:11544
Change-Id: If8b72cb4912d8b4fc4c286fc856ea59df5cf0bb7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3858576Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83090}

src/objects/js-temporal-objects.cc

@@ -3765,7 +3765,7 @@ Maybe<DurationRecord> CreateDurationRecord(Isolate* isolate,
   return Just(duration);
 }
 
-inline int64_t IfEmptyReturnZero(int64_t value) {
+inline double IfEmptyReturnZero(double value) {
   return value == ParsedISO8601Duration::kEmpty ? 0 : value;
 }
 
@@ -3774,18 +3774,10 @@ Maybe<DurationRecord> ParseTemporalDurationString(Isolate* isolate,
                                                   Handle<String> iso_string) {
   TEMPORAL_ENTER_FUNC();
   // In this funciton, we use 'double' as type for all mathematical values
-  // except the three three units < seconds. For all others, we use 'double'
   // because in
   // https://tc39.es/proposal-temporal/#sec-properties-of-temporal-duration-instances
   // they are "A float64-representable integer representing the number" in the
   // internal slots.
-  // For milliseconds_mv, microseconds_mv, and nanoseconds_mv, we use int32_t
-  // instead because their maximum number during calculation is 999999999,
-  // which can be encoded in 30 bits and the parsed.seconds_fraction return from
-  // the ISO8601 parser are stored in an integer, in the unit of nanoseconds.
-  // Therefore, use "int32_t" will avoid rounding error for the final
-  // calculating of nanoseconds_mv.
-  //
   // 1. Let duration be ParseText(StringToCodePoints(isoString),
   // TemporalDurationString).
   // 2. If duration is a List of errors, throw a RangeError exception.
@@ -3827,7 +3819,11 @@ Maybe<DurationRecord> ParseTemporalDurationString(Isolate* isolate,
                                    Nothing<DurationRecord>());
     }
     // b. Let fHoursDigits be the substring of CodePointsToString(fHours)
-    // from 1. c. Let fHoursScale be the length of fHoursDigits. d. Let
+    // from 1.
+    //
+    // c. Let fHoursScale be the length of fHoursDigits.
+    //
+    // d. Let
     // minutesMV be ! ToIntegerOrInfinity(fHoursDigits) / 10^fHoursScale × 60.
     minutes_mv = IfEmptyReturnZero(parsed->hours_fraction) * 60.0 / 1e9;
     // 10. Else,
@@ -3847,9 +3843,12 @@ Maybe<DurationRecord> ParseTemporalDurationString(Isolate* isolate,
                                    Nothing<DurationRecord>());
     }
     // b. Let fMinutesDigits be the substring of CodePointsToString(fMinutes)
-    // from 1. c. Let fMinutesScale be the length of fMinutesDigits. d. Let
-    // secondsMV be ! ToIntegerOrInfinity(fMinutesDigits) / 10^fMinutesScale
-    // × 60.
+    // from 1.
+    //
+    // c. Let fMinutesScale be the length of fMinutesDigits.
+    //
+    // d. Let secondsMV be ! ToIntegerOrInfinity(fMinutesDigits) /
+    // 10^fMinutesScale × 60.
     seconds_mv = IfEmptyReturnZero(parsed->minutes_fraction) * 60.0 / 1e9;
     // 12. Else if seconds is not empty, then
   } else if (parsed->whole_seconds != ParsedISO8601Duration::kEmpty) {
@@ -3860,13 +3859,21 @@ Maybe<DurationRecord> ParseTemporalDurationString(Isolate* isolate,
     // a. Let secondsMV be remainder(minutesMV, 1) × 60.
     seconds_mv = (minutes_mv - std::floor(minutes_mv)) * 60.0;
   }
-  int32_t milliseconds_mv;
-  int32_t nanoseconds_mv;
+  double milliseconds_mv, microseconds_mv, nanoseconds_mv;
+  // Note: In step 14-17, we calculate from nanoseconds_mv to miilliseconds_mv
+  // in the reversee order of the spec text to avoid numerical errors would be
+  // introduced by multiple division inside the remainder operations. If we
+  // strickly follow the order by using double, the end result of nanoseconds_mv
+  // will be wrong due to numerical errors.
+  //
   // 14. If fSeconds is not empty, then
   if (parsed->seconds_fraction != ParsedISO8601Duration::kEmpty) {
     // a. Let fSecondsDigits be the substring of CodePointsToString(fSeconds)
-    // from 1. b. Let fSecondsScale be the length of fSecondsDigits. c. Let
-    // millisecondsMV be ! ToIntegerOrInfinity(fSecondsDigits) /
+    // from 1.
+    //
+    // b. Let fSecondsScale be the length of fSecondsDigits.
+    //
+    // c. Let millisecondsMV be ! ToIntegerOrInfinity(fSecondsDigits) /
     // 10^fSecondsScale × 1000.
     DCHECK_LE(IfEmptyReturnZero(parsed->seconds_fraction), 1e9);
     nanoseconds_mv = std::round(IfEmptyReturnZero(parsed->seconds_fraction));
@@ -3875,15 +3882,13 @@ Maybe<DurationRecord> ParseTemporalDurationString(Isolate* isolate,
     // a. Let millisecondsMV be remainder(secondsMV, 1) × 1000.
     nanoseconds_mv = std::round((seconds_mv - std::floor(seconds_mv)) * 1e9);
   }
-  milliseconds_mv = nanoseconds_mv / 1000000;
+  milliseconds_mv = std::floor(nanoseconds_mv / 1000000);
   // 16. Let microsecondsMV be remainder(millisecondsMV, 1) × 1000.
-  int32_t microseconds_mv = (nanoseconds_mv / 1000) % 1000;
+  microseconds_mv = std::floor(nanoseconds_mv / 1000) -
+                    std::floor(nanoseconds_mv / 1000000) * 1000;
   // 17. Let nanosecondsMV be remainder(microsecondsMV, 1) × 1000.
-  nanoseconds_mv = nanoseconds_mv % 1000;
+  nanoseconds_mv -= std::floor(nanoseconds_mv / 1000) * 1000;
 
-  DCHECK_LE(milliseconds_mv, 1000);
-  DCHECK_LE(microseconds_mv, 1000);
-  DCHECK_LE(nanoseconds_mv, 1000);
   // 18. If sign contains the code point 0x002D (HYPHEN-MINUS) or 0x2212 (MINUS
   // SIGN), then a. Let factor be −1.
   // 19. Else,

src/temporal/temporal-parser.cc

@@ -186,14 +186,6 @@ int32_t ScanFractionalPart(base::Vector<Char> str, int32_t s, int32_t* out) {
   return cur - s;
 }
 
-template <typename Char>
-int32_t ScanFractionalPart(base::Vector<Char> str, int32_t s, int64_t* out) {
-  int32_t out32;
-  int32_t len = ScanFractionalPart(str, s, &out32);
-  *out = out32;
-  return len;
-}
-
 // TimeFraction: FractionalPart
 SCAN_FORWARD(TimeFractionalPart, FractionalPart, int32_t)
 
@@ -1438,21 +1430,10 @@ bool SatisfyTemporalCalendarString(base::Vector<Char> str,
 
 // Duration
 
-SCAN_FORWARD(TimeFractionalPart, FractionalPart, int64_t)
-
-template <typename Char>
-int32_t ScanFraction(base::Vector<Char> str, int32_t s, int64_t* out) {
-  if (str.length() < (s + 2) || !IsDecimalSeparator(str[s])) return 0;
-  int32_t len = ScanTimeFractionalPart(str, s + 1, out);
-  return (len == 0) ? 0 : len + 1;
-}
-
-SCAN_FORWARD(TimeFraction, Fraction, int64_t)
-
 // Digits : Digit [Digits]
 
 template <typename Char>
-int32_t ScanDigits(base::Vector<Char> str, int32_t s, int64_t* out) {
+int32_t ScanDigits(base::Vector<Char> str, int32_t s, double* out) {
   if (str.length() < (s + 1) || !IsDecimalDigit(str[s])) return 0;
   *out = ToInt(str[s]);
   int32_t len = 1;
@@ -1463,38 +1444,38 @@ int32_t ScanDigits(base::Vector<Char> str, int32_t s, int64_t* out) {
   return len;
 }
 
-SCAN_FORWARD(DurationYears, Digits, int64_t)
-SCAN_FORWARD(DurationMonths, Digits, int64_t)
-SCAN_FORWARD(DurationWeeks, Digits, int64_t)
-SCAN_FORWARD(DurationDays, Digits, int64_t)
+SCAN_FORWARD(DurationYears, Digits, double)
+SCAN_FORWARD(DurationMonths, Digits, double)
+SCAN_FORWARD(DurationWeeks, Digits, double)
+SCAN_FORWARD(DurationDays, Digits, double)
 
 // DurationWholeHours : Digits
-SCAN_FORWARD(DurationWholeHours, Digits, int64_t)
+SCAN_FORWARD(DurationWholeHours, Digits, double)
 
 // DurationWholeMinutes : Digits
-SCAN_FORWARD(DurationWholeMinutes, Digits, int64_t)
+SCAN_FORWARD(DurationWholeMinutes, Digits, double)
 
 // DurationWholeSeconds : Digits
-SCAN_FORWARD(DurationWholeSeconds, Digits, int64_t)
+SCAN_FORWARD(DurationWholeSeconds, Digits, double)
 
 // DurationHoursFraction : TimeFraction
-SCAN_FORWARD(DurationHoursFraction, TimeFraction, int64_t)
+SCAN_FORWARD(DurationHoursFraction, TimeFraction, int32_t)
 
 // DurationMinutesFraction : TimeFraction
-SCAN_FORWARD(DurationMinutesFraction, TimeFraction, int64_t)
+SCAN_FORWARD(DurationMinutesFraction, TimeFraction, int32_t)
 
 // DurationSecondsFraction : TimeFraction
-SCAN_FORWARD(DurationSecondsFraction, TimeFraction, int64_t)
+SCAN_FORWARD(DurationSecondsFraction, TimeFraction, int32_t)
 
 #define DURATION_WHOLE_FRACTION_DESIGNATOR(Name, name, d)                 \
   template <typename Char>                                                \
   int32_t ScanDurationWhole##Name##FractionDesignator(                    \
       base::Vector<Char> str, int32_t s, ParsedISO8601Duration* r) {      \
     int32_t cur = s;                                                      \
-    int64_t whole = ParsedISO8601Duration::kEmpty;                        \
+    double whole = ParsedISO8601Duration::kEmpty;                         \
     cur += ScanDurationWhole##Name(str, cur, &whole);                     \
     if (cur == s) return 0;                                               \
-    int64_t fraction = ParsedISO8601Duration::kEmpty;                     \
+    int32_t fraction = ParsedISO8601Duration::kEmpty;                     \
     int32_t len = ScanDuration##Name##Fraction(str, cur, &fraction);      \
     cur += len;                                                           \
     if (str.length() < (cur + 1) || AsciiAlphaToLower(str[cur++]) != (d)) \
@@ -1570,7 +1551,7 @@ int32_t ScanDurationTime(base::Vector<Char> str, int32_t s,
   int32_t ScanDuration##Name##Designator(base::Vector<Char> str, int32_t s, \
                                          ParsedISO8601Duration* r) {        \
     int32_t cur = s;                                                        \
-    int64_t name;                                                           \
+    double name;                                                            \
     if ((cur += ScanDuration##Name(str, cur, &name)) == s) return 0;        \
     if (str.length() < (cur + 1) || AsciiAlphaToLower(str[cur++]) != (d)) { \
       return 0;                                                             \

src/temporal/temporal-parser.h

@@ -95,20 +95,20 @@ struct ParsedISO8601Result {
  * field is "undefined" after parsing for all fields except sign.
  */
 struct ParsedISO8601Duration {
-  int64_t sign;              // Sign production
-  int64_t years;             // DurationYears production
-  int64_t months;            // DurationMonths production
-  int64_t weeks;             // DurationWeeks production
-  int64_t days;              // DurationDays production
-  int64_t whole_hours;       // DurationWholeHours production
-  int64_t hours_fraction;    // DurationHoursFraction, in unit of 1e-9 hours
-  int64_t whole_minutes;     // DurationWholeMinutes production
-  int64_t minutes_fraction;  // DurationMinuteFraction, in unit of 1e-9 minutes
-  int64_t whole_seconds;     // DurationWholeSeconds production
-  int64_t seconds_fraction;  // DurationSecondFraction, in unit of nanosecond (
+  double sign;               // Sign production
+  double years;              // DurationYears production
+  double months;             // DurationMonths production
+  double weeks;              // DurationWeeks production
+  double days;               // DurationDays production
+  double whole_hours;        // DurationWholeHours production
+  double whole_minutes;      // DurationWholeMinutes production
+  double whole_seconds;      // DurationWholeSeconds production
+  int32_t hours_fraction;    // DurationHoursFraction, in unit of 1e-9 hours
+  int32_t minutes_fraction;  // DurationMinuteFraction, in unit of 1e-9 minutes
+  int32_t seconds_fraction;  // DurationSecondFraction, in unit of nanosecond (
                              // 1e-9 seconds).
 
-  static constexpr int64_t kEmpty = -1;
+  static constexpr int32_t kEmpty = -1;
   ParsedISO8601Duration()
       : sign(1),
         years(kEmpty),
@@ -116,10 +116,10 @@ struct ParsedISO8601Duration {
         weeks(kEmpty),
         days(kEmpty),
         whole_hours(kEmpty),
-        hours_fraction(kEmpty),
         whole_minutes(kEmpty),
-        minutes_fraction(kEmpty),
         whole_seconds(kEmpty),
+        hours_fraction(kEmpty),
+        minutes_fraction(kEmpty),
         seconds_fraction(kEmpty) {}
 };
 

test/test262/test262.status

@@ -586,8 +586,6 @@
   'built-ins/Temporal/Duration/prototype/round/total-duration-nanoseconds-too-large-with-zoned-datetime': [PASS, FAIL],
   'built-ins/Temporal/Duration/prototype/total/relativeto-zoneddatetime-with-fractional-days-different-sign': [FAIL],
   'built-ins/Temporal/Duration/prototype/total/relativeto-zoneddatetime-with-fractional-days': [FAIL],
-  'built-ins/Temporal/PlainTime/prototype/add/argument-string-duration-too-large': [FAIL],
-  'built-ins/Temporal/PlainTime/prototype/subtract/argument-string-duration-too-large': [FAIL],
   'intl402/Temporal/TimeZone/prototype/getNextTransition/subtract-second-and-nanosecond-from-last-transition': [FAIL],
   'intl402/Temporal/TimeZone/prototype/getPreviousTransition/nanoseconds-subtracted-or-added-at-dst-transition': [FAIL],