Incorrect sign extension from int to 64-bit int - by Bruce Dawson

Status : 

  Fixed<br /><br />
		This item has been fixed in the current or upcoming version of this product.<br /><br />
		A more detailed explanation for the resolution of this particular item may have been provided in the comments section.


2
0
Sign in
to vote
ID 804947 Comments
Status Closed Workarounds
Type Bug Repros 1
Opened 10/9/2013 10:06:14 AM
Access Restriction Public

Description

A 32-bit signed-int constant (negative) is returned from a function as unsigned-int, and assigned to a signed-int variable. This signed variable is then compared to the original signed constant. In x64 release builds the variable is zero extended and the constant is sign extended causing them to not match.

Both numbers should be treated as 32-bit values, not 64-bit values. Sign or zero extending an int to 64-bits is only legal if it does not affect the results. Therefore this is a code-gen bug. I'm not sure why it doesn't happen more often as the circumstances seem quite common and the repro is quite stable.
Sign in to post a comment.
Posted by Microsoft on 6/6/2014 at 9:54 AM
Thank you for using Visual Studio and for reporting this bug. We are happy to let you know that this issue has been fixed in Visual Studio 2013 Update 2. If you already have Visual Studio 2013, you can upgrade to Update 2 for free or you can install a trial version from: http://go.microsoft.com/?linkid=9832436
Posted by Bruce Dawson on 10/15/2013 at 6:41 PM
I got an e-mail saying:

Field "Resolution" changed from "Not Set" to "Fixed".

However the Resolution field is not shown. Pity.
Posted by Bruce Dawson on 10/11/2013 at 3:06 PM
Thank you for the response. I look forward to a fix in future versions.

I was also looking at section 4.7 of the standard this morning. Note that it says that assigning from unsigned to int is *not* undefined, it is *implementation defined*. You describe it both ways, but the standard is clear that it is implementation defined. Thus, VC++ is required to do something reasonable and to document what it does. The Microsoft documentation on this topic is a bit vague, but it does say that the conversion can be done, with no limitations listed:

http://msdn.microsoft.com/en-us/library/37ct5baz.aspx
Posted by Microsoft on 10/11/2013 at 2:09 PM
Thank you for reporting this issue. Your reported case revealed a bug in our compiler. We will release a fix as soon as possible. If you need a temporary workaround, you can disable optimizations of affected functions using #pragma optimize ("", off). If you need a hotfix, please visit the support site: http://support.microsoft.com/common/international.aspx?RDPATH=dm;en-us;selectassist&target=assistance.

I also want to point out your code relies on undefined behavior when casting a negative signed integer to unsigned and then casting back to signed again. Based on C++ standard, when casting an unsigned integer to a signed integer and the value can't be represented in the destination type, the behavior is implementation-defined. So the second cast is undefined. So technically the compiler can generate anything here. But I can still repro the issue after removing the second cast by changing the type to make the code standard compliant. This is the bug we will release a fix in the future.

[From the standard]
4.7 Integral conversions [conv.integral]
1 A prvalue of an integer type can be converted to a prvalue of another integer type. A prvalue of an unscoped enumeration type can be converted to a prvalue of an integer type.
2 If the destination type is unsigned, the resulting value is the least unsigned integer congruent to the source integer (modulo 2n where n is the number of bits used to represent the unsigned type). [ Note: In a two’s complement representation, this conversion is conceptual and there is no change in the bit pattern (if there
is no truncation). —end note ]
3 If the destination type is signed, the value is unchanged if it can be represented in the destination type (and bit-field width); otherwise, the value is implementation-defined.

Thanks,
Rui Zhang
Visual C++
Posted by Microsoft on 10/9/2013 at 10:52 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)
Posted by Bruce Dawson on 10/9/2013 at 10:18 AM
This bug seems like it is related to the pointer sign extension bug that was reported in 2010 and which doesn't appear to be entirely fixed:

https://connect.microsoft.com/VisualStudio/feedback/details/622723/const-pointer-sign-extension-not-consistent-in-64-bit-compiler
Posted by Bruce Dawson on 10/9/2013 at 10:15 AM
A few more details:

The generated code basically goes:
mov         ebx,dword ptr [rax+48h] ; Load FFFFFE70 into ebx, zero extend to rbx
cmp         rbx,0FFFFFFFFFFFFFE70h ; Compare zero extended int against sign extended constant. Bug!

That's it. -400 fails to equal itself. Since both eClientLogonOSType and k_eOSUMQ are signed integers it is illegal to inconsistently sign extend them. Changing the return type of client_os_type() to int fixes the bug, but doing this in the actual code base is impractical.