WTF:gregorianDateTimeToMS() is in JavaScriptCore/wtf/DateMath.cpp. It returns negative values for huge years. For example, it returns -1.85543E+17 for GregorianDateTime with .year=11798100. This issue doesn't cause any real bugs for now because JSC Date instances are created with WTF::timeClip(), which maps a huge value to NaN. However this issue might cause a real bug in the future.
Oh, it's JSC::, not WTF::.
Created attachment 44511 [details] Proposed patch
style-queue ran check-webkit-style on attachment 44511 [details] without any errors.
Comment on attachment 44511 [details] Proposed patch This patch does not include a test case. Why not? > -static int dateToDayInYear(int year, int month, int day) > +static double dateToDayInYear(int year, int month, int day) This seems wrong. A "day in year" value can never be out of range for an int: There are a maximum of 366 days in a year. So I don't see how the change in return type is helpful unless there is a bug inside the dateToDayInYear function. You'll have to explain why changing to double is helpful. > + // 8.64E15 is a value near to the maximum integral number that > + // double can represent without precision loss. > if (fabs(t) > 8.64E15) That comment seems to me like it's probably right right, but the best way to document such things is typically to use a named constant. Also, I believe this number comes straight from the ECMAScript specification, and should not be changed. The comment makes it seem like you could come up with a value even closer to the maximum number, and improve the code, but I believe that is not true and would cause us to differ from the specification if we changed it.
Created attachment 44611 [details] Proposed patch (rev.2) (In reply to comment #4) > (From update of attachment 44511 [details]) > This patch does not include a test case. Why not? I added a test to the updated patch. If 'yearday' in dateToDayInYear() is int, the test causes an assertion failure. > > -static int dateToDayInYear(int year, int month, int day) > > +static double dateToDayInYear(int year, int month, int day) > > This seems wrong. A "day in year" value can never be out of range for an int: > There are a maximum of 366 days in a year. So I don't see how the change in > return type is helpful unless there is a bug inside the dateToDayInYear > function. You'll have to explain why changing to double is helpful. This function returns the number of days from 1970-01-01 to the specified date. You know, (year - 1970) * 365 can be larger than INT_MAX. The function name is very wrong. I renamed it in the updated patch. > > + // 8.64E15 is a value near to the maximum integral number that > > + // double can represent without precision loss. > > if (fabs(t) > 8.64E15) > > That comment seems to me like it's probably right right, but the best way to > document such things is typically to use a named constant. Also, I believe this > number comes straight from the ECMAScript specification, and should not be > changed. The comment makes it seem like you could come up with a value even > closer to the maximum number, and improve the code, but I believe that is not > true and would cause us to differ from the specification if we changed it. ok, I introduced a named constant.
style-queue ran check-webkit-style on attachment 44611 [details] without any errors.
Please test SunSpider with and without your patch, and post results.
Created attachment 44652 [details] sunspider-compare-results output > Please test SunSpider with and without your patch, and post results. Here it is. The baseline is r51974 without this patch, and the competitor is r51974 + this patch. It seems there are no significant differences.
Comment on attachment 44611 [details] Proposed patch (rev.2) Rejecting patch 44611 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11764 test cases. fast/js/date-daysfrom1970-overflow.html -> failed Exiting early after 1 failures. 7083 tests run. 113.75s total testing time 7082 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 2 test cases (<1%) had stderr output
> fast/js/date-daysfrom1970-overflow.html -> failed The test depends on the local timezone. I'll fix it.
Created attachment 44655 [details] Proposed patch (rev.3) * Fix a timezone-dependency of the test
style-queue ran check-webkit-style on attachment 44655 [details] without any errors.
Comment on attachment 44655 [details] Proposed patch (rev.3) Clearing flags on attachment: 44655 Committed r51986: <http://trac.webkit.org/changeset/51986>
All reviewed patches have been landed. Closing bug.