Modify

Opened 5 years ago

Closed 4 years ago

#8070 closed Bugs (fixed)

prefer GetTickCount64 over GetTickCount

Reported by: alex@… Owned by: viboes
Milestone: Boost 1.56.0 Component: thread
Version: Boost 1.55.0 Severity: Problem
Keywords: Cc:

Description

Visual Studio 2012 static analysis issues a warning C28159 (Consider using another function instead), namely for the three calls to GetTickCount? in thread_data.hpp.

This should be an almost in-place replacement (with the addition that the variables should be ULONGLONG instead of ULONG).

Attachments (1)

gettickcount64.patch (5.1 KB) - added by raad@… 4 years ago.
Determine GetTickCount64 support at runtime

Download all attachments as: .zip

Change History (32)

comment:1 Changed 5 years ago by viboes

Owner: changed from Anthony Williams to viboes
Severity: ProblemCosmetic
Status: newassigned

Could you post the exact compiler report here?

comment:2 Changed 5 years ago by alex@…

Absolutely:

C28159 Consider using another function instead.

Consider using 'GetTickCount64' instead of 'GetTickCount?'.

Reason: GetTickCount? overflows roughly every 49 days. Code that does not take that into account can loop indefinitely. GetTickCount64 operates on 64 bit values and does not have that problem.

comment:3 Changed 5 years ago by viboes

Can we check if this function is available using conditional compilation?

comment:4 Changed 5 years ago by alex@…

According to MSDN, this is available for WINNT >= 0x0600 (aka Vista/Server? 2008)

comment:5 Changed 5 years ago by viboes

Milestone: To Be DeterminedBoost 1.54.0

comment:6 Changed 5 years ago by viboes

Resolution: fixed
Status: assignedclosed

Committed revision [83525].

comment:7 Changed 4 years ago by anonymous

Boost 1.54.0: This is breaking Windows XP compatibility for me: I'm compiling on Windows 7 with MinGW, but my users are on XP and are now seeing" Procedure Entry Point Not Found in Kernel32.dll". I have to revert back to Boost 1.52.

Regards, Zenju

comment:8 Changed 4 years ago by schorsch_76@…

Same problem here as Zenju,

there seems no compatibility switch in the source. Is there an official statement for boost to not support XP anymore? Have not found anything in the release notes, that XP is no more supported.

Regards, Georg

comment:9 Changed 4 years ago by viboes

Resolution: fixed
Status: closedreopened

Sorry for the disturbance. Please could you comment this line

22 #define BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64

in the following context.

branches/release/boost/thread/win32/thread_primitives.hpp
r81667 	r83525 	 
18	18	#include <algorithm> 
19	19	 
 	20	#ifndef BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 
 	21	#if _WIN32_WINNT >= 0x0600 
 	22	#define BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 
 	23	#endif 
 	24	#endif 

Could someone try using a more restrictive check than

_WIN32_WINNT >= 0x0600

?

Maybe something specific to Vista?

comment:10 Changed 4 years ago by viboes

Committed revision [84946]. Rollback.

comment:11 Changed 4 years ago by anonymous

Thanks for the quick response! After commenting out BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 and recompiling Boost 1.54, my binaries are running fine on XP again! Currently 30% of my users are still running XP so thanks for supporting this not so small scenario!

Regards, Zenju

comment:12 Changed 4 years ago by viboes

Milestone: Boost 1.54.0To Be Determined

comment:13 Changed 4 years ago by viboes

Could someone try if

branches/release/boost/thread/win32/thread_primitives.hpp

#ifndef BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#if _WIN32_WINNT >= 0x0600 && ! defined _WIN32_WINNT_WS08
#define BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#endif
#endif

works for Windows XP?

Last edited 4 years ago by viboes (previous) (diff)

comment:14 Changed 4 years ago by anonymous

yes it works

comment:15 Changed 4 years ago by viboes

Milestone: To Be DeterminedBoost 1.55.0

Committed revision [85701].

Version 0, edited 4 years ago by viboes (next)

comment:16 Changed 4 years ago by ixSci <beholder@…>

Just tested on Win8.1(boost rev. 85771) and it doesn't compile: libs\thread\src\win32\thread.cpp(433) : error C2653: 'win32' : is not a class or namespace name I believe it should be: unsigned long const elapsed_milliseconds=detail::win32::GetTickCount?()-target_time.start;

comment:17 Changed 4 years ago by viboes

My bad :( Committed revision [85772].

comment:18 Changed 4 years ago by viboes

Committed revision [85815].

comment:19 Changed 4 years ago by viboes

Resolution: fixed
Status: reopenedclosed

comment:20 Changed 4 years ago by anonymous

Resolution: fixed
Severity: CosmeticProblem
Status: closedreopened
Version: Boost 1.52.0Boost 1.55.0

Hi,

the "Procedure Entry Point Not Found" for GetTickCount64 is back or I should better say was not resolved in first place. The

#ifndef BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#if _WIN32_WINNT >= 0x0600 && ! defined _WIN32_WINNT_WS08
#define BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#endif
#endif

check always enables the GetTickCount64, because _WIN32_WINNT is usually set to 0x0600. This value for _WIN32_WINNT OTOH is *always* required, even when targetting XP in order to have access to Vista and later Win32 structs when the very same executable is supposed to run on later versions of Windows. Commenting out BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 again fixes the issue, but I would very much prefer to use a vanilla version of boost without requiring manual source changes before I can use.

Regards, Zenju

comment:21 in reply to:  20 Changed 4 years ago by viboes

Replying to anonymous:

Hi,

the "Procedure Entry Point Not Found" for GetTickCount64 is back or I should better say was not resolved in first place. The

#ifndef BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#if _WIN32_WINNT >= 0x0600 && ! defined _WIN32_WINNT_WS08
#define BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64
#endif
#endif

check always enables the GetTickCount64, because _WIN32_WINNT is usually set to 0x0600. This value for _WIN32_WINNT OTOH is *always* required, even when targetting XP in order to have access to Vista and later Win32 structs when the very same executable is supposed to run on later versions of Windows.

Is _WIN32_WINNT_WS08 defined?

comment:22 Changed 4 years ago by viboes

BTW, what are the android.exe?

comment:23 Changed 4 years ago by tcourtney

I have a question about this issue and the proposed solution. We compile our software on Windows 7 but want the binaries to be compatible for all versions of Windows. It seems that any solution for this issue that relies on #ifdef, #define to determine the OS properties is only valid for the OS that the software was compiled on. I don't see how a solution based on static, compile time #ifdef logic could support both GetTickCount? for XP and GetTickCount64 for Windows 7 using the same binary. This #ifdef fix only makes it possible to compile the code on XP or Windows7. But that doesn't fix the issue for my company because we need to release the same binary for XP and Windows 7.

comment:24 in reply to:  23 Changed 4 years ago by viboes

Replying to tcourtney:

I have a question about this issue and the proposed solution. We compile our software on Windows 7 but want the binaries to be compatible for all versions of Windows. It seems that any solution for this issue that relies on #ifdef, #define to determine the OS properties is only valid for the OS that the software was compiled on. I don't see how a solution based on static, compile time #ifdef logic could support both GetTickCount? for XP and GetTickCount64 for Windows 7 using the same binary. This #ifdef fix only makes it possible to compile the code on XP or Windows7. But that doesn't fix the issue for my company because we need to release the same binary for XP and Windows 7.

And you propose ...

comment:25 Changed 4 years ago by raad@…

_WIN32_WINNT_WS08 is always defined if the windows.h from at least the Windows SDK 6.0 is included before Boost.Thread, so the value of BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 can now vary in different source files depending on include order, which leads to serious bugs.

_WIN32_WINNT is the lowest Windows version the software is supposed to run on, so defining it as 0x0600 while still wanting to support Windows XP is incorrect and boost users should fix their code. Boost.Thread is by far not the only library that would drop support for Windows XP when _WIN32_WINNT is defined as 0x0600.

So if you want a static check for the minimum required Windows version at compile time, the check for (_WIN32_WINNT >= 0x0600) was absolutely correct. If you want a dynamic check at runtime, you need to call GetProcAddress?.

comment:26 in reply to:  25 Changed 4 years ago by viboes

Replying to raad@…:

_WIN32_WINNT_WS08 is always defined if the windows.h from at least the Windows SDK 6.0 is included before Boost.Thread, so the value of BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64 can now vary in different source files depending on include order, which leads to serious bugs.

_WIN32_WINNT is the lowest Windows version the software is supposed to run on, so defining it as 0x0600 while still wanting to support Windows XP is incorrect and boost users should fix their code. Boost.Thread is by far not the only library that would drop support for Windows XP when _WIN32_WINNT is defined as 0x0600.

So if you want a static check for the minimum required Windows version at compile time, the check for (_WIN32_WINNT >= 0x0600) was absolutely correct. If you want a dynamic check at runtime, you need to call GetProcAddress?.

I have no access to a Windows machine now. I would be grateful of could you provide a patch that works for any windows release.

comment:27 Changed 4 years ago by viboes

Milestone: Boost 1.55.0To Be Determined

Changed 4 years ago by raad@…

Attachment: gettickcount64.patch added

Determine GetTickCount64 support at runtime

comment:28 Changed 4 years ago by raad@…

Please find attached a patch with a run-time check for GetTickCount64 availability. Please note that ticks_type is always 64-bit with this patch and I don't think it's possible to implement this header-only without a huge run-time overhead, so there is a new cpp file.

Alternatively, for the correct compile-time check for the target Windows version, you could just revert commit 3ac48bdd65c4713438779e65a3f5ee4324a03b55 and add a macro like BOOST_THREAD_WIN32_DONT_USE_GET_TICK_COUNT_64 for those who are not willing to set their target Windows version correctly via _WIN32_WINNT. Or only let the user set BOOST_THREAD_WIN32_HAS_GET_TICK_COUNT_64.

comment:29 Changed 4 years ago by viboes

Thanks for the patch. I will take a deeper look as soon as I reinstall all the needed tools on my new Windows machine.

comment:31 Changed 4 years ago by viboes

Resolution: fixed
Status: reopenedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain viboes.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.