64-bit issues are pretty hard to detect because they are like a timebomb: it may take quite a while before they show up. The PVS-Studio static analyzer makes it easier to find and fix such errors. But we have made even a few more steps forward: we have recently revised with more care the 64-bit diagnostics implemented in our tool, which resulted in changing their distribution among severity levels. In this article, I'm going to tell you about these changes and how it affected the tool handling and bug search. You will also find real-life examples of 64-bit errors.
For a start, I'd like to clarify on this article's contents. Here we will discuss the following topics:
Clause 1 speaks for itself: in this section we will discuss the major changes made in PVS-Studio regarding 64-bit bugs analysis as well as the impact of these changes on the way the user works with the tool.
Section 2 discusses 64-bit bugs found in real-life projects. Besides citing error samples, I will also briefly comment on them so you may learn something new from them.
In Section 3, we will compare the efficiency of diagnostics of these errors by the PVS-Studio analyzer and the means of the Microsoft Visual Studio 2013 IDE. The latter includes both the compiler and the static analyzer coming with this IDE.
Please keep in mind that this article discusses only few of the many bugs found in projects. When working with real code, you will surely get way more of them and they will be more diverse. At the end of the article, there is a list of reference materials for you to learn more about the world of 64-bit errors.
Not so long ago, we revised our 64-bit diagnostic rules with more care and regrouped them according to the severity levels to improve the tool's diagnostic capabilities.
Now the distribution of 64-bit diagnostics across the levels is as follows:
Level 1. Critical errors which are sure to do harm in any application. These, for example, include the bug when a pointer is stored in a 32-bit variable of the int type. When developing a 64-bit program, these first-level warnings must be always paid attention to and fixed.
Level 2. Errors that usually only emerge in applications processing large data arrays. An example of these is using a variable of the 'int' type to index a huge array.
Level 3. All the other bugs. The corresponding warnings are usually irrelevant. However, a few of these diagnostics might be of use in some applications.
So, by enabling message filtering for 64-bit issues of the first level only, you will get a list of diagnostic messages pointing out code fragments that are very probable to contain errors. Don't underestimate these warnings as the consequences of 64-bit bugs may be very different yet often painful and unexpected. It is this type of warnings that we will talk about in this article.
While reading on, I believe, you'll gradually get the idea of how difficult such errors would be to detect without a tool like PVS-Studio.
One should be very careful to use correct data types. So let's start with that.
LRESULT CSaveDlg::OnGraphNotify(WPARAM wParam, LPARAM lParam)
{
LONG evCode, evParam1, evParam2;
while (pME && SUCCEEDED(pME->GetEvent(&evCode,
(LONG_PTR*)&evParam1,
(LONG_PTR*)&evParam2, 0)))
{
....
}
return 0;
}PVS-Studio's diagnostic messages:
To understand what this error is about, note the types of the variables 'evParam1' and 'evParam2' and the declaration of the 'GetEvent' method:
virtual HRESULT STDMETHODCALLTYPE GetEvent(
/* [out] */ __RPC__out long *lEventCode,
/* [out] */ __RPC__out LONG_PTR *lParam1,
/* [out] */ __RPC__out LONG_PTR *lParam2,
/* [in] */ long msTimeout) = 0;As the analyzer's message reads, this code contains a dangerous explicit type conversion. The reason is that the 'LONG_PTR' type is a 'memsize-type' whose size is 32 bits on the Win32 architecture (data model ILP32) and 64 bits on the Win64 one (data model LLP64). At the same time, the 'LONG' type's size is 32 bits on both. Since these types have different sizes on the 64-bit architecture, the program may incorrectly handle objects these pointers refer to.
Going on with dangerous type conversions. Take a look at the following code:
BOOL WINAPI TrackPopupMenu(
_In_ HMENU hMenu,
_In_ UINT uFlags,
_In_ int x,
_In_ int y,
_In_ int nReserved,
_In_ HWND hWnd,
_In_opt_ const RECT *prcRect
);
struct JABBER_LIST_ITEM
{
....
};
INT_PTR CJabberDlgGcJoin::DlgProc(....)
{
....
int res = TrackPopupMenu(
hMenu, TPM_RETURNCMD, rc.left, rc.bottom, 0, m_hwnd, NULL);
....
if (res) {
JABBER_LIST_ITEM *item = (JABBER_LIST_ITEM *)res;
....
}
....
}PVS-Studio's diagnostic message: V204 Explicit conversion from 32-bit integer type to pointer type: (JABBER_LIST_ITEM *) res test.cpp 57
First let's examine the function 'TrackPopupMenu'. It returns the identifier of a menu item selected by the user or a zero value in case of an error or if no selection was made. The 'BOOL' type is obviously a poor choice for this purpose, but let it be.
The return result of this function is stored in the 'res' variable. If the user does select some item (res!=0), then this variable is cast to a pointer to a structure. An interesting approach, but since we are talking about 64-bit errors in this article, let's see how this code will execute on both 32-bit and 64-bit architectures and if there can be any problems about that.
The trouble is that type conversions like that are legal and feasible on the 32-bit architecture because the types 'pointer' and 'BOOL' have the same size. But it will turn into a trap when moving to 64 bits. In Win64 applications, these types are of different sizes (64 bits and 32 bits correspondingly). The potential error here is a probable loss of the most significant bits of the pointer.
Let's go on. The next code fragment:
static int hash_void_ptr(void *ptr)
{
int hash;
int i;
hash = 0;
for (i = 0; i < (int)sizeof(ptr) * 8 / TABLE_BITS; i++)
{
hash ^= (unsigned long)ptr >> i * 8;
hash += i * 17;
hash &= TABLE_MASK;
}
return hash;
}PVS-Studio's diagnostic message: V205 Explicit conversion of pointer type to 32-bit integer type: (unsigned long) ptr test.cpp 76
Let's figure out the issue with casting a variable of the 'void*' type to 'unsigned long' in this function. As I already said, these types have different sizes in the LLP64 data model: 'void*' is 64 bits and 'unsigned long' is 32 bits. It will result in the most significant bits of the value stored in the 'ptr' variable to be truncated (lost). At the same time, the 'i' variable increments with every loop iteration, and the bit-by-bit shift to the right will be affecting more and more bits. Since the 'ptr' variable has been truncated, all of its bits will start being filled with zeroes after a certain iteration. The result of this all will be incorrect 'hash' composing in Win64 applications. Because of 'hash' being filled with zeroes, collisions may occur, i.e. getting identical hashes for different input data (pointers in this case). As a result, it may cause incorrect program operation. If there were a conversion to the 'memsize-type', no truncation would have taken place and the shift (and therefore the hash composing) would have been executed properly.
Take a look at the following code:
class CValueList : public CListCtrl
{
....
public:
BOOL SortItems(_In_ PFNLVCOMPARE pfnCompare,
_In_ DWORD_PTR dwData);
....
};
void CLastValuesView::OnListViewColumnClick(....)
{
....
m_wndListCtrl.SortItems(CompareItems, (DWORD)this);
....
}PVS-Studio's diagnostic message: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being cast: 'this'. test.cpp 87
Warning V220 indicates a dangerous double data conversion. First a variable of the 'memsize-type' is cast to a 32-bit value and then immediately extended back to the 'memsize-type'. This in fact means truncation of the most significant bits. And that's almost always an error.
Going on with dangerous type conversions:
#define YAHOO_LOGINID "yahoo_id"
DWORD_PTR __cdecl CYahooProto::GetCaps(int type, HANDLE /*hContact*/)
{
int ret = 0;
switch (type)
{
....
case PFLAG_UNIQUEIDSETTING:
ret = (DWORD_PTR)YAHOO_LOGINID;
break;
....
}
return ret;
}PVS-Studio's diagnostic message: V221 Suspicious sequence of types castings: pointer -> memsize -> 32-bit integer. The value being cast: '"yahoo_id"'. test.cpp 99
I've noticed this tendency that we have more and more type conversions with every new example. Here we have 3 at once, 2 of them being dangerous due to the same reason as described above. Since 'YAHOO_LOGINID' is a string literal, its type is 'const char*' which has the same size on the 64-bit architecture as 'DWORD_PTR', so an explicit type conversion is legal here. But then bad things start to happen. The 'DWORD_PTR' type is implicitly cast to an integer 32-bit one. But that's not all. Since the function return result has the 'DWORD_PTR' type, there'll be one more implicit conversion - this time, back to the 'memsize-type'. Apparently, in this case, the user handles the returned value at their own risk.
Notice that the Visual Studio 2013 compiler output the following message for this code:
warning C4244: '=' : conversion from 'DWORD_PTR' to 'int', possible loss of data
It's just a good moment to ask why we mentioned a warning generated by Visual Studio 2013 for this example only. The question is fair but please be patient: we'll talk about it a bit later.
For now, let's continue our discussion. Here's another code fragment with class hierarchy:
class CWnd : public CCmdTarget
{
....
virtual void WinHelp(DWORD_PTR dwData, UINT nCmd = HELP_CONTEXT);
....
};
class CFrameWnd : public CWnd
{
....
};
class CFrameWndEx : public CFrameWnd
{
....
virtual void WinHelp(DWORD dwData, UINT nCmd = HELP_CONTEXT);
....
};PVS-Studio's diagnostic message: V301 Unexpected function overloading behavior. See first argument of function 'WinHelpA' in derived class 'CFrameWndEx' and base class 'CWnd'. test.cpp 122
What's interesting about this example is that it was taken from the analysis report for Visual C++ 2012's libraries. So, as you can see, even Visual C++ developers make 64-bit errors.
This bug is discussed in detail in this article; and in this one, I'd like to only outline it in brief. On the 32-bit architecture, this code will be processed correctly since the types 'DWORD' and 'DWORD_PTR' have the same sizes; this function will be redefined in the descendant class, so the code will execute correctly. But the trap is still there and will show up on the 64-bit architecture. Since the types 'DWORD' and 'DWORD_PTR' will have different sizes in this case, the polymorphism will be broken. We'll end up with 2 different functions, which contradicts the initially intended scenario.
The next example:
void CSymEngine::GetMemInfo(CMemInfo& rMemInfo)
{
MEMORYSTATUS ms;
GlobalMemoryStatus(&ms);
_ultot_s(ms.dwMemoryLoad, rMemInfo.m_szMemoryLoad,
countof(rMemInfo.m_szMemoryLoad), 10);
....
}PVS-Studio's diagnostic message: V303 The function 'GlobalMemoryStatus' is deprecated in the Win64 system. It is safer to use the 'GlobalMemoryStatusEx' function. test.cpp 130
I don't think you need any special comments on this. It all is clear from the message text: the programmer should have used the 'GlobalMemoryStatusEx' function because the 'GlobalMemoryStatus' function may work incorrectly on the 64-bit architecture. This issue is explained in detail in the function description at the MSDN portal.
Note.
Notice that all the bugs described above can be found in any ordinary application. For them to occur, the program doesn't necessarily need to handle large memory amounts. And this is why we put the diagnostics detecting these bugs into the first-level group.
Before proceeding to talk about the analysis results demonstrated by Visual Studio 2013's integrated static analyzer, I'd like to say a few words about the compiler warnings. Attentive readers surely have noticed that I cited only 1 compiler warning in the text above. Why so? You see, there simply were no other warnings related to 64-bit errors in any way. It was with the 3rd level enabled, mind you.
But once you compile this example with all the warnings enabled (EnableAllWarnings), you'll get...
Quite unexpectedly, these warnings point to header files (for example winnt.h). If you are patient enough to spend some time to search through this pile of warnings for those related to the project itself, there'll be something of interest for you. For example:
warning C4312: 'type cast' : conversion from 'int' to 'JABBER_LIST_ITEM *' of greater size
warning C4311: 'type cast' : pointer truncation from 'void *' to 'unsigned long'
warning C4311: 'type cast' : pointer truncation from 'CLastValuesView *const ' to 'DWORD'
warning C4263: 'void CFrameWndEx::WinHelpA(DWORD,UINT)' : member function does not override any base class virtual member function
In total, the compiler output 10 warnings for the file with these examples, only 3 of them directly pointing to 64-bit errors (compiler warnings C4311 and C4312). Among them, there are also a few pointing to narrowing type conversions (C4244) or issues when virtual functions are not redefined (C4263). These warnings also indirectly point to 64-bit errors.
So, after excluding the warnings that repeat each other one way or another, we'll get 5 warnings left related to the 64-bit errors discussed in this article.
That is, as you can see, the Visual Studio compiler has failed to detect some of the 64-bit errors. PVS-Studio, as you remember, has found 9 first-level errors in the same file.
You will ask, "And what about the integrated static analyzer coming with Visual Studio 2013?" Maybe it did better and found more bugs? Let's see.
The results of analyzing these examples by the static analyzer coming with the Visual Studio 2013 IDE included 3 warnings:
Well, but we are discussing 64-bit errors, aren't we? How many bugs from this list refer to 64-bit ones? Only the last one (using a function that may return incorrect values).
So it turns out that Visual Studio 2013's static analyzer found only 1 64-bit error. Compare it to 9 found by the PVS-Studio analyzer. Impressive, isn't it? Now imagine what this difference will turn into in large-scale projects.
Now let me remind you once again that the static code analyzers coming with Visual Studio 2013 and Visual Studio 2015 versions are identical regarding their capabilities (to learn more, see this post).
It would be best to present the results in a table form.
Table 1. The results of 64-bit errors analysis by the PVS-Studio analyzer and the means of Microsoft Visual Studio 2013
As seen from the table, PVS-Studio found 9 64-bit errors while Microsoft Visual Studio 2013's combined means found 6. You may argue that it's not a huge difference really. But I don't think so. Let's look closer:
The above said implies that you need to make some additional preparations to be able to see 64-bit errors found by the means of Visual Studio 2013. And now imagine how much this amount of work will grow when working with a really large project.
What about PVS-Studio? Well, it takes you just a few mouse clicks to run the analysis, enable filtering for 64-bit bugs and warnings you need, and get the result.
Hopefully I've managed to show that software porting to the 64-bit architecture involves a number of difficulties. Errors like the ones described in this article are pretty easy to make yet very hard to find. Add to this the fact that not all of such errors are diagnosed by the means of Microsoft Visual Studio 2013, and even then, you'll need to do some additional work to make it find anything. On the contrary, the PVS-Studio static analyzer has coped with this task very well. Besides, it makes the bug search and filtering processes much more convenient and easy. I don't think you'd argue that in really large projects, this task would be pretty tough without a tool like this, so a good static analyzer is just vitally necessary in such cases.
You are a 64-bit software developer? Welcome to download PVS-Studio's trial version to check your project and see how many 64-bit first level messages you'll get. If you do find a few - please fix them. Thus you will make this world a tiny bit better.
As I promised, here is a list of reference materials to read on 64-bit issues: