Re: [patch,fortran] Fix corner case where consecutive calls to date_and_time can return times out of sequence (PR30015)

[Tobias Burnus <burnus at net-b dot de>]

*ping*

This patch removes one corner case in favour of another occurring corner
case & reduces the change that the corner case occurs.

Currently, the time can go backwards for [illustrative example only; the calls shouldn't take that long]
   call date_and_time()
     ! call at 19.0s, returns at 19.3s, returned value 19.1s
   ! calculate for some time
   call date_and_time()
     ! call at 19.9s, returns at 19.2s, returned value 19.0s
     ! => Backward in time!

With the new patch it can happen for
   call date_and_time()
     ! call at 03:10.9s, returns at 03:11.2s, returned value 03:11.0s
   ! calculate for some time
   call date_and_time()
     ! call at 03:59.9s, returns at 04:00.2s, returned value 03:00.0s
     ! => Backward in time!


In a simple view, the former is 60 times more likely to occur, whereas
the second is 60times more prominent.

However, there is another reason why the second case is even less likely
than 60 times.

Before we had two extra function calls in-between
  lt = time (NULL);
  if (lt != (time_t) -1)
    {
      local_time = *localtime (&lt);
      UTC_time = *gmtime (&lt);
// lots of assignments
        if (!gettimeofday (&tp))

Now one has no function call in-between
  lt = time (NULL);
  if (lt != (time_t) -1)
        if (!gettimeofday (&tp))

Thus, if you don't like the patch, since the effect of the corner case
is now in the minute/seconds rather than in the second/milliseconds, we
should still move the gettimeofday() up.

Tobias


Tobias Burnus schrieb:
> :ADDPATCH fortran:
>
> As found (and fixed) by Mat Cross:
>
> In libgfortran's date_and_time(...), we currently have:
>
>   lt = time (NULL);
>   [...]
>       local_time = *localtime (&lt);
>       UTC_time = *gmtime (&lt);
>   [...]
>       values[6] = local_time.tm_sec;
>       values[7] = 0;
>   [...]
>       gettimeofday (&tp
>       values[7] = tp.tv_usec / 1000;
>
>
> Example for the border case:
>
> First call to date_and_time(...)
>      lt = time() // assume time 19.0s
>      values[7] = tp.tv_usec / 1000; // assume time 19.4s
> gives 19.4s
>
> Second call to date_and_time(...)
>      lt = time() // assume time 19.8s
>      values[7] = tp.tv_usec / 1000; // assume time 20.1s
> gives 19.1s
> Thus we produced a time machine!
>
> The suggested fix is use:
>    lt = time (NULL);
>    [...]
>       gettimeofday (&tp)
>       lt = tp.tv_sec
>       values[7] = tp.tv_usec / 1000;
>
> The change by Mat Cross is therefore:
> - Moving the  gettimeofday() block up in the file
> - Using the result of gettimeofday() for lt and value[7], instead of
> using time() for lt and gettimeofday() for value[7]
>  date_and_time.c |   61
> +++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 36 insertions(+), 25 deletions(-)
>
> As it was mostly moving things around and changing
>    value[7] = 0;  // at one place
> into
>      lt = tp.tv_sec; // at another place
> #   else
>       value[7] = 0;
> I think it is copyright-wise ok, isn't it?
>
> Ok for the trunk and after a week or so for 4.2 and 4.1 (which are
> identical)?
>
>
> Build and regression tested on the trunk with x86_64-unknown-linux-gnu.
>
> The PR contains also an example, but I failed to trigger the error.
> If you think it makes sense, we can also add the example to our test suite.
>
> Tobias
>
>
> 2006-11-29  Mat Cross  <mathewc@nag.co.uk>
>
>     PR fortran/30015
>     * intrinsics/date_and_time.c (date_and_time): Fix case where time
> can go backwards.
>   
> ------------------------------------------------------------------------
>
> Index: libgfortran/intrinsics/date_and_time.c
> ===================================================================
> --- libgfortran/intrinsics/date_and_time.c	(Revision 119312)
> +++ libgfortran/intrinsics/date_and_time.c	(Arbeitskopie)
> @@ -158,6 +158,42 @@
>  
>    if (lt != (time_t) -1)
>      {
> +#if HAVE_GETTIMEOFDAY
> +      {
> +        struct timeval tp;
> +
> +#if GETTIMEOFDAY_ONE_ARGUMENT
> +        if (!gettimeofday (&tp))
> +#else
> +
> +#if HAVE_STRUCT_TIMEZONE
> +          struct timezone tzp;
> +
> +        /* Some systems such as HP-UX, do have struct timezone, but
> +           gettimeofday takes void* as the 2nd arg.  However, the
> +           effect of passing anything other than a null pointer is
> +           unspecified on HP-UX.  Configure checks if gettimeofday
> +           actually fails with a non-NULL arg and pretends that
> +           struct timezone is missing if it does fail.  */
> +        if (!gettimeofday (&tp, &tzp))
> +#else
> +        if (!gettimeofday (&tp, (void *) 0))
> +#endif /* HAVE_STRUCT_TIMEZONE  */
> +
> +#endif /* GETTIMEOFDAY_ONE_ARGUMENT  */
> +
> +        /* All arguments can be derived from tp.  */
> +        lt = tp.tv_sec;
> +        values[7] = tp.tv_usec / 1000;
> +      }
> +#else
> +      {
> +        /* All arguments can be derived from lt.  */
> +        values[7] = 0;
> +      }
> +
> +#endif /* HAVE_GETTIMEOFDAY */
> +
>        local_time = *localtime (&lt);
>        UTC_time = *gmtime (&lt);
>  
> @@ -171,31 +207,6 @@
>        values[4] = local_time.tm_hour;
>        values[5] = local_time.tm_min;
>        values[6] = local_time.tm_sec;
> -      values[7] = 0;
> -
> -#if HAVE_GETTIMEOFDAY
> -      {
> -	struct timeval tp;
> -#  if GETTIMEOFDAY_ONE_ARGUMENT
> -	if (!gettimeofday (&tp))
> -#  else
> -#    if HAVE_STRUCT_TIMEZONE
> -	struct timezone tzp;
> -
> -      /* Some systems such as HP-UX, do have struct timezone, but
> -	 gettimeofday takes void* as the 2nd arg.  However, the
> -	 effect of passing anything other than a null pointer is
> -	 unspecified on HP-UX.  Configure checks if gettimeofday
> -	 actually fails with a non-NULL arg and pretends that
> -	 struct timezone is missing if it does fail.  */
> -	if (!gettimeofday (&tp, &tzp))
> -#    else
> -	if (!gettimeofday (&tp, (void *) 0))
> -#    endif /* HAVE_STRUCT_TIMEZONE  */
> -#  endif /* GETTIMEOFDAY_ONE_ARGUMENT  */
> -	values[7] = tp.tv_usec / 1000;
> -      }
> -#endif /* HAVE_GETTIMEOFDAY */
>  
>  #if HAVE_SNPRINTF
>        if (__date)
>