Problem with WORK_AROUND_QTBUG_53071
Happy leap second. In 2016i Pacific/Tongatapu changed to: (1) use numeric time zone abbreviations, and (2) observe DST from the first Sunday in November through the third Sunday in January. That is, its POSIX future rule became "<+13>-13<+14>,M11.1.0,M1.3.0/3". (1) results, under WORK_AROUND_QTBUG_53071, in zic adding a "y2038_boundary - 1" (i.e., 2038-01-19 03:14:07 +00:00) no-op transition to the end of the generated transitions. So, the Pacific/Tongatapu zoneinfo file looks like ... : 2115810000 gmtoff=46800 is_dst=F abbr="+13" 2140606800 gmtoff=50400 is_dst=T abbr="+14" 2147483647 gmtoff=50400 is_dst=T abbr="+14" When loading Pacific/Tongatapu at runtime, (2) results in tzloadbody() generating additional transitions ... : 2140606800 gmtoff=50400 is_dst=T abbr="+14" 2147259600 gmtoff=46800 is_dst=F abbr="+13" 2172661200 gmtoff=50400 is_dst=T abbr="+14" 2178709200 gmtoff=46800 is_dst=F abbr="+13" : When those generated transitions are patched onto the end of the zic transitions, we end up with ... 2115810000 gmtoff=46800 is_dst=F abbr="+13" 2140606800 gmtoff=50400 is_dst=T abbr="+14" 2147483647 gmtoff=50400 is_dst=T abbr="+14" 2172661200 gmtoff=50400 is_dst=T abbr="+14" 2178709200 gmtoff=46800 is_dst=F abbr="+13" This is wrong. The presence of the 2147483647 transition caused us to drop the 2147259600 transition. That is, we lost the shift out of DST at 2038-01-17 02:00:00 +13:00 (2038-01-16 13:00:00 +00:00). One solution would be to trim trailing, no-op transitions from the end of the zic transitions before adding the rule-generated ones. The reference implementation doesn't need them. Bradley
Bradley White wrote:
One solution would be to trim trailing, no-op transitions from the end of the zic transitions before adding the rule-generated ones. The reference implementation doesn't need them.
Thanks for catching that bug. The attached patch takes your suggestion for localtime.c. It also attempts to fix zic.c to not generate the incorrect transition in Pacific/Tongatapu in January 2038. Either part of the patch should suffice to fix the bug.
On Thu, Jan 5, 2017 at 2:37 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
Bradley White wrote:
One solution would be to trim trailing, no-op transitions from the end of
the zic transitions before adding the rule-generated ones. The reference implementation doesn't need them.
Thanks for catching that bug. The attached patch takes your suggestion for localtime.c. It also attempts to fix zic.c to not generate the incorrect transition in Pacific/Tongatapu in January 2038. Either part of the patch should suffice to fix the bug.
Thanks. I can verify that: (1) the zic.c change only affects Pacific/Tongatapu and Pacific/Fiji, and (2) the zic.c and/or the localtime.c changes fix the problem. Regarding the localtime.c change, perhaps something better than ignoring a transition that was "almost surely generated" because of WORK_AROUND_QTBUG_53071, is to just ignore any trailing, no-op transitions. That is, ... + /* Ignore any trailing, no-op transitions generated + * by zic as they don't help here and can run afoul + * of bugs in zic 2016j or earlier. */ + while (1 < sp->timecnt && + sp->types[sp->timecnt - 1] == + sp->types[sp->timecnt - 2]) + sp->timecnt--;
On 01/05/2017 10:05 PM, Bradley White wrote:
Regarding the localtime.c change, perhaps something better than ignoring a transition that was "almost surely generated" because of WORK_AROUND_QTBUG_53071, is to just ignore any trailing, no-op transitions. That is, ...
+ /* Ignore any trailing, no-op transitions generated + * by zic as they don't help here and can run afoul + * of bugs in zic 2016j or earlier. */ + while (1 < sp->timecnt && + sp->types[sp->timecnt - 1] == + sp->types[sp->timecnt - 2]) + sp->timecnt--;
I tried something like that but it that worked only on 32-bit clients because the transitions were still in the 64-bit data and were not at the end. My memory could be wrong (I didn't save all my work). Plus, perhaps it could be fixed up somehow. I agree that if someone could get something like the above to work in both 32- and 64-bit clients, with both the unfixed and the fixed zic, it'd be a better fix.
On Fri, Jan 6, 2017 at 1:25 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
On 01/05/2017 10:05 PM, Bradley White wrote:
Regarding the localtime.c change, perhaps something better than ignoring a transition that was "almost surely generated" because of WORK_AROUND_QTBUG_53071, is to just ignore any trailing, no-op transitions. That is, ...
+ /* Ignore any trailing, no-op transitions generated + * by zic as they don't help here and can run afoul + * of bugs in zic 2016j or earlier. */ + while (1 < sp->timecnt && + sp->types[sp->timecnt - 1] == + sp->types[sp->timecnt - 2]) + sp->timecnt--;
I tried something like that but it that worked only on 32-bit clients because the transitions were still in the 64-bit data and were not at the end. My memory could be wrong (I didn't save all my work). Plus, perhaps it could be fixed up somehow. I agree that if someone could get something like the above to work in both 32- and 64-bit clients, with both the unfixed and the fixed zic, it'd be a better fix.
Here is what I see for trailing 32-bit and 64-bit data in Pacific/Tongatapu using both the unfixed and fixed zic. 32-bit data 64-bit data Unfixed zic: 2115810000 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2115810000 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2140606800 [type=5] gmtoff=50400 is_dst=T abbr="+14" 2140606800 [type=5] gmtoff=50400 is_dst=T abbr="+14" 2147483647 [type=5] gmtoff=50400 is_dst=T abbr="+14" 2147483647 [type=5] gmtoff=50400 is_dst=T abbr="+14" Fixed zic: 2115810000 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2115810000 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2140606800 [type=5] gmtoff=50400 is_dst=T abbr="+14" 2140606800 [type=5] gmtoff=50400 is_dst=T abbr="+14" 2147259600 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2147259600 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2147483647 [type=2] gmtoff=46800 is_dst=F abbr="+13" 2147483647 [type=2] gmtoff=46800 is_dst=F abbr="+13" All of that looks good. The extra transition is at the end in each case, so the proposed "trim trailing no-op transitions" patch to localtime.c would always do the trick ... correcting the dropped-transition (2147259600) problem for the unfixed zic, and having no effect for the fixed zic. I'm not sure what issue you may have seen with it. [One thing I see that has changed in the fixed zic is that the last two "real" transitions are no longer necessarily in the same calendar year. That's not a problem per se, but my future-extension code assumes it. :-( I'll have to fix that.] Bradley
Bradley White wrote:
Here is what I see for trailing 32-bit and 64-bit data in Pacific/Tongatapu using both the unfixed and fixed zic.
Thanks, quite possibly my memory is faulty. Can you propose a "git format-patch" style patch to localtime.c that omits the unwanted fix and has the fix you're proposing?
On Sat, Jan 7, 2017 at 10:19 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
Bradley White wrote:
Here is what I see for trailing 32-bit and 64-bit data in Pacific/Tongatapu using both the unfixed and fixed zic.
Thanks, quite possibly my memory is faulty. Can you propose a "git format-patch" style patch to localtime.c that omits the unwanted fix and has the fix you're proposing?
Attached.
participants (2)
-
Bradley White -
Paul Eggert