From 2b72887ac834b0f0f675baff1af90771c7e36c87 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Fri, 30 Nov 2018 22:39:57 +0100 Subject: [PATCH] Cygwin: clocks: fix a hang on pre-Windows 10 machines when calling clocks too early in DLL init, the vtables are not correctly set up for some reason. Calls to init() from now() fail because the init pointer in the vtable is NULL. Real life example is mintty which runs into a minor problem at startup, triggering a system_printf call. Strace is another problem, it's called the first time prior to any class initialization. Workaround is to make sure that no virtual methods are called in an early stage. Make init() non-virtual and convert resolution() to a virtual method instead. Add a special non-virtual clk_monotonic_t::strace_usecs. While at it: - Inline internal-only methods. - Drop the `inited' member. Convert period/ticks_per_sec toa union. Initialize period/ticks_per_sec via InterlockeExchange64. - Fix GetTickCount64 usage. No, it's not returning ticks but milliseconds since boot (unbiased). - Fix comment indentation. Signed-off-by: Corinna Vinschen --- winsup/cygwin/clock.cc | 73 ++++++++++++++++++++++------------------- winsup/cygwin/clock.h | 40 +++++++++++++++------- winsup/cygwin/strace.cc | 8 ++--- 3 files changed, 71 insertions(+), 50 deletions(-) diff --git a/winsup/cygwin/clock.cc b/winsup/cygwin/clock.cc index 0c8390a49..18d628884 100644 --- a/winsup/cygwin/clock.cc +++ b/winsup/cygwin/clock.cc @@ -5,7 +5,7 @@ #include "miscfuncs.h" #include "spinlock.h" -static LONGLONG +static inline LONGLONG system_qpc_tickspersec () { LARGE_INTEGER qpf; @@ -15,7 +15,7 @@ system_qpc_tickspersec () return qpf.QuadPart; } -static LONGLONG +static inline LONGLONG system_tickcount_period () { ULONG coarsest = 0, finest, actual; @@ -23,40 +23,37 @@ system_tickcount_period () if (!coarsest) { /* The actual resolution of the OS timer is a system-wide setting which - can be changed any time, by any process. The only fixed value we - can rely on is the coarsest value. */ + can be changed any time, by any process. The only fixed value we + can rely on is the coarsest value. */ NtQueryTimerResolution (&coarsest, &finest, &actual); } return coarsest; } -void +void inline clk_t::init () { - spinlock spin (inited, 1); - if (!spin) - period = system_tickcount_period (); + if (!period) + InterlockedExchange64 (&period, system_tickcount_period ()); } -void +void inline clk_realtime_t::init () { - spinlock spin (inited, 1); - if (!spin) + if (wincap.has_precise_system_time ()) { - if (wincap.has_precise_system_time ()) - ticks_per_sec = system_qpc_tickspersec (); - else - period = system_tickcount_period (); + if (!ticks_per_sec) + InterlockedExchange64 (&ticks_per_sec, system_qpc_tickspersec ()); } + else if (!period) + InterlockedExchange64 (&period, system_tickcount_period ()); } -void +void inline clk_monotonic_t::init () { - spinlock spin (inited, 1); - if (!spin) - ticks_per_sec = system_qpc_tickspersec (); + if (!ticks_per_sec) + InterlockedExchange64 (&ticks_per_sec, system_qpc_tickspersec ()); } int @@ -169,8 +166,7 @@ clk_monotonic_t::now (clockid_t clockid, struct timespec *ts) LARGE_INTEGER now; struct timespec bts; - if (inited <= 0) - init (); + init (); do { bias = SharedUserData.InterruptTimeBias; @@ -209,13 +205,10 @@ clk_monotonic_coarse_t::now (clockid_t clockid, struct timespec *ts) /* Vista-only: GetTickCount64 is biased but it's coarse and monotonic. */ ULONGLONG now; - if (inited <= 0) - init (); - now = GetTickCount64 (); - now *= period; /* Normalize to 100ns period */ - ts->tv_sec = now / NS100PERSEC; - now %= NS100PERSEC; - ts->tv_nsec = now * (NSPERSEC/NS100PERSEC); + now = GetTickCount64 (); /* Returns ms since boot */ + ts->tv_sec = now / MSPERSEC; + now %= MSPERSEC; + ts->tv_nsec = now * (NSPERSEC/MSPERSEC); } return 0; } @@ -237,8 +230,7 @@ clk_boottime_t::now (clockid_t clockid, struct timespec *ts) { LARGE_INTEGER now; - if (inited <= 0) - init (); + init (); QueryPerformanceCounter (&now); ts->tv_sec = now.QuadPart / ticks_per_sec; now.QuadPart %= ticks_per_sec; @@ -250,15 +242,30 @@ clk_boottime_t::now (clockid_t clockid, struct timespec *ts) void clk_t::resolution (struct timespec *ts) { - if (inited <= 0) - init (); + init (); ts->tv_sec = 0; - if (ticks_per_sec) + ts->tv_nsec = period * (NSPERSEC/NS100PERSEC); +} + +void +clk_realtime_t::resolution (struct timespec *ts) +{ + init (); + ts->tv_sec = 0; + if (wincap.has_precise_system_time ()) ts->tv_nsec = NSPERSEC / ticks_per_sec; else ts->tv_nsec = period * (NSPERSEC/NS100PERSEC); } +void +clk_monotonic_t::resolution (struct timespec *ts) +{ + init (); + ts->tv_sec = 0; + ts->tv_nsec = NSPERSEC / ticks_per_sec; +} + static clk_realtime_coarse_t clk_realtime_coarse; static clk_realtime_t clk_realtime; static clk_process_t clk_process; diff --git a/winsup/cygwin/clock.h b/winsup/cygwin/clock.h index 075aaed1d..c05bf477c 100644 --- a/winsup/cygwin/clock.h +++ b/winsup/cygwin/clock.h @@ -52,13 +52,15 @@ details. */ class clk_t { protected: - LONG inited; /* Some values are returned as ticks/s, some as 100ns period of a single tick. Store the original value and use a computation method making the most sense for the value given, to avoid rounding issues. */ - LONGLONG ticks_per_sec; - LONGLONG period; - virtual void init (); + union + { + LONGLONG ticks_per_sec; + LONGLONG period; + }; + void init (); virtual int now (clockid_t, struct timespec *) = 0; public: @@ -66,32 +68,35 @@ class clk_t { return now (_id, ts); } - void resolution (struct timespec *); + virtual void resolution (struct timespec *); /* shortcuts for non-process/thread clocks */ - void nsecs (struct timespec *ts) { nsecs (0, ts); } + void nsecs (struct timespec *ts) + { + now (0, ts); + } ULONGLONG nsecs () { struct timespec ts; - nsecs (&ts); + now (0, &ts); return (ULONGLONG) ts.tv_sec * NSPERSEC + ts.tv_nsec; } LONGLONG n100secs () { struct timespec ts; - nsecs (&ts); + now (0, &ts); return ts.tv_sec * NS100PERSEC + ts.tv_nsec / (NSPERSEC/NS100PERSEC); } LONGLONG usecs () { struct timespec ts; - nsecs (&ts); + now (0, &ts); return ts.tv_sec * USPERSEC + ts.tv_nsec / (NSPERSEC/USPERSEC); } LONGLONG msecs () { struct timespec ts; - nsecs (&ts); + now (0, &ts); return ts.tv_sec * MSPERSEC + ts.tv_nsec / (NSPERSEC/MSPERSEC); } }; @@ -103,8 +108,10 @@ class clk_realtime_coarse_t : public clk_t class clk_realtime_t : public clk_t { - virtual void init (); + void init (); virtual int now (clockid_t, struct timespec *); + public: + virtual void resolution (struct timespec *); }; class clk_process_t : public clk_t @@ -120,9 +127,18 @@ class clk_thread_t : public clk_t class clk_monotonic_t : public clk_t { protected: - virtual void init (); + void init (); private: virtual int now (clockid_t, struct timespec *); + public: + virtual void resolution (struct timespec *); + /* Under strace 1st call is so early that vtable is NULL. */ + LONGLONG strace_usecs () + { + struct timespec ts; + clk_monotonic_t::now (0, &ts); + return ts.tv_sec * USPERSEC + ts.tv_nsec / (NSPERSEC/USPERSEC); + } }; class clk_monotonic_coarse_t : public clk_t diff --git a/winsup/cygwin/strace.cc b/winsup/cygwin/strace.cc index 21c2d6d4a..35f8a59ae 100644 --- a/winsup/cygwin/strace.cc +++ b/winsup/cygwin/strace.cc @@ -82,14 +82,12 @@ strace::dll_info () int strace::microseconds () { - /* Need a local clock instance because this function is called before - the global constructors of the inferior process have been called. */ - static clk_monotonic_t clock_monotonic; static LONGLONG process_start NO_COPY; + clk_monotonic_t *clk = (clk_monotonic_t *) get_clock (CLOCK_MONOTONIC); if (!process_start) - process_start = clock_monotonic.usecs (); - return (int) (clock_monotonic.usecs () - process_start); + process_start = clk->strace_usecs (); + return (int) (clk->strace_usecs () - process_start); } static int __stdcall