[Git][NTPsec/ntpsec][master] Partially address Gitlab issue #270: Loss of precision in step_systime()
Eric S. Raymond
gitlab at mg.gitlab.com
Thu Aug 17 13:37:41 UTC 2017
Eric S. Raymond pushed to branch master at NTPsec / ntpsec
Commits:
3705a499 by Eric S. Raymond at 2017-08-17T09:32:38-04:00
Partially address Gitlab issue #270: Loss of precision in step_systime()
Change system residual and time-step variable to a new doubletime_t type
(long double) so these variables can handle the full range of l_fp 64-bit
time values. See the comment on the doubletime_t typedef in
include/ntp_types.h for extended discussion.
- - - - -
5 changed files:
- include/ntp_fp.h
- include/ntp_types.h
- libntp/pymodule.c
- libntp/systime.c
- ntpfrob/jitter.c
Changes:
=====================================
include/ntp_fp.h
=====================================
--- a/include/ntp_fp.h
+++ b/include/ntp_fp.h
@@ -133,27 +133,23 @@ typedef struct {
*/
#define FRAC 4294967296.0 /* 2^32 as a double */
-#include <math.h> /* ldexp() */
+#include <math.h> /* ldexpl() */
-static inline l_fp dtolfp(double d)
-/* double to l_fp
+static inline l_fp dtolfp(doubletime_t d)
+/* long double to l_fp
* assumes signed l_fp, i.e. a time offset
* undefined return if d in NaN
*/
{
-/* Please check issue 264 before changing this to use llround
- * https://gitlab.com/NTPsec/ntpsec/issues/264
- * llround is broken on NetBSD, 2017-May-05
- */
- return (l_fp)(int64_t)(ldexp(d, 32));
+ return (l_fp)(int64_t)(ldexpl(d, 32));
}
-static inline double lfptod(l_fp r)
-/* l_fp to double
+static inline doubletime_t lfptod(l_fp r)
+/* l_fp to long double
* assumes signed l_fp, i.e. a time offset
*/
{
- return ldexp((double)((int64_t)r), -32);
+ return ldexpl((double)((int64_t)r), -32);
}
/*
@@ -171,7 +167,7 @@ extern char * rfc3339time (time_t);
extern void set_sys_fuzz (double);
extern void get_systime (l_fp *);
-extern bool step_systime (double, int (*settime)(struct timespec *));
+extern bool step_systime (doubletime_t, int (*settime)(struct timespec *));
extern bool adj_systime (double, int (*adjtime)(const struct timeval *, struct timeval *));
#define lfptoa(fpv, ndec) mfptoa((fpv), (ndec))
=====================================
include/ntp_types.h
=====================================
--- a/include/ntp_types.h
+++ b/include/ntp_types.h
@@ -63,6 +63,27 @@ typedef uint32_t keyid_t; /* cryptographic key ID */
#define KEYID_T_MAX (0xffffffff)
/*
+ * Ordinary double has only 53 bits of precision in IEEE754. But l_fp
+ * needs 64 bits of precision, arguably 65 bits after 2026. Thus, to have
+ * a float type coextensive with it, we need one with at least 65 bits of
+ * precision.
+ *
+ * The C standard does not specify the precision of a double. C99
+ * Annex F makes IEEE754 compliance optional and very few C compilers
+ * are fully IEEE754 compliant. C doubles may be may be 24 bits, 53
+ * bits, or something else. Only rarely would a C double be able to
+ * hold a 65 bit number without loss of precision.
+ *
+ * This matters because initial steps may be large, such as when a host
+ * has no valid RTC and thinks the current time is 1Jan70. Thus truncating
+ * an l_fp conversion to double after 2026 risks loss of precision.
+ *
+ * On the other hand, long double is usually at least 80 bits.
+ * See https://en.wikipedia.org/wiki/Long_double for discussion and caveats.
+ */
+typedef long double doubletime_t;
+
+/*
* Cloning malloc()'s behavior of always returning pointers suitably
* aligned for the strictest alignment requirement of any type is not
* easy to do portably, as the maximum alignment required is not
=====================================
libntp/pymodule.c
=====================================
--- a/libntp/pymodule.c
+++ b/libntp/pymodule.c
@@ -127,11 +127,19 @@ static PyObject *
ntpc_step_systime(PyObject *self, PyObject *args)
{
double adjustment;
+ doubletime_t full_adjustment;
UNUSED_ARG(self);
+ /*
+ * What we really want is for Python to parse a long double.
+ * As this is, it's a potential source of problems in the Python
+ * utilties if and when the time difference between the Unix epoch
+ * and now exceeds the range of a double.
+ */
if (!PyArg_ParseTuple(args, "d", &adjustment))
return NULL;
- return Py_BuildValue("d", step_systime(adjustment, ntp_set_tod));
+ full_adjustment = adjustment;
+ return Py_BuildValue("d", step_systime(full_adjustment, ntp_set_tod));
}
int32_t ntp_random(void)
=====================================
libntp/systime.c
=====================================
--- a/libntp/systime.c
+++ b/libntp/systime.c
@@ -65,8 +65,8 @@ double sys_fuzz = 0; /* min. time to read the clock (s) */
bool trunc_os_clock; /* sys_tick > measured_tick */
time_stepped_callback step_callback;
-static double sys_residual = 0; /* adjustment residue (s) */
-static long sys_fuzz_nsec = 0; /* min. time to read the clock (ns) */
+static doubletime_t sys_residual = 0; /* adjustment residue (s) */
+static long sys_fuzz_nsec = 0; /* minimum time to read clock (ns) */
/* perlinger at ntp.org: As 'get_systime()' does its own check for clock
* backstepping, this could probably become a local variable in
@@ -236,7 +236,7 @@ adj_systime(
struct timeval adjtv; /* new adjustment */
struct timeval oadjtv; /* residual adjustment */
double quant; /* quantize to multiples of */
- double dtemp;
+ doubletime_t dtemp;
long ticks;
bool isneg = false;
@@ -315,7 +315,7 @@ adj_systime(
bool
step_systime(
- double step,
+ doubletime_t step,
int (*settime)(struct timespec *)
)
{
=====================================
ntpfrob/jitter.c
=====================================
--- a/ntpfrob/jitter.c
+++ b/ntpfrob/jitter.c
@@ -25,8 +25,8 @@
#define NBUF 800002
#define NSAMPLES 10
-static double sys_residual;
-static double average;
+static doubletime_t sys_residual;
+static doubletime_t average;
/*
* get_clocktime - return system time in NTP timestamp format.
@@ -36,7 +36,7 @@ get_clocktime(
l_fp *now /* system time */
)
{
- double dtemp;
+ doubletime_t dtemp;
struct timespec ts; /* seconds and nanoseconds */
@@ -107,7 +107,7 @@ void jitter(const iomode mode)
qsort(gtod, NBUF, sizeof(gtod[0]), doublecmp);
average = average / (NBUF - 2);
if (mode == json) {
- fprintf(stdout, "{\"Average\":%13.9f,", average);
+ fprintf(stdout, "{\"Average\":%13.9Lf,", average);
fprintf(stdout, "\"First rank\":[");
for (i = 0; i < NSAMPLES; i++) {
fprintf(stdout, "%13.9f", gtod[i]);
@@ -125,7 +125,7 @@ void jitter(const iomode mode)
}
else if (mode != raw)
{
- fprintf(stdout, "Average %13.9f\n", average);
+ fprintf(stdout, "Average %13.9Lf\n", average);
fprintf(stdout, "First rank\n");
for (i = 0; i < NSAMPLES; i++)
fprintf(stdout, "%2d %13.9f\n", i, gtod[i]);
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/3705a499961391748c2b2cf1383270924f2f9df9
---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/3705a499961391748c2b2cf1383270924f2f9df9
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20170817/5d45a7ec/attachment.html>
More information about the vc
mailing list