Subject: bug#77535: timeout treats very short durations as `0`

From: Paul Eggert <[email-masked]>
Date: Sat, 5 Apr 2025 01:03:48 -0700

On 2025-04-04 12:27, Pádraig Brady wrote:
> +  /* Clamp underflow to 1ns, as 0 disables the timeout.  */
> +  if (duration == 0 && errno == ERANGE)
> +    duration = 1e-9;

That isn't exactly right as the 1e-9 double-rounds to 2e-9 when we 
compute the struct timespec. Also, even with the patch the code 
mishandles 16777216.000000001 (2**24 + 10**-9) by treating it as if it 
were just 16777216. These are tiny bugs, I know, but I took up the 
challenge of doing this things more correctly by installing the attached.

From a3b862ece2aa04d5c0b5e93fec8e66e45b9b3560 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Apr 2025 00:58:22 -0700
Subject: [PATCH] timeout: round timeouts up
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This handles timeouts like 16777216.000000001 correctly;
formerly the subsecond part of that timeout was ignored.
* bootstrap.conf (gnulib_modules): Add fenv-rounding, signbit.
* src/local.mk (src_timeout_LDADD): Append $(FENV_ROUNDING_LIBM).
* src/timeout.c: Include fenv.h, math.h.
Don’t include xstrtod.h, as xstrtod’s checking now gets in the way.
(parse_duration): Round up when calling cl_strtod.
Check for -1e-1000.  Don’t double-round 1e-9.
* tests/timeout/timeout-parameters.sh: Test for -0.1,
-1e-1000, 1e-1000.
---
 bootstrap.conf                      |  2 ++
 src/local.mk                        |  3 +++
 src/timeout.c                       | 39 ++++++++++++++++++++++++-----
 tests/timeout/timeout-parameters.sh |  7 +++++-
 4 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index c99838e95..3c133ef71 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -99,6 +99,7 @@ gnulib_modules="
   fdatasync
   fdopen
   fdutimensat
+  fenv-rounding
   file-has-acl
   file-type
   fileblocks
@@ -242,6 +243,7 @@ gnulib_modules="
   settime
   sig2str
   sigaction
+  signbit
   skipchars
   smack
   ssize_t
diff --git a/src/local.mk b/src/local.mk
index fd9dc81c2..0a4e3af43 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -257,6 +257,9 @@ src_stat_LDADD += $(LIB_SELINUX)
 # for nvlist_lookup_uint64_array
 src_stat_LDADD += $(LIB_NVPAIR)
 
+# for fegetround, fesetround
+src_timeout_LDADD += $(FENV_ROUNDING_LIBM)
+
 # for gettime, settime, tempname, utimecmp, utimens
 copy_ldadd += $(CLOCK_TIME_LIB)
 src_date_LDADD += $(CLOCK_TIME_LIB)
diff --git a/src/timeout.c b/src/timeout.c
index 6756cd888..5947c1942 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -45,7 +45,9 @@
    Written by Pádraig Brady.  */
 
 #include <config.h>
+#include <fenv.h>
 #include <getopt.h>
+#include <math.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <signal.h>
@@ -56,7 +58,6 @@
 
 #include "system.h"
 #include "cl-strtod.h"
-#include "xstrtod.h"
 #include "sig2str.h"
 #include "operand2sig.h"
 #include "quote.h"
@@ -356,12 +357,29 @@ apply_time_suffix (double *x, char suffix_char)
 static double
 parse_duration (char const *str)
 {
-  double duration;
-  char const *ep;
+  /* If possible tell strtod to round up, so that we always wait at
+     least as long as STR specifies.  Although Standard C requires
+     FENV_ACCESS ON, don't bother if using GCC as it warns.  */
+#ifdef FE_UPWARD
+# if !defined __GNUC__ && 199901 <= __STDC_VERSION__
+#  pragma STDC FENV_ACCESS ON
+# endif
+  int round = fegetround ();
+  fesetround (FE_UPWARD);
+#endif
+
+  char *ep;
+  double duration = cl_strtod (str, &ep);
+
+#ifdef FE_UPWARD
+  fesetround (round);
+#endif
 
-  if (! (xstrtod (str, &ep, &duration, cl_strtod) || errno == ERANGE)
+  if (ep == str
       /* Nonnegative interval.  */
       || ! (0 <= duration)
+      /* The interval did not underflow to -0.  */
+      || (signbit (duration) && errno == ERANGE)
       /* No extra chars after the number and an optional s,m,h,d char.  */
       || (*ep && *(ep + 1))
       /* Check any suffix char and update timeout based on the suffix.  */
@@ -371,9 +389,18 @@ parse_duration (char const *str)
       usage (EXIT_CANCELED);
     }
 
-  /* Clamp underflow to 1ns, as 0 disables the timeout.  */
+  /* Do not let the duration underflow to 0, as 0 disables the timeout.
+     Use 2**-30 instead of 0; settimeout will round it up to 1 ns,
+     whereas 1e-9 might double-round to 2 ns.
+
+     If FE_UPWARD is defined, this code is not needed on glibc as its
+     strtod rounds upward correctly.  Although this code might not be
+     needed on non-glibc platforms too, it's too much trouble to
+     worry about that.  */
+#if !defined FE_UPWARD || !defined __GLIBC__
   if (duration == 0 && errno == ERANGE)
-    duration = 1e-9;
+    duration = 9.313225746154785e-10;
+#endif
 
   return duration;
 }
diff --git a/tests/timeout/timeout-parameters.sh b/tests/timeout/timeout-parameters.sh
index 84b450e90..408846262 100755
--- a/tests/timeout/timeout-parameters.sh
+++ b/tests/timeout/timeout-parameters.sh
@@ -23,8 +23,10 @@ getlimits_
 
 # internal errors are 125, distinct from execution failure
 
-# invalid timeout
+# invalid timeouts
 returns_ 125 timeout invalid sleep 0 || fail=1
+returns_ 125 timeout ' -0.1' sleep 0 || fail=1
+returns_ 125 timeout ' -1e-10000' sleep 0 || fail=1
 
 # invalid kill delay
 returns_ 125 timeout --kill-after=invalid 1 sleep 0 || fail=1
@@ -38,6 +40,9 @@ timeout 10.34 sleep 0 || fail=1
 # nanoseconds potentially supported
 timeout 9.999999999 sleep 0 || fail=1
 
+# round underflow up to 1 ns
+returns_ 124 timeout 1e-10000 sleep 1 || fail
+
 # invalid signal spec
 returns_ 125 timeout --signal=invalid 1 sleep 0 || fail=1
 
-- 
2.45.2