ICU-13745 fix undefined behavior: GregorianCalendar::setGregorianChange()

- Julian days outside of INT32_MIN..INT32_MAX are normalized
- Add a test case

diff --git a/icu4c/source/i18n/gregocal.cpp b/icu4c/source/i18n/gregocal.cpp
index ee15b5291a934740999b8015010db3a5241af418..6b15171c12b9ac4e2911f2749afaaaefe66d95c2 100644 (file)
--- a/icu4c/source/i18n/gregocal.cpp
+++ b/icu4c/source/i18n/gregocal.cpp
@@ -324,26 +324,26 @@ GregorianCalendar::setGregorianChange(UDate date, UErrorCode& status)
     if (U_FAILURE(status)) 
         return;
 
-    fGregorianCutover = date;
-
     // Precompute two internal variables which we use to do the actual
     // cutover computations.  These are the normalized cutover, which is the
     // midnight at or before the cutover, and the cutover year.  The
     // normalized cutover is in pure date milliseconds; it contains no time
     // of day or timezone component, and it used to compare against other
     // pure date values.
-    int32_t cutoverDay = (int32_t)ClockMath::floorDivide(fGregorianCutover, (double)kOneDay);
-    fNormalizedGregorianCutover = cutoverDay * kOneDay;
-
-    // Handle the rare case of numeric overflow.  If the user specifies a
-    // change of UDate(Long.MIN_VALUE), in order to get a pure Gregorian
-    // calendar, then the epoch day is -106751991168, which when multiplied
-    // by ONE_DAY gives 9223372036794351616 -- the negative value is too
-    // large for 64 bits, and overflows into a positive value.  We correct
-    // this by using the next day, which for all intents is semantically
-    // equivalent.
-    if (cutoverDay < 0 && fNormalizedGregorianCutover > 0) {
-        fNormalizedGregorianCutover = (cutoverDay + 1) * kOneDay;
+    double cutoverDay = ClockMath::floorDivide(date, (double)kOneDay);
+
+    // Handle the rare case of numeric overflow where the user specifies a time
+    // outside of INT32_MIN .. INT32_MAX number of days.
+    
+    if (cutoverDay <= INT32_MIN) {
+        cutoverDay = INT32_MIN;
+        fGregorianCutover = fNormalizedGregorianCutover = cutoverDay * kOneDay;
+    } else if (cutoverDay >= INT32_MAX) {
+        cutoverDay = INT32_MAX;
+        fGregorianCutover = fNormalizedGregorianCutover = cutoverDay * kOneDay;
+    } else {
+        fNormalizedGregorianCutover = cutoverDay * kOneDay;
+        fGregorianCutover = date;
     }
 
     // Normalize the year so BC values are represented as 0 and negative
@@ -360,7 +360,7 @@ GregorianCalendar::setGregorianChange(UDate date, UErrorCode& status)
     fGregorianCutoverYear = cal->get(UCAL_YEAR, status);
     if (cal->get(UCAL_ERA, status) == BC) 
         fGregorianCutoverYear = 1 - fGregorianCutoverYear;
-    fCutoverJulianDay = cutoverDay;
+    fCutoverJulianDay = (int32_t)cutoverDay;
     delete cal;
 }
 

diff --git a/icu4c/source/test/intltest/calregts.cpp b/icu4c/source/test/intltest/calregts.cpp
index 8806d04be18f23fb2873d1d2e91127dc1e60bd94..744ca69baf83c42084cb84207d174cf8fdf246df 100644 (file)
--- a/icu4c/source/test/intltest/calregts.cpp
+++ b/icu4c/source/test/intltest/calregts.cpp
@@ -96,6 +96,7 @@ CalendarRegressionTest::runIndexedTest( int32_t index, UBool exec, const char* &
         CASE(52,TestPersianCalOverflow);
         CASE(53,TestIslamicCalOverflow);
         CASE(54,TestWeekOfYear13548);
+        CASE(55,Test13745);
     default: name = ""; break;
     }
 }
@@ -1536,6 +1537,41 @@ void CalendarRegressionTest::test4141665()
     delete cal2;
 }
 
+const UDate MILLIS_IN_DAY = 86400000.0;
+/**
+ * ICU-13745
+ * GregorianCalendar::setGregorianChange() overflow
+ */
+void CalendarRegressionTest::Test13745()
+{
+    UErrorCode status = U_ZERO_ERROR;
+    GregorianCalendar *cal = new GregorianCalendar(status);
+    if(U_FAILURE(status)) {
+      dataerrln("Error creating calendar %s", u_errorName(status));
+      delete cal;
+      return;
+    }
+
+    // this line would overflow before fix 13745
+    cal->setGregorianChange(((double)INT32_MAX+1.0) * MILLIS_IN_DAY, status);
+    if(U_FAILURE(status)) {
+        errln("%s:%d Failure setting INT32_MAX+1 change on calendar: %s\n", __FILE__, __LINE__, u_errorName(status));
+        return;
+    }
+    assertEquals("getGregorianChange()", (double)INT32_MAX * MILLIS_IN_DAY, cal->getGregorianChange());
+
+    // test underflow
+    cal->setGregorianChange(((double)INT32_MIN-1.0) * MILLIS_IN_DAY, status);
+    if(U_FAILURE(status)) {
+        errln("%s:%d Failure setting INT32_MAX-1 change on calendar: %s\n", __FILE__, __LINE__, u_errorName(status));
+        return;
+    }
+    assertEquals("getGregorianChange()", (double)INT32_MIN * MILLIS_IN_DAY, cal->getGregorianChange());
+
+    delete cal;
+}
+
+
 /**
  * @bug 4142933
  * Bug states that ArrayIndexOutOfBoundsException is thrown by GregorianCalendar::roll()

diff --git a/icu4c/source/test/intltest/calregts.h b/icu4c/source/test/intltest/calregts.h
index b4166a0c61d3f72a09a0c3afdf9cf7def75be523..6fc4bdda16ec2000712fce34ee5610ba005d7792 100644 (file)
--- a/icu4c/source/test/intltest/calregts.h
+++ b/icu4c/source/test/intltest/calregts.h
@@ -82,6 +82,8 @@ public:
     void TestIslamicCalOverflow(void);
     void TestWeekOfYear13548(void);
 
+    void Test13745(void);
+
     void printdate(GregorianCalendar *cal, const char *string);
     void dowTest(UBool lenient) ;