From patchwork Mon Jul 13 23:30:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 40092 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C9C0E3857036; Mon, 13 Jul 2020 23:30:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C9C0E3857036 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1594683058; bh=8kA+gZq9IYpxnOWDEgnTred/9H9o74XNzB5Stx5Q+V8=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ndPli0eRUHtXOcf393Xz+ZKXT5PmC/tX9g56dHZcEZGBLmi3h969cJa5Yx6h27WNU FVrUH5WiHQy3iPa2DMFqMXPAYr0XNWNaaZekfRN45NEPoNniO0nlxbqv6qOIWgsYC3 BnI5pYRRaYsKCgkGzoo5/PmuYHXyI1iAp5X3Xxtk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) by sourceware.org (Postfix) with ESMTPS id E16C53858D37 for ; Mon, 13 Jul 2020 23:30:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E16C53858D37 Received: by mail-io1-xd42.google.com with SMTP id p205so6852046iod.8 for ; Mon, 13 Jul 2020 16:30:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8kA+gZq9IYpxnOWDEgnTred/9H9o74XNzB5Stx5Q+V8=; b=lHKe5uNwHyMPZrI2fKZtRbGyMXxdC8n/PoieuPupIjkyusDamI8589abk5l1yn6dwH IcoXUQJZTxCileZxcMaO2njYzNF//gmkrvB6c/kJNJqVgCIkgrrm3wfBqf9G5bMqDGIm 41OR/Yb1yGlQXE/Wz/HAwXWNj4xm+q1I+LdQ9mgykjpNHgX820kOFOSm5D775ovel9XI U0xdig7UYpob0grC0mMGW5afOczq1mldZID2vx7M3d7VNBQPEq7klHPtNbwfP3Y6LtHS bd1axoackRFEsZs+Eo+p/2+P58nBFEHoS89RDWVbTB8JFIIl44IJAvhasZno6VNvlVJ9 SSdQ== X-Gm-Message-State: AOAM532kvGFmXG0X8tNHXcubwysv3zaO4lIqH9HNIp1nBJGJr3Z2EPm8 3DW2C1yffAf+XIUPq/2lPYXDtssuIkzaEJWpawM= X-Google-Smtp-Source: ABdhPJzNFzX1I5vzAlSf6SxutkHtM0ODmWXWr3KjvrHOJqadOUy/NLFVpeFEC0WC0I5t7e/TPZuKEjjs+DOZ4uCk1XU= X-Received: by 2002:a02:5b83:: with SMTP id g125mr2811179jab.91.1594683054290; Mon, 13 Jul 2020 16:30:54 -0700 (PDT) MIME-Version: 1.0 References: <20200421174456.28663-1-lamm@linux.ibm.com> <20200612152817.1347220-1-lamm@linux.ibm.com> <875zavezq6.fsf@linux.ibm.com> In-Reply-To: Date: Mon, 13 Jul 2020 16:30:18 -0700 Message-ID: Subject: [PATCH] Correct timespec implementation [BZ #26232] To: Tulio Magno Quites Machado Filho X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: "H.J. Lu via Libc-alpha" From: "H.J. Lu" Reply-To: "H.J. Lu" Cc: GNU C Library Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Sat, Jul 11, 2020 at 9:31 AM H.J. Lu wrote: > > On Sat, Jul 11, 2020 at 7:45 AM H.J. Lu wrote: > > > > On Fri, Jul 10, 2020 at 4:10 PM Tulio Magno Quites Machado Filho > > wrote: > > > > > > Carlos O'Donell via Libc-alpha 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 > > > > > > 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 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. From 3a3003d3a429035d7cfb8485c64a22cb9517ec48 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 13 Jul 2020 16:15:56 -0700 Subject: [PATCH] Correct timespec implementation [BZ #26232] commit 04deeaa9ea74b0679dfc9d9155a37b6425f19a9f Author: Lucas A. M. Magalhaes 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 #include #include +#include #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