Bug 172637 – REGRESSION(r217458): 55 javascript tests are failing after r217458 on Linux and Windows

• Component: JavaScriptCore
• Status: RESOLVED DUPLICATE

[Carlos Garcia Campos]

I guess localtime behaves differently in Mac OS and Linux, maybe we need to bring back the deleted code for linux only?

[Carlos Garcia Campos]

** 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]

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]

(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]

*** Bug 172641 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

Apple Windows port is affected too:
https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/807

[Zan Dobersek]

In the original patch, some tests that were affected by this were skipped. We could do the same for these tests.

[Yusuke Suzuki]

(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]

(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]

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]

(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]

(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]

I reverted r217458 in http://trac.webkit.org/changeset/217499

[Ryan Haddad]

*** This bug has been marked as a duplicate of bug 172592 ***