From b330766c30df81def90969c954871b7da5beecd8 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Fri, 22 May 2020 12:20:19 +0100 Subject: [PATCH] Fix overflow condition with QueryPerformanceCounter The previous code for OS_Windows::get_ticks_usec() multiplied the tick count by 1000000 before dividing by ticks_per_second. The ticks is counted in a 64 bit integer and is susceptible to overflow when a machine has been running for a long period of time (days) with a high frequency timer. This PR separates the overall calculation into one for seconds and one for the remainder, removing the possibility of overflow due to the multiplier. (cherry picked from commit db9fa8816056b367a5d97f57c2498241aaef974a) --- platform/uwp/os_uwp.cpp | 21 +++++++++++++++++++-- platform/windows/os_windows.cpp | 21 +++++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/platform/uwp/os_uwp.cpp b/platform/uwp/os_uwp.cpp index 90c1697e1c5..35838acef1b 100644 --- a/platform/uwp/os_uwp.cpp +++ b/platform/uwp/os_uwp.cpp @@ -618,11 +618,28 @@ void OS_UWP::delay_usec(uint32_t p_usec) const { uint64_t OS_UWP::get_ticks_usec() const { uint64_t ticks; - uint64_t time; + // This is the number of clock ticks since start QueryPerformanceCounter((LARGE_INTEGER *)&ticks); + // Divide by frequency to get the time in seconds - time = ticks * 1000000L / ticks_per_second; + // original calculation shown below is subject to overflow + // with high ticks_per_second and a number of days since the last reboot. + // time = ticks * 1000000L / ticks_per_second; + + // we can prevent this by either using 128 bit math + // or separating into a calculation for seconds, and the fraction + uint64_t seconds = ticks / ticks_per_second; + + // compiler will optimize these two into one divide + uint64_t leftover = ticks % ticks_per_second; + + // remainder + uint64_t time = (leftover * 1000000L) / ticks_per_second; + + // seconds + time += seconds * 1000000L; + // Subtract the time at game start to get // the time since the game started time -= ticks_start; diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp index 596acf1f71b..a8879cf4888 100755 --- a/platform/windows/os_windows.cpp +++ b/platform/windows/os_windows.cpp @@ -2538,12 +2538,29 @@ void OS_Windows::delay_usec(uint32_t p_usec) const { uint64_t OS_Windows::get_ticks_usec() const { uint64_t ticks; - uint64_t time; + // This is the number of clock ticks since start if (!QueryPerformanceCounter((LARGE_INTEGER *)&ticks)) ticks = (UINT64)timeGetTime(); + // Divide by frequency to get the time in seconds - time = ticks * 1000000L / ticks_per_second; + // original calculation shown below is subject to overflow + // with high ticks_per_second and a number of days since the last reboot. + // time = ticks * 1000000L / ticks_per_second; + + // we can prevent this by either using 128 bit math + // or separating into a calculation for seconds, and the fraction + uint64_t seconds = ticks / ticks_per_second; + + // compiler will optimize these two into one divide + uint64_t leftover = ticks % ticks_per_second; + + // remainder + uint64_t time = (leftover * 1000000L) / ticks_per_second; + + // seconds + time += seconds * 1000000L; + // Subtract the time at game start to get // the time since the game started time -= ticks_start;