[03/59] Fix mishandling of default DST rule

Message ID 20250105055750.1668721-4-eggert@cs.ucla.edu (mailing list archive)
State New
Headers
Series time: sync mktime from Gnulib |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Paul Eggert Jan. 5, 2025, 5:56 a.m. UTC
  For timezone settings like TZ="AST4ADT" where POSIX says the
default is implementation-defined, time/tzfile.c has long had
vestiges of an old attempt to be upward compatible with UNIX
System V.  This code has not worked for decades and evidently is
not being used, so remove it and use the simpler fallback code
already present in time/tzset.c, which uses US DST.  Also, add a
few test cases to demonstrate some bugs in the removed code.
* time/tst-posixtz.c: Include stdckdint.h.
(tests): WHEN is now intmax_t, not time_t, to support
skipping tests after 2038 on 32-bit time_t.  All uses changed.
Make it const while we're at it.  Add several tests for AST4ADT.
* time/tzfile.c (rule_dstoff): Remove.  All uses removed.
(__tzfile_read): Remove args EXTRA, EXTRAP.  All uses changed.
(__tzfile_default): Remove.  All uses removed.
(__tzfile_compute): Remove no-longer-needed code dealing
with __tzfile_default.
* timezone/Makefile (build-testdata):
Add '-p America/New_York' option if compiling 'northamerica',
so that we can test non-use of posixrules.
* timezone/tst-timezone.c: Include stdbool.h, stdckdint.h.
(failed): Now bool, not int.  All uses changed.
(do_test): Test AST4ADT too.
---
 NEWS                    |   7 +++
 include/time.h          |   6 +--
 manual/time.texi        |   2 +-
 time/tst-posixtz.c      |  56 ++++++++++++++++++--
 time/tzfile.c           | 112 ++--------------------------------------
 time/tzset.c            |  19 +------
 timezone/Makefile       |   3 +-
 timezone/tst-timezone.c |  64 ++++++++++++++++++++---
 8 files changed, 124 insertions(+), 145 deletions(-)
  

Patch

diff --git a/NEWS b/NEWS
index 00c569fe85..3e6227a448 100644
--- a/NEWS
+++ b/NEWS
@@ -84,6 +84,13 @@  Deprecated and removed features, and other changes affecting compatibility:
   explicitly because of the executable bit in GNU_STACK, and the stack is
   not already executable.  Instead, loading such objects will fail.
 
+* The default daylight saving rules for old Unix System V style TZ
+  strings like TZ="AST4ADT" are now those of current US DST.  Although
+  the rules were supposed to be those of /usr/share/zoneinfo/posixrules,
+  this feature, which has been disabled by default and marked obsolete
+  upstream in tzcode, did not work in either glibc or tzcode, and in
+  practice posixrules invariably specified current US DST anyway.
+
 Changes to build and runtime requirements:
 
 * On recent Linux kernels with vDSO getrandom support, getrandom does not
diff --git a/include/time.h b/include/time.h
index f599eeed4e..52bb22abd9 100644
--- a/include/time.h
+++ b/include/time.h
@@ -54,14 +54,10 @@  extern char *__tzstring (const char *string) attribute_hidden;
 
 extern int __use_tzfile attribute_hidden;
 
-extern void __tzfile_read (const char *file, size_t extra,
-			   char **extrap) attribute_hidden;
+extern void __tzfile_read (const char *file) attribute_hidden;
 extern void __tzfile_compute (__time64_t timer, int use_localtime,
 			      long int *leap_correct, int *leap_hit,
 			      struct tm *tp) attribute_hidden;
-extern void __tzfile_default (const char *std, const char *dst,
-			      int stdoff, int dstoff)
-  attribute_hidden;
 extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
   __THROW attribute_hidden;
diff --git a/manual/time.texi b/manual/time.texi
index 90bc9a2566..899128ca7f 100644
--- a/manual/time.texi
+++ b/manual/time.texi
@@ -2724,7 +2724,7 @@  and offset for the corresponding daylight saving time zone; if the
 
 The remainder of the proleptic format, which starts with the first comma,
 describes when daylight saving time is in effect.  This remainder is
-optional and if omitted, @theglibc{} defaults to the daylight saving
+optional and if omitted, @theglibc{} defaults to US daylight saving rules.
 rules that would be used if @env{TZ} had the value @t{"posixrules"}.
 However, other POSIX implementations default to different daylight
 saving rules, so portable @env{TZ} settings should not omit the
diff --git a/time/tst-posixtz.c b/time/tst-posixtz.c
index 9bec7ae4bb..936c5b8fc6 100644
--- a/time/tst-posixtz.c
+++ b/time/tst-posixtz.c
@@ -1,3 +1,4 @@ 
+#include <stdckdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -6,10 +7,10 @@ 
 
 struct
 {
-  time_t when;
+  intmax_t when;
   const char *tz;
   const char *result;
-} tests[] =
+} const tests[] =
 {
   { 909312849L, "AEST-10AEDST-11,M10.5.0,M3.5.0",
     "1998/10/25 21:54:09 dst=1 zone=AEDST" },
@@ -27,6 +28,43 @@  struct
     "1999/04/23 06:54:09 dst=1 zone=EDT" },
   { 919973892L, "EST+5EDT,M4.1.0/2,M10.5.0/2",
     "1999/02/25 15:18:12 dst=0 zone=EST" },
+
+  /* Test Atlantic Standard / Atlantic Daylight transitions in 2024 and 2038,
+     with explicit DST rule.  */
+  { 1710050399, "AST4ADT,M3.2.0,M11.1.0",
+    "2024/03/10 01:59:59 dst=0 zone=AST" },
+  { 1710050400, "AST4ADT,M3.2.0/2,M11.1.0/2",
+    "2024/03/10 03:00:00 dst=1 zone=ADT" },
+  { 1730609999, "AST4ADT,M3.2.0/02:00,M11.1.0/02:00",
+    "2024/11/03 01:59:59 dst=1 zone=ADT" },
+  { 1730610000, "AST4ADT,M3.2.0/02:00:00,M11.1.0/02:00:00",
+    "2024/11/03 01:00:00 dst=0 zone=AST" },
+  { 2152159199, "AST4ADT,M3.2.0,M11.1.0",
+    "2038/03/14 01:59:59 dst=0 zone=AST" },
+  { 2152159200, "AST4ADT,M3.2.0/2,M11.1.0/2",
+    "2038/03/14 03:00:00 dst=1 zone=ADT" },
+  { 2172718799, "AST4ADT,M3.2.0/02:00,M11.1.0/02:00",
+    "2038/11/07 01:59:59 dst=1 zone=ADT" },
+  { 2172718800, "AST4ADT,M3.2.0/02:00:00,M11.1.0/02:00:00",
+    "2038/11/07 01:00:00 dst=0 zone=AST" },
+
+  /* Likewise, but with default DST rule.  */
+  { 1710050399, "AST4ADT",
+    "2024/03/10 01:59:59 dst=0 zone=AST" },
+  { 1710050400, "AST4ADT",
+    "2024/03/10 03:00:00 dst=1 zone=ADT" },
+  { 1730609999, "AST4ADT",
+    "2024/11/03 01:59:59 dst=1 zone=ADT" },
+  { 1730610000, "AST4ADT",
+    "2024/11/03 01:00:00 dst=0 zone=AST" },
+  { 2152159199, "AST4ADT",
+    "2038/03/14 01:59:59 dst=0 zone=AST" },
+  { 2152159200, "AST4ADT",
+    "2038/03/14 03:00:00 dst=1 zone=ADT" },
+  { 2172718799, "AST4ADT",
+    "2038/11/07 01:59:59 dst=1 zone=ADT" },
+  { 2172718800, "AST4ADT",
+    "2038/11/07 01:00:00 dst=0 zone=AST" },
 };
 
 static int
@@ -34,6 +72,7 @@  do_test (void)
 {
   int result = 0;
   size_t cnt;
+  time_t when;
 
   for (cnt = 0; cnt < sizeof (tests) / sizeof (tests[0]); ++cnt)
     {
@@ -41,12 +80,18 @@  do_test (void)
       struct tm *tmp;
 
       printf ("TZ = \"%s\", time = %jd => ", tests[cnt].tz,
-	      (intmax_t) tests[cnt].when);
+	      tests[cnt].when);
       fflush (stdout);
 
+      if (ckd_add (&when, tests[cnt].when, 0))
+	{
+	  puts ("SKIPPED [time_t out of range for this platform]");
+	  continue;
+	}
+
       setenv ("TZ", tests[cnt].tz, 1);
 
-      tmp = localtime (&tests[cnt].when);
+      tmp = localtime (&when);
 
       snprintf (buf, sizeof (buf),
 		"%04d/%02d/%02d %02d:%02d:%02d dst=%d zone=%s",
@@ -66,7 +111,8 @@  do_test (void)
     }
 
   setenv ("TZ", "Universal", 1);
-  localtime (&tests[0].when);
+  when = tests[0].when;
+  localtime (&when);
   printf ("TZ = \"Universal\" daylight %d tzname = { \"%s\", \"%s\" }",
 	  daylight, tzname[0], tzname[1]);
   if (! daylight)
diff --git a/time/tzfile.c b/time/tzfile.c
index edb5643335..bca8b3f4ef 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -57,7 +57,6 @@  static size_t num_types;
 static struct ttinfo *types;
 static char *zone_names;
 static long int rule_stdoff;
-static long int rule_dstoff;
 static size_t num_leaps;
 static struct leap *leaps;
 static char *tzspec;
@@ -103,7 +102,7 @@  decode64 (const void *ptr)
 
 
 void
-__tzfile_read (const char *file, size_t extra, char **extrap)
+__tzfile_read (const char *file)
 {
   static const char default_tzdir[] = TZDIR;
   size_t num_isstd, num_isgmt;
@@ -257,7 +256,6 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
      unsigned char type_idxs[num_types];
      char zone_names[chars];
      char tzspec[tzspec_len];
-     char extra_array[extra]; // Stored into *pextras if requested.
 
      The piece-wise allocations from buf below verify that no
      overflow/wraparound occurred in these computations.
@@ -277,7 +275,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 			 + num_types * sizeof (struct ttinfo)
 			 + num_transitions /* type_idxs */
 			 + chars /* zone_names */
-			 + tzspec_len + extra);
+			 + tzspec_len);
     transitions = malloc (total_size);
     if (transitions == NULL)
       goto lose;
@@ -295,8 +293,6 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
     tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_len);
   else
     tzspec = NULL;
-  if (extra > 0)
-    *extrap = alloc_buffer_alloc_array (&buf, char, extra);
   if (alloc_buffer_has_failed (&buf))
     goto lose;
 
@@ -447,7 +443,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
   daylight_saved = 0;
   if (num_transitions == 0)
     /* Use the first rule (which should also be the only one).  */
-    rule_stdoff = rule_dstoff = types[0].offset;
+    rule_stdoff = types[0].offset;
   else
     {
       rule_stdoff = 0;
@@ -488,98 +484,6 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
   transitions = NULL;
 }
 
-/* The user specified a hand-made timezone, but not its DST rules.
-   We will use the names and offsets from the user, and the rules
-   from the TZDEFRULES file.  */
-
-void
-__tzfile_default (const char *std, const char *dst,
-		  int stdoff, int dstoff)
-{
-  size_t stdlen = strlen (std) + 1;
-  size_t dstlen = strlen (dst) + 1;
-  size_t i;
-  int isdst;
-  char *cp;
-
-  __tzfile_read (TZDEFRULES, stdlen + dstlen, &cp);
-  if (!__use_tzfile)
-    return;
-
-  if (num_types < 2)
-    {
-      __use_tzfile = 0;
-      return;
-    }
-
-  /* Ignore the zone names read from the file and use the given ones
-     instead.  */
-  __mempcpy (__mempcpy (cp, std, stdlen), dst, dstlen);
-  zone_names = cp;
-
-  /* Now there are only two zones, regardless of what the file contained.  */
-  num_types = 2;
-
-  /* Now correct the transition times for the user-specified standard and
-     daylight offsets from GMT.  */
-  isdst = 0;
-  for (i = 0; i < num_transitions; ++i)
-    {
-      struct ttinfo *trans_type = &types[type_idxs[i]];
-
-      /* We will use only types 0 (standard) and 1 (daylight).
-	 Fix up this transition to point to whichever matches
-	 the flavor of its original type.  */
-      type_idxs[i] = trans_type->isdst;
-
-      if (trans_type->isgmt)
-	/* The transition time is in GMT.  No correction to apply.  */ ;
-      else if (isdst && !trans_type->isstd)
-	/* The type says this transition is in "local wall clock time", and
-	   wall clock time as of the previous transition was DST.  Correct
-	   for the difference between the rule's DST offset and the user's
-	   DST offset.  */
-	transitions[i] += dstoff - rule_dstoff;
-      else
-	/* This transition is in "local wall clock time", and wall clock
-	   time as of this iteration is non-DST.  Correct for the
-	   difference between the rule's standard offset and the user's
-	   standard offset.  */
-	transitions[i] += stdoff - rule_stdoff;
-
-      /* The DST state of "local wall clock time" for the next iteration is
-	 as specified by this transition.  */
-      isdst = trans_type->isdst;
-    }
-
-  /* Now that we adjusted the transitions to the requested offsets,
-     reset the rule_stdoff and rule_dstoff values appropriately.  They
-     are used elsewhere.  */
-  rule_stdoff = stdoff;
-  rule_dstoff = dstoff;
-
-  /* Reset types 0 and 1 to describe the user's settings.  */
-  types[0].idx = 0;
-  types[0].offset = stdoff;
-  types[0].isdst = 0;
-  types[1].idx = stdlen;
-  types[1].offset = dstoff;
-  types[1].isdst = 1;
-
-  /* Reset time zone abbreviations to point to the user's abbreviations.  */
-  __tzname[0] = (char *) std;
-  __tzname[1] = (char *) dst;
-
-  /* Set the timezone.  */
-  __timezone = -types[0].offset;
-
-  /* Invalidate the tzfile attribute cache to force rereading
-     TZDEFRULES the next time it is used.  */
-  tzfile_dev = 0;
-  tzfile_ino = 0;
-  tzfile_mtime = 0;
-}
-
 void
 __tzfile_compute (__time64_t timer, int use_localtime,
 		  long int *leap_correct, int *leap_hit,
@@ -642,16 +546,6 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 	  /* Use the rules from the TZ string to compute the change.  */
 	  __tz_compute (timer, tp, 1);
 
-	  /* If tzspec comes from posixrules loaded by __tzfile_default,
-	     override the STD and DST zone names with the ones user
-	     requested in TZ envvar.  */
-	  if (__glibc_unlikely (zone_names == (char *) &leaps[num_leaps]))
-	    {
-	      assert (num_types == 2);
-	      __tzname[0] = __tzstring (zone_names);
-	      __tzname[1] = __tzstring (&zone_names[strlen (zone_names) + 1]);
-	    }
-
 	  goto leap;
 	}
       else
diff --git a/time/tzset.c b/time/tzset.c
index 0ddc203287..e178742222 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -331,22 +331,7 @@  __tzset_parse_tz (const char *tz)
       if (*tz != '\0')
 	{
 	  if (parse_tzname (&tz, 1))
-	    {
-	      parse_offset (&tz, 1);
-	      if (*tz == '\0' || (tz[0] == ',' && tz[1] == '\0'))
-		{
-		  /* There is no rule.  See if there is a default rule
-		     file.  */
-		  __tzfile_default (tz_rules[0].name, tz_rules[1].name,
-				    tz_rules[0].offset, tz_rules[1].offset);
-		  if (__use_tzfile)
-		    {
-		      free (old_tz);
-		      old_tz = NULL;
-		      return;
-		    }
-		}
-	    }
+	    parse_offset (&tz, 1);
 	  /* Figure out the standard <-> DST rules.  */
 	  if (parse_rule (&tz, 0))
 	    parse_rule (&tz, 1);
@@ -402,7 +387,7 @@  tzset_internal (int always)
   old_tz = tz ? __strdup (tz) : NULL;
 
   /* Try to read a data file.  */
-  __tzfile_read (tz, 0, NULL);
+  __tzfile_read (tz);
   if (__use_tzfile)
     return;
 
diff --git a/timezone/Makefile b/timezone/Makefile
index ebe5cf73a1..6e28a2e670 100644
--- a/timezone/Makefile
+++ b/timezone/Makefile
@@ -80,7 +80,8 @@  CFLAGS-zic.c += $(tz-cflags) -Wno-unused-variable
 # Don't add leapseconds here since test-tz made checks that work only without
 # leapseconds.
 define build-testdata
-$(built-program-cmd) -d $(testdata) -y ./yearistype $<; \
+$(built-program-cmd) -d $(testdata) \
+  $(if $(findstring northamerica,$<),-p America/New_York) $<; \
 $(evaluate-test)
 endef
 
diff --git a/timezone/tst-timezone.c b/timezone/tst-timezone.c
index 2d90be305d..882b3b8fd6 100644
--- a/timezone/tst-timezone.c
+++ b/timezone/tst-timezone.c
@@ -16,13 +16,15 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <stdbool.h>
+#include <stdckdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdint.h>
 #include <unistd.h>
 
-int failed = 0;
+bool failed = false;
 
 struct test_times
 {
@@ -69,20 +71,20 @@  check_tzvars (const char *name, int dayl, int timez, const char *const tznam[])
     {
       printf ("*** Timezone: %s, daylight is: %d but should be: %d\n",
 	      name, daylight, dayl);
-      ++failed;
+      failed = true;
     }
   if (timezone != timez)
     {
       printf ("*** Timezone: %s, timezone is: %ld but should be: %d\n",
 	      name, timezone, timez);
-      ++failed;
+      failed = true;
     }
   for (i = 0; i <= 1; ++i)
     if (strcmp (tzname[i], tznam[i]) != 0)
       {
 	printf ("*** Timezone: %s, tzname[%d] is: %s but should be: %s\n",
 		name, i, tzname[i], tznam[i]);
-	++failed;
+	failed = true;
       }
 }
 
@@ -106,7 +108,7 @@  do_test (void)
       if (putenv (buf))
 	{
 	  puts ("putenv failed.");
-	  failed = 1;
+	  failed = true;
 	}
       tzset ();
       print_tzvars ();
@@ -136,7 +138,7 @@  do_test (void)
     puts ("TZ=Europe/London 892162800 0 0 0 10 3 98 5 99 1");
     if (strcmp (buf, "TZ=Europe/London 892162800 0 0 0 10 3 98 5 99 1") != 0)
       {
-	failed = 1;
+	failed = true;
 	fputs (" FAILED ***", stdout);
       }
   }
@@ -159,11 +161,59 @@  do_test (void)
     puts ("TZ=GMT 892166400 0 0 0 10 3 98 5 99 0");
     if (strcmp (buf, "TZ=GMT 892166400 0 0 0 10 3 98 5 99 0") != 0)
       {
-	failed = 1;
+	failed = true;
 	fputs (" FAILED ***", stdout);
       }
   }
 
+  /* Test AST4ADT here, as well as in ../time/tst-posixtz.c, because
+     formerly the tzset implementation read the posixrules file
+     generated as part of the timezone data, and some bugs of that old
+     implementation are covered by this test.  */
+  {
+    static struct
+    {
+      intmax_t when;
+      char const *tz;
+      char const *result;
+    } const default_dst_test[] =
+    {
+      /* Test Atlantic Standard / Atlantic Daylight transitions in
+	 2024 and 2038, with default DST rule.  */
+      { 1710050399, "AST4ADT", "2024-03-10 01:59:59 -0400 (AST)" },
+      { 1710050400, "AST4ADT", "2024-03-10 03:00:00 -0300 (ADT)" },
+      { 1730609999, "AST4ADT", "2024-11-03 01:59:59 -0300 (ADT)" },
+      { 1730610000, "AST4ADT", "2024-11-03 01:00:00 -0400 (AST)" },
+      { 2152159199, "AST4ADT", "2038-03-14 01:59:59 -0400 (AST)" },
+      { 2152159200, "AST4ADT", "2038-03-14 03:00:00 -0300 (ADT)" },
+      { 2172718799, "AST4ADT", "2038-11-07 01:59:59 -0300 (ADT)" },
+      { 2172718800, "AST4ADT", "2038-11-07 01:00:00 -0400 (AST)" },
+    };
+
+    for (int i = 0; i < sizeof default_dst_test / sizeof *default_dst_test; i++)
+      {
+	sprintf (envstring, "TZ=%s", default_dst_test[i].tz);
+	/* No putenv call needed!  */
+
+	time_t when;
+	if (ckd_add (&when, default_dst_test[i].when, 0))
+	  {
+	    printf ("time=%jd out of range for test; test skipped\n",
+		    default_dst_test[i].when);
+	    continue;
+	  }
+	char buf[100];
+	strftime (buf, sizeof buf, "%Y-%m-%d %H:%M:%S %z (%Z)",
+		  localtime (&when));
+	bool bad = strcmp (buf, default_dst_test[i].result) != 0;
+	printf ("%s should be\n%s when time=%jd TZ=%s%s\n\n",
+		buf, default_dst_test[i].result,
+		default_dst_test[i].when, default_dst_test[i].tz,
+		bad ? " FAILED ***" : "");
+	failed |= bad;
+      }
+  }
+
   return failed ? EXIT_FAILURE : EXIT_SUCCESS;
 }