4887 – Right-shifting by 32 is allowed and broken

D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 4887 - Right-shifting by 32 is allowed and broken
Summary: Right-shifting by 32 is allowed and broken
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: x86 All
: P2 major
Assignee: No Owner
URL:
Keywords: accepts-invalid, spec
Depends on:
Blocks:
Reported: 2010年09月18日 03:15 UTC by Vladimir Panteleev
Modified: 2015年06月09日 05:11 UTC (History)
4 users (show)

See Also:


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description Vladimir Panteleev 2010年09月18日 03:15:15 UTC
const int x = 12345;
static assert(x>>32 == 0); // fails
It looks like x>>32==x when x is a 32-bit or smaller integer.
Same problem at runtime.
I guess that right-shifting by 32 should be forbidden.
Comment 1 Vladimir Panteleev 2010年09月18日 03:16:36 UTC
BTW, this is a real-world bug that took me several hours to track down. I was writing an application that's 64-bit-agnostic, and was left-shifting a size_t to get the high-order dword.
Comment 2 Vladimir Panteleev 2010年09月18日 03:17:18 UTC
Oops, right-shifting*
Comment 3 Stewart Gordon 2010年09月18日 08:53:09 UTC
http://www.digitalmars.com/d/1.0/expression.html#ShiftExpression
"It's illegal to shift by more bits than the size of the quantity being shifted:
int c;
c << 33;	// error"
However, there's a real problem with that spec: what if the number of bits to shift by isn't known at compile time?
Comment 4 Vladimir Panteleev 2010年09月18日 08:58:22 UTC
Yeah, I saw that - the problem is with shifting by exactly 32 bits. Runtime behavior of 33 bits or more is a completely different problem...
Comment 5 Don 2010年09月18日 11:43:17 UTC
(In reply to comment #3)
> http://www.digitalmars.com/d/1.0/expression.html#ShiftExpression
> "It's illegal to shift by more bits than the size of the quantity being
> shifted:
> 
> int c;
> c << 33; // error"
> 
> However, there's a real problem with that spec: what if the number of bits to
> shift by isn't known at compile time?
That may be solvable with range propagation. Make it an error if it's not known to be less than 32. It'd be painful to completely implement that rule right now, but perhaps not later on when range propagation becomes more capable. We could at least make it an error if the range of the expression doesn't include ANY values in the legal range, and that would cover this test case.
I suspect that shifts by runtime-determined numbers of bits are relatively rare -- and my experience is that they're quite bug-prone.
Raising priority to major, since this is a nasty trap.
Comment 6 Stewart Gordon 2010年09月18日 12:12:33 UTC
(In reply to comment #5)
> (In reply to comment #3)
> > http://www.digitalmars.com/d/1.0/expression.html#ShiftExpression
> > "It's illegal to shift by more bits than the size of the quantity being
> > shifted:
> > 
> > int c;
> > c << 33; // error"
> > 
> > However, there's a real problem with that spec: what if the number of bits to
> > shift by isn't known at compile time?
> 
> That may be solvable with range propagation. Make it an error if it's not known
> to be less than 32. It'd be painful to completely implement that rule right
> now, but perhaps not later on when range propagation becomes more capable. We
> could at least make it an error if the range of the expression doesn't include
> ANY values in the legal range, and that would cover this test case.
> I suspect that shifts by runtime-determined numbers of bits are relatively rare
> -- and my experience is that they're quite bug-prone.
> 
> Raising priority to major, since this is a nasty trap.
But range propagation isn't going to be implemented in D1, or is it?
Trying it in 1.064 (Windows):
* (u)int << 32 or (u)int >> 32 is accepted by the compiler, but is a nop
* (u)int << 33 or (u)int >> 33 is rejected by the compiler, but if the 33 is passed through a variable then it's equivalent to shifting by 1
* Shifting a (u)long by 32 or 33 seems to work, but the bug affects shifting one by 64 or 65.
What is the "Other" platform for which this bug is filed? (I'm guessing the behaviour to be processor-dependent. Mine, for the record, is a 2.4GHz Intel Core Duo.)
Comment 7 Vladimir Panteleev 2010年09月18日 12:16:26 UTC
(In reply to comment #6)
> What is the "Other" platform for which this bug is filed? (I'm guessing the
> behaviour to be processor-dependent. Mine, for the record, is a 2.4GHz Intel
> Core Duo.)
Sorry, I forgot to fill out that field.
Pretty sure things like this have to be deterministic across x86 implementations. Of course the behavior in 64-bit programs may differ.
Comment 8 Austin Hastings 2010年10月03日 18:40:49 UTC
I encountered a similar problem. I was taking code I had written with ulongs and trying to template-ize the code. For me, the code below prints "plain error?".
======
import std.stdio;
void main() {
	uint foo = 0;
	plain_sub( foo );
}
int plain_sub( const uint value ) {
	
	if( uint t32 = value >> 32 ) {
		writeln( "plain error?" );
	}
	
	return 0;
}
=====
C99 says that shifts >= the width of the victim are undefined behavior. The D2 manual says shifting /more/ than the width of the victim is illegal. 
 
Apparently, shifting equal to the width is legal-but-surprising. I'd like it to be either illegal, with a warning, or legal-but-not-surprising.
Comment 10 github-bugzilla 2012年01月23日 23:19:27 UTC
Commit pushed to https://github.com/D-Programming-Language/dmd
https://github.com/D-Programming-Language/dmd/commit/c62d4696239a189106aacbded87cceb25331a39f
fix Issue 4887 - Right-shifting by 32 is allowed and broken
Comment 11 github-bugzilla 2012年01月23日 23:25:19 UTC
Commit pushed to https://github.com/D-Programming-Language/phobos
https://github.com/D-Programming-Language/phobos/commit/ecbc5db9c1ac2d4d025d6426195a2925452378ad
fix Issue 4887 - Right-shifting by 32 is allowed and broken


AltStyle によって変換されたページ (->オリジナル) /