From: "H.J. Lu" <hjl.tools@gmail.com>
To: Tulio Magno Quites Machado Filho <tuliom@ascii.art.br>
Cc: "Carlos O'Donell" <carlos@redhat.com>,
"Lucas A. M. Magalhaes" <lamm@linux.ibm.com>,
GNU C Library <libc-alpha@sourceware.org>
Subject: [PATCH] Correct timespec implementation [BZ #26232]
Date: Mon, 13 Jul 2020 16:30:18 -0700 [thread overview]
Message-ID: <CAMe9rOoo-=67Mu+=+frB-2GYX7H=KQdOnjDKo53vNzh4R2HUoA@mail.gmail.com> (raw)
In-Reply-To: <CAMe9rOpdodSJbp9uBHhe+YLuV_Gzh=GFGwdEKos1SvNWMePh2Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
On Sat, Jul 11, 2020 at 9:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Jul 11, 2020 at 7:45 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Jul 10, 2020 at 4:10 PM Tulio Magno Quites Machado Filho
> > <tuliom@ascii.art.br> wrote:
> > >
> > > Carlos O'Donell via Libc-alpha <libc-alpha@sourceware.org> writes:
> > >
> > > > OK for master. I'd like to see this fixed in glibc 2.32.
> > > > Thank you for fixing the test case!
> > > >
> > > > Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> > >
> > > Pushed as 04deeaa9ea74b0679dfc9d9155a37b6425f19a9f.
> > >
> >
> > support/tst-timespec failed on i686 and x32:
> >
> > Test case 10
> > tst-timespec.c:312: numeric comparison failure
> > left: 0 (0x0); from: support_timespec_check_in_range
> > (check_cases[i].expected, check_cases[i].observed,
> > check_cases[i].lower_bound, check_cases[i].upper_bound)
> > right: 1 (0x1); from: check_cases[i].result
> > Test case 11
> >
>
> Usage of long may be the problem.
>
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).
OK for master?
Thanks.
--
H.J.
[-- Attachment #2: 0001-Correct-timespec-implementation-BZ-26232.patch --]
[-- Type: text/x-patch, Size: 11377 bytes --]
From 3a3003d3a429035d7cfb8485c64a22cb9517ec48 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 13 Jul 2020 16:15:56 -0700
Subject: [PATCH] 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).
---
support/Makefile | 2 +
support/timespec.c | 18 +++-----
support/timespec.h | 2 +-
support/tst-timespec.c | 98 ++++++++++++++++++++++++------------------
4 files changed, 66 insertions(+), 54 deletions(-)
diff --git a/support/Makefile b/support/Makefile
index e74e0dd519..94f84e01eb 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -196,6 +196,8 @@ CFLAGS-support_paths.c = \
-DROOTSBINDIR_PATH=\"$(rootsbindir)\" \
-DCOMPLOCALEDIR_PATH=\"$(complocaledir)\"
+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);
--
2.26.2
next prev parent reply other threads:[~2020-07-13 23:30 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-06 14:48 [PATCH v2] Fix time/tst-cpuclock1 intermitent failures Lucas A. M. Magalhaes
2020-02-17 16:44 ` Lucas A. M. Magalhaes
2020-02-18 12:44 ` Adhemerval Zanella
2020-02-19 16:42 ` Lucas A. M. Magalhaes
2020-02-19 18:51 ` Adhemerval Zanella
2020-02-20 18:17 ` [PATCH v3] " Lucas A. M. Magalhaes
2020-03-04 19:24 ` Matheus Castanho
2020-03-06 17:31 ` Lucas A. M. Magalhaes
2020-03-10 16:20 ` [PATCH v4] " Lucas A. M. Magalhaes
2020-03-10 16:30 ` Andreas Schwab
2020-03-10 17:45 ` Carlos O'Donell
2020-03-23 17:20 ` [PATCH v5] " Lucas A. M. Magalhaes
2020-03-23 21:06 ` Carlos O'Donell
2020-03-24 19:42 ` Lucas A. M. Magalhaes
2020-03-31 18:55 ` Carlos O'Donell
2020-03-31 11:34 ` [PATCH v6] " Lucas A. M. Magalhaes
2020-03-31 19:02 ` Carlos O'Donell
2020-04-03 19:24 ` [PATCH v7] " Lucas A. M. Magalhaes
2020-04-03 20:48 ` Carlos O'Donell
2020-04-07 13:59 ` [PATCH v8] " Lucas A. M. Magalhaes
2020-04-16 17:30 ` Lucas A. M. Magalhaes
2020-04-16 19:21 ` Carlos O'Donell
2020-04-21 17:44 ` [PATCH v9] " Lucas A. M. Magalhaes
2020-05-11 17:41 ` Lucas A. M. Magalhaes
2020-05-25 11:46 ` Lucas A. M. Magalhaes
2020-06-08 13:58 ` Lucas A. M. Magalhaes
2020-06-08 16:52 ` Carlos O'Donell
2020-06-12 15:28 ` [PATCH v10] " Lucas A. M. Magalhaes
2020-06-25 17:26 ` Lucas A. M. Magalhaes
2020-07-06 14:15 ` Lucas A. M. Magalhaes
2020-07-07 20:12 ` Carlos O'Donell
2020-07-10 23:07 ` Tulio Magno Quites Machado Filho
2020-07-11 14:45 ` H.J. Lu
2020-07-11 16:31 ` H.J. Lu
2020-07-13 23:30 ` H.J. Lu [this message]
2020-07-14 2:35 ` [PATCH] Correct timespec implementation [BZ #26232] Carlos O'Donell
2020-07-14 11:16 ` Florian Weimer
2020-07-14 11:42 ` H.J. Lu
2020-07-14 12:04 ` H.J. Lu
2020-07-14 12:18 ` Florian Weimer
2020-07-14 13:12 ` H.J. Lu
2020-07-14 13:14 ` Florian Weimer
2020-07-14 13:17 ` H.J. Lu
2020-07-15 19:38 ` Paul Eggert
2020-07-15 19:44 ` H.J. Lu
2020-07-14 13:08 ` Lucas A. M. Magalhaes
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='CAMe9rOoo-=67Mu+=+frB-2GYX7H=KQdOnjDKo53vNzh4R2HUoA@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=carlos@redhat.com \
--cc=lamm@linux.ibm.com \
--cc=libc-alpha@sourceware.org \
--cc=tuliom@ascii.art.br \
/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).