Bug 5514 – Date conversion to local time gets the DST flag wrong for some dates

• Status: VERIFIED
• Resolution: FIXED
• Component: JavaScriptCore

For dates before 1970-01-01 00:00 UTC, conversion to local time takes the DST flag from the same day in 
a different year. This leads to incorrect results in many cases, since DST cutoff dates are not the same 
every year.

The test case demonstrates this for several time zones: now that bug 4142 is fixed, "new Date(y,m,d)" 
works as expected, but the conversion back to local time is wrong due to using DST status from the same 
date in 2002.

Since the fix for bug 4142 involves passing negative and large positive values to localtime_r(), I'm not sure 
why the same isn't done in DateProtoFuncImp::callAsFunction().

Created attachment 4484 [details]
Testcase

[Geoffrey Garen]

Mitz mentioned on IRC that eliminating the yearOffset hack fixes this bug. But that would make the code 
slightly less portable.

Created attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary

Comment on attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary

Haven't tested it on any platform other than PPC OS X.

[Darin Adler]

Comment on attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary

This line of code:

+const bool time_tIsSigned = isTime_tSigned();

will result in code run at load time. We don't allow that in JavaScriptCore and
WebCore. We'll need an alternate approach that does the same thing without a
global that requires executing code at library load time.

Created attachment 4510 [details]
Apply the realYearOffset hack only when absolutely necessary

Initialize time_t limits only the first time they're actually needed.

[Geoffrey Garen]

Here's an idea:

static bool isTime_tSigned()
{
  time_t minusOne == -1;
  return minusOne < 0;
}

[Geoffrey Garen]

(In reply to comment #7)

s/==/=/

[Darin Adler]

Comment on attachment 4510 [details]
Apply the realYearOffset hack only when absolutely necessary

Looks good. r=me

(In reply to comment #7)
> Here's an idea:
>   time_t minusOne = -1;

Clever! But the above will give a compiler warning if time_t is unsigned. This should work:

    time_t minusOne = (time_t)(-1);

Created attachment 4517 [details]
Better patch

Comment on attachment 4517 [details]
Better patch

Did as Geoffrey suggested