From db9fa8816056b367a5d97f57c2498241aaef974a 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. --- 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 4ddb5463d0f..5ef1a5ca25a 100644 --- a/platform/uwp/os_uwp.cpp +++ b/platform/uwp/os_uwp.cpp @@ -575,11 +575,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 f78c87be93b..c32e50e4729 100644 --- a/platform/windows/os_windows.cpp +++ b/platform/windows/os_windows.cpp @@ -418,12 +418,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;