RESOLVED DUPLICATE of bug 172592 172637
REGRESSION(r217458): 55 javascript tests are failing after r217458 on Linux and Windows 
https://bugs.webkit.org/show_bug.cgi?id=172637
Summary REGRESSION(r217458): 55 javascript tests are failing after r217458 on Linux a...
Carlos Garcia Campos
Reported 2017-05-26 02:04:10 PDT
I guess localtime behaves differently in Mac OS and Linux, maybe we need to bring back the deleted code for linux only?
Attachments
Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2017-05-26 02:04:38 PDT
** The following JSC stress test failures have been introduced: ChakraCore.yaml/ChakraCore/test/Date/DateCtr.js.default jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/date-constructor.js.layout-no-llint jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/date-timeClip-large-values.js.layout-no-llint mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla-baseline mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla-ftl-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla-llint mozilla-tests.yaml/ecma/Date/15.9.5.16.js.mozilla-no-ftl mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla-baseline mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla-ftl-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla-llint mozilla-tests.yaml/ecma/Date/15.9.5.18.js.mozilla-no-ftl mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla-baseline mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla-ftl-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla-llint mozilla-tests.yaml/ecma/Date/15.9.5.22-1.js.mozilla-no-ftl mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla-baseline mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla-ftl-eager-no-cjit-validate-phases mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla-llint mozilla-tests.yaml/ecma/Date/15.9.5.22-2.js.mozilla-no-ftl stress/date-relaxed.js.default stress/date-relaxed.js.dfg-eager stress/date-relaxed.js.dfg-eager-no-cjit-validate stress/date-relaxed.js.dfg-maximal-flush-validate-no-cjit stress/date-relaxed.js.ftl-eager stress/date-relaxed.js.ftl-eager-no-cjit stress/date-relaxed.js.ftl-eager-no-cjit-b3o1 stress/date-relaxed.js.ftl-no-cjit-b3o1 stress/date-relaxed.js.ftl-no-cjit-no-inline-validate stress/date-relaxed.js.ftl-no-cjit-no-put-stack-validate stress/date-relaxed.js.ftl-no-cjit-small-pool stress/date-relaxed.js.ftl-no-cjit-validate-sampling-profiler stress/date-relaxed.js.no-cjit-collect-continuously stress/date-relaxed.js.no-cjit-validate-phases stress/date-relaxed.js.no-ftl stress/date-relaxed.js.no-llint Results for JSC stress tests: 55 failures found.
Yusuke Suzuki
Comment 2 2017-05-26 03:35:41 PDT
It seems something goes wrong. // Set TZ=America/Los_Angeles let date = new Date("May 8 -1"); print(date); "Fri May 07 -001 23:59:02 GMT-0752 (LMT)" Maybe, Linux returns wrong value if the year is not within expected ones. I dumped some of is_dst and tm_gmtoff * msPerSecond. If dst is set, 1 -25200000.000000 If dst is not set, 0 -28800000.000000 That's correct. In the above (-1 year) case, 0 -28378000.000000 That's clearly wrong. I think we should recover the previous code for non-darwin environment.
Yusuke Suzuki
Comment 3 2017-05-26 03:53:45 PDT
(In reply to Yusuke Suzuki from comment #2) > It seems something goes wrong. > > // Set TZ=America/Los_Angeles > let date = new Date("May 8 -1"); > print(date); > > "Fri May 07 -001 23:59:02 GMT-0752 (LMT)" > > Maybe, Linux returns wrong value if the year is not within expected ones. > > I dumped some of is_dst and tm_gmtoff * msPerSecond. > > If dst is set, > 1 -25200000.000000 > > If dst is not set, > 0 -28800000.000000 > > That's correct. In the above (-1 year) case, > > 0 -28378000.000000 > > That's clearly wrong. > I think we should recover the previous code for non-darwin environment. Ah, No! That's more correct. It is Local Mean Time! Before timezone is introduced, this mechanism is used. And it is replaced with timezone after 1883 Nov 18 12:07:02. https://github.com/tzinfo/tzinfo-data/blob/master/data/northamerica#L487 But... who wants it?
Csaba Osztrogonác
Comment 4 2017-05-26 04:11:10 PDT
*** Bug 172641 has been marked as a duplicate of this bug. ***
Csaba Osztrogonác
Comment 5 2017-05-26 04:15:32 PDT
Apple Windows port is affected too: https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/807
Zan Dobersek
Comment 6 2017-05-26 04:29:53 PDT
In the original patch, some tests that were affected by this were skipped. We could do the same for these tests.
Yusuke Suzuki
Comment 7 2017-05-26 06:35:35 PDT
(In reply to Zan Dobersek from comment #6) > In the original patch, some tests that were affected by this were skipped. > We could do the same for these tests. I'm a bit worried about the current result. If that thing just changes DST switch, it sounds good. However, the current implementation poses a bit storange Date for old dates. (Like "Fri May 07 -001 23:59:02 GMT-0752 (LMT)"). What do you think of reverting the patch for non Darwin environment?
Zan Dobersek
Comment 8 2017-05-26 06:59:08 PDT
(In reply to Yusuke Suzuki from comment #7) > (In reply to Zan Dobersek from comment #6) > > In the original patch, some tests that were affected by this were skipped. > > We could do the same for these tests. > > I'm a bit worried about the current result. If that thing just changes DST > switch, it sounds good. > However, the current implementation poses a bit storange Date for old dates. > (Like "Fri May 07 -001 23:59:02 GMT-0752 (LMT)"). > > What do you think of reverting the patch for non Darwin environment? Do we remain compliant with the relevant specs? We should maybe wait for Keith to weigh in. One additional observation -- in run-javascriptcore-tests, we set the TZ env to the PST timezone. But at least for the failing tests under mozilla-tests.yaml/ecma/Date/, if that TZ env is set to UTC or GMT, the test passes because LMT doesn't kick in for the date that is far back in the past.
Keith Miller
Comment 9 2017-05-26 07:13:13 PDT
Hmm, it looks like FF and Chrome turn "new Date("May 8 -1")" into NaN. Perhaps we should have the same result here? As far as LMT it doesn't look like the spec mentions it: https://tc39.github.io/ecma262/#sec-local-time-zone-adjustment. So it seems like we should avoid using it.
Yusuke Suzuki
Comment 10 2017-05-26 07:25:32 PDT
(In reply to Keith Miller from comment #9) > Hmm, it looks like FF and Chrome turn "new Date("May 8 -1")" into NaN. > Perhaps we should have the same result here? > > As far as LMT it doesn't look like the spec mentions it: > https://tc39.github.io/ecma262/#sec-local-time-zone-adjustment. So it seems > like we should avoid using it. LMT can be easily seen if you use a bit old date (but it's not so old, year < 1883). For example, `new Date("May 8 1880")` will show "Fri May 07 1880 23:59:02 GMT-0752 (LMT)".
Yusuke Suzuki
Comment 11 2017-05-26 07:30:28 PDT
(In reply to Yusuke Suzuki from comment #10) > (In reply to Keith Miller from comment #9) > > Hmm, it looks like FF and Chrome turn "new Date("May 8 -1")" into NaN. > > Perhaps we should have the same result here? > > > > As far as LMT it doesn't look like the spec mentions it: > > https://tc39.github.io/ecma262/#sec-local-time-zone-adjustment. So it seems > > like we should avoid using it. > > LMT can be easily seen if you use a bit old date (but it's not so old, year > < 1883). > For example, `new Date("May 8 1880")` will show "Fri May 07 1880 23:59:02 > GMT-0752 (LMT)". Another problem is that the end of LMT depends on each TZ. PDT says "1883 Nov 18 12:07:02". Europe/Amsterdam　is +00:19:32 (0:19:32) - LMT 1835 Asia/Dubai is +03:41:12 (3:41:12) - LMT 1920
Ryan Haddad
Comment 12 2017-05-26 12:29:24 PDT
I reverted r217458 in http://trac.webkit.org/changeset/217499
Ryan Haddad
Comment 13 2017-05-26 12:29:59 PDT
*** This bug has been marked as a duplicate of bug 172592 ***
Note You need to log in before you can comment on or make changes to this bug.