public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
From: H.J. Lu <hjl@sourceware.org>
To: glibc-cvs@sourceware.org
Subject: [glibc] Correct timespec implementation [BZ #26232]
Date: Tue, 14 Jul 2020 11:43:22 +0000 (GMT)	[thread overview]
Message-ID: <20200714114322.3044E3858D37@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f896fc0f2bfc9f3f8df0563a7c99dcbf24bab655

commit f896fc0f2bfc9f3f8df0563a7c99dcbf24bab655
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Jul 13 16:15:56 2020 -0700

    Correct timespec implementation [BZ #26232]
    
    commit 04deeaa9ea74b0679dfc9d9155a37b6425f19a9f
    Author: Lucas A. M. Magalhaes <lamm@linux.ibm.com>
    Date:   Fri Jul 10 19:41:06 2020 -0300
    
        Fix time/tst-cpuclock1 intermitent failures
    
    has 2 issues:
    
    1. It assumes time_t == long which is false on x32.
    2. tst-timespec.c is compiled without -fexcess-precision=standard which
    generates incorrect results on i686 in support_timespec_check_in_range:
    
      double ratio = (double)observed_norm / expected_norm;
      return (lower_bound <= ratio && ratio <= upper_bound);
    
    This patch does
    
    1. Compile tst-timespec.c with -fexcess-precision=standard.
    2. Replace long with time_t.
    3. Replace LONG_MIN and LONG_MAX with TYPE_MINIMUM (time_t) and
    TYPE_MAXIMUM (time_t).

Diff:
---
 support/Makefile       |  7 ++++
 support/timespec.c     | 18 ++++------
 support/timespec.h     |  2 +-
 support/tst-timespec.c | 98 ++++++++++++++++++++++++++++----------------------
 4 files changed, 71 insertions(+), 54 deletions(-)

diff --git a/support/Makefile b/support/Makefile
index e74e0dd519..93faafddf9 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -196,6 +196,13 @@ CFLAGS-support_paths.c = \
 		-DROOTSBINDIR_PATH=\"$(rootsbindir)\" \
 		-DCOMPLOCALEDIR_PATH=\"$(complocaledir)\"
 
+# In support_timespec_check_in_range we may be passed a very tight
+# range for which we should produce a correct result for expected
+# being withing the observed range.  The code uses double internally
+# in support_timespec_check_in_range and for that computation we use
+# -fexcess-precision=standard.
+CFLAGS-timespec.c += -fexcess-precision=standard
+
 ifeq (,$(CXX))
 LINKS_DSO_PROGRAM = links-dso-program-c
 else
diff --git a/support/timespec.c b/support/timespec.c
index 9f5449e49e..edbdb165ec 100644
--- a/support/timespec.c
+++ b/support/timespec.c
@@ -60,21 +60,17 @@ test_timespec_equal_or_after_impl (const char *file, int line,
   }
 }
 
-/* Convert TIME to nanoseconds stored in a long.
-   Returns long maximum or minimum if the conversion overflows
+/* Convert TIME to nanoseconds stored in a time_t.
+   Returns time_t maximum or minimum if the conversion overflows
    or underflows, respectively.  */
-long
+time_t
 support_timespec_ns (struct timespec time)
 {
-  long time_ns;
+  time_t time_ns;
   if (INT_MULTIPLY_WRAPV(time.tv_sec, TIMESPEC_HZ, &time_ns))
-   {
-      return (time.tv_sec < 0) ? TYPE_MINIMUM(long): TYPE_MAXIMUM(long);
-   }
+    return time.tv_sec < 0 ? TYPE_MINIMUM(time_t) : TYPE_MAXIMUM(time_t);
   if (INT_ADD_WRAPV(time_ns, time.tv_nsec, &time_ns))
-   {
-      return (time.tv_nsec < 0) ? TYPE_MINIMUM(long): TYPE_MAXIMUM(long);
-   }
+    return time.tv_nsec < 0 ? TYPE_MINIMUM(time_t) : TYPE_MAXIMUM(time_t);
   return time_ns;
 }
 
@@ -113,7 +109,7 @@ support_timespec_check_in_range (struct timespec expected, struct timespec obser
 			      double lower_bound, double upper_bound)
 {
   assert (upper_bound >= lower_bound);
-  long expected_norm, observed_norm;
+  time_t expected_norm, observed_norm;
   expected_norm = support_timespec_ns (expected);
   /* Don't divide by zero  */
   assert(expected_norm != 0);
diff --git a/support/timespec.h b/support/timespec.h
index fd5466745d..1a6775a882 100644
--- a/support/timespec.h
+++ b/support/timespec.h
@@ -48,7 +48,7 @@ void test_timespec_equal_or_after_impl (const char *file, int line,
                                         const struct timespec left,
                                         const struct timespec right);
 
-long support_timespec_ns (struct timespec time);
+time_t support_timespec_ns (struct timespec time);
 
 struct timespec support_timespec_normalize (struct timespec time);
 
diff --git a/support/tst-timespec.c b/support/tst-timespec.c
index 71423555a9..ac5ed228ba 100644
--- a/support/tst-timespec.c
+++ b/support/tst-timespec.c
@@ -19,13 +19,14 @@
 #include <support/timespec.h>
 #include <support/check.h>
 #include <limits.h>
+#include <intprops.h>
 
 #define TIMESPEC_HZ 1000000000
 
 struct timespec_ns_test_case
 {
   struct timespec time;
-  long time_ns;
+  time_t time_ns;
 };
 
 struct timespec_norm_test_case
@@ -43,6 +44,9 @@ struct timespec_test_case
   int result;
 };
 
+#define TIME_T_MIN TYPE_MINIMUM (time_t)
+#define TIME_T_MAX TYPE_MAXIMUM (time_t)
+
 /* Test cases for timespec_ns */
 struct timespec_ns_test_case ns_cases[] = {
   {.time = {.tv_sec = 0, .tv_nsec = 0},
@@ -73,36 +77,42 @@ struct timespec_ns_test_case ns_cases[] = {
    .time_ns = -TIMESPEC_HZ + 1,
   },
   /* Overflow bondary by 2  */
-  {.time = {.tv_sec = LONG_MAX / TIMESPEC_HZ, .tv_nsec = LONG_MAX%TIMESPEC_HZ - 1},
-   .time_ns = LONG_MAX - 1,
+  {.time = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MAX % TIMESPEC_HZ - 1},
+   .time_ns = TIME_T_MAX - 1,
   },
   /* Overflow bondary  */
-  {.time = {.tv_sec = LONG_MAX / TIMESPEC_HZ, .tv_nsec = LONG_MAX%TIMESPEC_HZ},
-   .time_ns = LONG_MAX,
+  {.time = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MAX % TIMESPEC_HZ},
+   .time_ns = TIME_T_MAX,
   },
   /* Underflow bondary by 1  */
-  {.time = {.tv_sec = LONG_MIN / TIMESPEC_HZ, .tv_nsec = LONG_MIN%TIMESPEC_HZ + 1},
-   .time_ns = LONG_MIN + 1,
+  {.time = {.tv_sec = TIME_T_MIN / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MIN % TIMESPEC_HZ + 1},
+   .time_ns = TIME_T_MIN + 1,
   },
   /* Underflow bondary  */
-  {.time = {.tv_sec = LONG_MIN / TIMESPEC_HZ, .tv_nsec = LONG_MIN%TIMESPEC_HZ},
-   .time_ns = LONG_MIN,
+  {.time = {.tv_sec = TIME_T_MIN / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MIN % TIMESPEC_HZ},
+   .time_ns = TIME_T_MIN,
   },
   /* Multiplication overflow  */
-  {.time = {.tv_sec = LONG_MAX / TIMESPEC_HZ + 1, .tv_nsec = 1},
-   .time_ns = LONG_MAX,
+  {.time = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ + 1, .tv_nsec = 1},
+   .time_ns = TIME_T_MAX,
   },
   /* Multiplication underflow  */
-  {.time = {.tv_sec = LONG_MIN / TIMESPEC_HZ - 1, .tv_nsec = -1},
-   .time_ns = LONG_MIN,
+  {.time = {.tv_sec = TIME_T_MIN / TIMESPEC_HZ - 1, .tv_nsec = -1},
+   .time_ns = TIME_T_MIN,
   },
   /* Sum overflows  */
-  {.time = {.tv_sec = LONG_MAX / TIMESPEC_HZ, .tv_nsec = LONG_MAX%TIMESPEC_HZ + 1},
-   .time_ns = LONG_MAX,
+  {.time = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MAX % TIMESPEC_HZ + 1},
+   .time_ns = TIME_T_MAX,
   },
   /* Sum underflow  */
-  {.time = {.tv_sec = LONG_MIN / TIMESPEC_HZ, .tv_nsec = LONG_MIN%TIMESPEC_HZ - 1},
-   .time_ns = LONG_MIN,
+  {.time = {.tv_sec = TIME_T_MIN / TIMESPEC_HZ,
+	    .tv_nsec = TIME_T_MIN % TIMESPEC_HZ - 1},
+   .time_ns = TIME_T_MIN,
   }
 };
 
@@ -144,28 +154,28 @@ struct timespec_norm_test_case norm_cases[] = {
    .norm = {.tv_sec = -2, .tv_nsec = -1}
   },
   /* Overflow bondary by 2  */
-  {.time = {.tv_sec = LONG_MAX - 2, .tv_nsec = TIMESPEC_HZ + 1},
-   .norm = {.tv_sec = LONG_MAX - 1, 1},
+  {.time = {.tv_sec = TIME_T_MAX - 2, .tv_nsec = TIMESPEC_HZ + 1},
+   .norm = {.tv_sec = TIME_T_MAX - 1, 1},
   },
   /* Overflow bondary by 1  */
-  {.time = {.tv_sec = LONG_MAX - 1, .tv_nsec = TIMESPEC_HZ + 1},
-   .norm = {.tv_sec = LONG_MAX, .tv_nsec = 1},
+  {.time = {.tv_sec = TIME_T_MAX - 1, .tv_nsec = TIMESPEC_HZ + 1},
+   .norm = {.tv_sec = TIME_T_MAX, .tv_nsec = 1},
   },
   /* Underflow bondary by 2  */
-  {.time = {.tv_sec = LONG_MIN + 2, .tv_nsec = -TIMESPEC_HZ - 1},
-   .norm = {.tv_sec = LONG_MIN + 1, -1},
+  {.time = {.tv_sec = TIME_T_MIN + 2, .tv_nsec = -TIMESPEC_HZ - 1},
+   .norm = {.tv_sec = TIME_T_MIN + 1, -1},
   },
   /* Underflow bondary by 1  */
-  {.time = {.tv_sec = LONG_MIN + 1, .tv_nsec = -TIMESPEC_HZ - 1},
-   .norm = {.tv_sec = LONG_MIN, .tv_nsec = -1},
+  {.time = {.tv_sec = TIME_T_MIN + 1, .tv_nsec = -TIMESPEC_HZ - 1},
+   .norm = {.tv_sec = TIME_T_MIN, .tv_nsec = -1},
   },
   /* SUM overflow  */
-  {.time = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ},
-   .norm = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ - 1},
+  {.time = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ},
+   .norm = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ - 1},
   },
   /* SUM underflow  */
-  {.time = {.tv_sec = LONG_MIN, .tv_nsec = -TIMESPEC_HZ},
-   .norm = {.tv_sec = LONG_MIN, .tv_nsec = -1 * (TIMESPEC_HZ - 1)},
+  {.time = {.tv_sec = TIME_T_MIN, .tv_nsec = -TIMESPEC_HZ},
+   .norm = {.tv_sec = TIME_T_MIN, .tv_nsec = -1 * (TIMESPEC_HZ - 1)},
   }
 };
 
@@ -243,39 +253,41 @@ struct timespec_test_case check_cases[] = {
   },
   /* Maximum/Minimum long values  */
   /* 14  */
-  {.expected = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ - 1},
-   .observed = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ - 2},
+  {.expected = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ - 1},
+   .observed = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ - 2},
    .upper_bound = 1, .lower_bound = .9, .result = 1,
   },
   /* 15 - support_timespec_ns overflow  */
-  {.expected = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ},
-   .observed = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ},
+  {.expected = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ},
+   .observed = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ},
    .upper_bound = 1, .lower_bound = 1, .result = 1,
   },
   /* 16 - support_timespec_ns overflow + underflow  */
-  {.expected = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ},
-   .observed = {.tv_sec = LONG_MIN, .tv_nsec = -TIMESPEC_HZ},
+  {.expected = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ},
+   .observed = {.tv_sec = TIME_T_MIN, .tv_nsec = -TIMESPEC_HZ},
    .upper_bound = 1, .lower_bound = 1, .result = 0,
   },
   /* 17 - support_timespec_ns underflow  */
-  {.expected = {.tv_sec = LONG_MIN, .tv_nsec = -TIMESPEC_HZ},
-   .observed = {.tv_sec = LONG_MIN, .tv_nsec = -TIMESPEC_HZ},
+  {.expected = {.tv_sec = TIME_T_MIN, .tv_nsec = -TIMESPEC_HZ},
+   .observed = {.tv_sec = TIME_T_MIN, .tv_nsec = -TIMESPEC_HZ},
    .upper_bound = 1, .lower_bound = 1, .result = 1,
   },
   /* 18 - support_timespec_ns underflow + overflow  */
-  {.expected = {.tv_sec = LONG_MIN, .tv_nsec = -TIMESPEC_HZ},
-   .observed = {.tv_sec = LONG_MAX, .tv_nsec = TIMESPEC_HZ},
+  {.expected = {.tv_sec = TIME_T_MIN, .tv_nsec = -TIMESPEC_HZ},
+   .observed = {.tv_sec = TIME_T_MAX, .tv_nsec = TIMESPEC_HZ},
    .upper_bound = 1, .lower_bound = 1, .result = 0,
   },
   /* 19 - Biggest division  */
-  {.expected = {.tv_sec = LONG_MAX / TIMESPEC_HZ , .tv_nsec = TIMESPEC_HZ - 1},
+  {.expected = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ,
+		.tv_nsec = TIMESPEC_HZ - 1},
    .observed = {.tv_sec = 0, .tv_nsec = 1},
    .upper_bound = 1, .lower_bound = 1.0842021724855044e-19, .result = 1,
   },
   /* 20 - Lowest division  */
   {.expected = {.tv_sec = 0, .tv_nsec = 1},
-   .observed = {.tv_sec = LONG_MAX / TIMESPEC_HZ , .tv_nsec = TIMESPEC_HZ - 1},
-   .upper_bound = LONG_MAX, .lower_bound = 1, .result = 1,
+   .observed = {.tv_sec = TIME_T_MAX / TIMESPEC_HZ,
+		.tv_nsec = TIMESPEC_HZ - 1},
+   .upper_bound = TIME_T_MAX, .lower_bound = 1, .result = 1,
   },
 };
 
@@ -288,6 +300,7 @@ do_test (void)
   printf("Testing support_timespec_ns\n");
   for (i = 0; i < ntests; i++)
     {
+      printf("Test case %d\n", i);
       TEST_COMPARE (support_timespec_ns (ns_cases[i].time),
 		    ns_cases[i].time_ns);
     }
@@ -297,6 +310,7 @@ do_test (void)
   printf("Testing support_timespec_normalize\n");
   for (i = 0; i < ntests; i++)
     {
+      printf("Test case %d\n", i);
       result = support_timespec_normalize (norm_cases[i].time);
       TEST_COMPARE (norm_cases[i].norm.tv_sec, result.tv_sec);
       TEST_COMPARE (norm_cases[i].norm.tv_nsec, result.tv_nsec);


                 reply	other threads:[~2020-07-14 11:43 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200714114322.3044E3858D37@sourceware.org \
    --to=hjl@sourceware.org \
    --cc=glibc-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).