[patch] Fix libfortran/98507, handling of timezone near year boundaries

[patch] Fix libfortran/98507, handling of timezone near year boundaries

[FX]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

Hi,

DATE_AND_TIME can return incorrect values for non-UTC timezones, near the new year, when the local time and UTC time are in different years. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98507

Attached patch fixes the issue by correcting the logic to account for that wrapping of “day of the year” around new year. I include a testcase, which checks the sanity of values returned by DATE_AND_TIME. Since the bug only occurs for a few hours every year, and depends on local timezone, I could not think of a better (systematic) test.

I also want to propose (it’s not directly needed to fix the bug) that we switch our time routines to rely on clock_gettime() instead of gettimeofday(), when available. This is in line with POSIX.1-2008, which marks gettimeofday() as obsolete, recommending the use of 
clock_gettime() instead.

Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK to commit?

FX



[-- Attachment #2: pr98507.patch --]
[-- Type: application/octet-stream, Size: 4520 bytes --]

commit 81631ceaccf9c170e547e06f94c2e8a9e644134c
Author: Francois-Xavier Coudert <fxcoudert@gmail.com>
Date:   2021-12-16 12:40:03 +0100

    Fix timezone handling near year boundaries
    
    PR libfortran/98507
    
    libgfortran/ChangeLog:
    
            * intrinsics/time_1.h: Prefer clock_gettime() over
              gettimeofday().
            * intrinsics/date_and_time.c: Fix timezone wrapping.
    
    gcc/testsuite/ChangeLog:
    
            * gfortran.dg/date_and_time_1.f90: New file.

diff --git a/gcc/testsuite/gfortran.dg/date_and_time_1.f90 b/gcc/testsuite/gfortran.dg/date_and_time_1.f90
new file mode 100644
index 00000000000..0cd0c390d8c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/date_and_time_1.f90
@@ -0,0 +1,35 @@
+! PR libfortran/98507
+! { dg-do run }
+
+program demo_time_and_date
+  implicit none
+  character(8)  :: date
+  character(10) :: time
+  character(5)  :: zone
+  integer :: val(8)
+  integer :: h, m
+
+  call date_and_time(values=val)
+
+  if (val(1) < 2000 .or. val(1) > 2100) call abort
+  if (val(2) < 1 .or. val(2) > 12) call abort
+  if (val(3) < 1 .or. val(3) > 31) call abort
+
+  ! Maximum offset is 14 hours (UTC+14)
+  if (val(4) < -14*60 .or. val(4) > 14*60) call abort
+
+  if (val(5) < 0 .or. val(5) > 23) call abort
+  if (val(6) < 0 .or. val(6) > 59) call abort
+  if (val(7) < 0 .or. val(7) > 60) call abort
+  if (val(8) < 0 .or. val(8) > 999) call abort
+
+  call date_and_time(zone=zone)
+  if (len(zone) /= 0) then
+    ! If ZONE is present, it should present the same information as
+    ! given in VALUES(4)
+    if (len(zone) /= 5) call abort
+    read(zone(1:3),*) h
+    read(zone(4:5),*) m
+    if (val(4) /= 60*h+m) call abort
+  endif
+end
diff --git a/libgfortran/intrinsics/date_and_time.c b/libgfortran/intrinsics/date_and_time.c
index 8213127ec95..de40bbc964e 100644
--- a/libgfortran/intrinsics/date_and_time.c
+++ b/libgfortran/intrinsics/date_and_time.c
@@ -113,9 +113,6 @@ gmtime_r (const time_t * timep, struct tm * result)
    VALUES for INTEGER(kind=4) and INTEGER(kind=8).
 
    Based on libU77's date_time_.c.
-
-   TODO :
-   - Check year boundaries.
 */
 #define DATE_LEN 8
 #define TIME_LEN 10   
@@ -131,7 +128,7 @@ date_and_time (char *__date, char *__time, char *__zone,
 	       gfc_array_i4 *__values, GFC_INTEGER_4 __date_len,
 	       GFC_INTEGER_4 __time_len, GFC_INTEGER_4 __zone_len)
 {
-  int i;
+  int i, delta_day;
   char date[DATE_LEN + 1];
   char timec[TIME_LEN + 1];
   char zone[ZONE_LEN + 1];
@@ -154,9 +151,22 @@ date_and_time (char *__date, char *__time, char *__zone,
       values[0] = 1900 + local_time.tm_year;
       values[1] = 1 + local_time.tm_mon;
       values[2] = local_time.tm_mday;
-      values[3] = (local_time.tm_min - UTC_time.tm_min +
-	           60 * (local_time.tm_hour - UTC_time.tm_hour +
-		     24 * (local_time.tm_yday - UTC_time.tm_yday)));
+
+      /* Day difference with UTC should always be -1, 0 or +1.
+	 Near year boundaries, we may obtain a large positive (+364,
+	 or +365 on leap years) or negative (-364, or -365 on leap years)
+	 number, which we have to handle.
+	 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98507
+       */
+      delta_day = local_time.tm_yday - UTC_time.tm_yday;
+      if (delta_day < -1)
+	delta_day = 1;
+      else if (delta_day > 1)
+	delta_day = -1;
+
+      values[3] = local_time.tm_min - UTC_time.tm_min
+		  + 60 * (local_time.tm_hour - UTC_time.tm_hour + 24 * delta_day);
+
       values[4] = local_time.tm_hour;
       values[5] = local_time.tm_min;
       values[6] = local_time.tm_sec;
diff --git a/libgfortran/intrinsics/time_1.h b/libgfortran/intrinsics/time_1.h
index 2d238fd075b..b2adca0c5f3 100644
--- a/libgfortran/intrinsics/time_1.h
+++ b/libgfortran/intrinsics/time_1.h
@@ -213,19 +213,19 @@ gf_cputime (long *user_sec, long *user_usec, long *system_sec, long *system_usec
 static inline int
 gf_gettime (time_t * secs, long * usecs)
 {
-#ifdef HAVE_GETTIMEOFDAY
+#ifdef HAVE_CLOCK_GETTIME
+  struct timespec ts;
+  int err = clock_gettime (CLOCK_REALTIME, &ts);
+  *secs = ts.tv_sec;
+  *usecs = ts.tv_nsec / 1000;
+  return err;
+#elif defined(HAVE_GETTIMEOFDAY)
   struct timeval tv;
   int err;
   err = gettimeofday (&tv, NULL);
   *secs = tv.tv_sec;
   *usecs = tv.tv_usec;
   return err;
-#elif defined(HAVE_CLOCK_GETTIME)
-  struct timespec ts;
-  int err = clock_gettime (CLOCK_REALTIME, &ts);
-  *secs = ts.tv_sec;
-  *usecs = ts.tv_nsec / 1000;
-  return err;
 #else
   time_t t = time (NULL);
   *secs = t;

Re: [patch] Fix libfortran/98507, handling of timezone near year boundaries

[Harald Anlauf]

Hi FX,

Am 16.12.21 um 15:17 schrieb FX via Fortran:
> Hi,
>
> DATE_AND_TIME can return incorrect values for non-UTC timezones, near the new year, when the local time and UTC time are in different years. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98507
>
> Attached patch fixes the issue by correcting the logic to account for that wrapping of “day of the year” around new year. I include a testcase, which checks the sanity of values returned by DATE_AND_TIME. Since the bug only occurs for a few hours every year, and depends on local timezone, I could not think of a better (systematic) test.

please change "call abort" to "stop n".  The former should no longer be
used in the testsuite.

> I also want to propose (it’s not directly needed to fix the bug) that we switch our time routines to rely on clock_gettime() instead of gettimeofday(), when available. This is in line with POSIX.1-2008, which marks gettimeofday() as obsolete, recommending the use of
> clock_gettime() instead.

Agreed.

> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> OK to commit?
>
> FX
>
>

OK after fixing the above, and thanks for the patch!

Harald

Re: [patch] Fix libfortran/98507, handling of timezone near year boundaries

[Harald Anlauf]

Hi FX,

Am 16.12.21 um 15:17 schrieb FX via Fortran:
> Hi,
> 
> DATE_AND_TIME can return incorrect values for non-UTC timezones, near the new year, when the local time and UTC time are in different years. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98507
> 
> Attached patch fixes the issue by correcting the logic to account for that wrapping of “day of the year” around new year. I include a testcase, which checks the sanity of values returned by DATE_AND_TIME. Since the bug only occurs for a few hours every year, and depends on local timezone, I could not think of a better (systematic) test.

please change "call abort" to "stop n".  The former should no longer be 
used in the testsuite.

> I also want to propose (it’s not directly needed to fix the bug) that we switch our time routines to rely on clock_gettime() instead of gettimeofday(), when available. This is in line with POSIX.1-2008, which marks gettimeofday() as obsolete, recommending the use of
> clock_gettime() instead.

Agreed.

> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> OK to commit?
> 
> FX
> 
> 

OK after fixing the above, and thanks for the patch!

Harald

Re: [patch] Fix libfortran/98507, handling of timezone near year boundaries

[FX]

> OK after fixing the above, and thanks for the patch!

Patch committed, after changing “call abort” to “stop”.
Thanks for the review.

FX

Re: [patch] Fix libfortran/98507, handling of timezone near year boundaries

[Thomas Koenig]

Hi FX,

> DATE_AND_TIME can return incorrect values for non-UTC timezones, near the new year, when the local time and UTC time are in different years. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98507
> 
> Attached patch fixes the issue by correcting the logic to account for that wrapping of “day of the year” around new year. I include a testcase, which checks the sanity of values returned by DATE_AND_TIME. Since the bug only occurs for a few hours every year, and depends on local timezone, I could not think of a better (systematic) test.
> 
> I also want to propose (it’s not directly needed to fix the bug) that we switch our time routines to rely on clock_gettime() instead of gettimeofday(), when available. This is in line with POSIX.1-2008, which marks gettimeofday() as obsolete, recommending the use of
> clock_gettime() instead.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> OK to commit?

OK.

Thanks for fixing this in time for the new year 2022 :-)

Regards

	Thomas