www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 4887] New: Right-shifting by 32 is allowed and broken

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887

           Summary: Right-shifting by 32 is allowed and broken
           Product: D
           Version: 1.057
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: thecybershadow gmail.com


--- Comment #0 from Vladimir <thecybershadow gmail.com> 2010-09-18 03:15:15 PDT
---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 18 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #1 from Vladimir <thecybershadow gmail.com> 2010-09-18 03:16:36 PDT
---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #2 from Vladimir <thecybershadow gmail.com> 2010-09-18 03:17:18 PDT
---
Oops, right-shifting*

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887


Stewart Gordon <smjg iname.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |spec
                 CC|                            |smjg iname.com


--- Comment #3 from Stewart Gordon <smjg iname.com> 2010-09-18 08:53:09 PDT ---
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?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #4 from Vladimir <thecybershadow gmail.com> 2010-09-18 08:58:22 PDT
---
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...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au
           Severity|normal                      |major


--- Comment #5 from Don <clugdbug yahoo.com.au> 2010-09-18 11:43:17 PDT ---
(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. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #6 from Stewart Gordon <smjg iname.com> 2010-09-18 12:12:33 PDT ---
(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.) -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887


Vladimir <thecybershadow gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Platform|Other                       |x86
         OS/Version|Windows                     |All


--- Comment #7 from Vladimir <thecybershadow gmail.com> 2010-09-18 12:16:26 PDT
---
(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. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Sep 18 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887


Austin Hastings <ah08010-d yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ah08010-d yahoo.com


--- Comment #8 from Austin Hastings <ah08010-d yahoo.com> 2010-10-03 18:40:49
PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 03 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #9 from github-bugzilla puremagic.com 2012-01-23 23:08:33 PST ---
Commit pushed to
https://github.com/D-Programming-Language/d-programming-language.org

https://github.com/D-Programming-Language/d-programming-language.org/commit/93e88288b5246370d61fd1266022eb5850f0cde5
Issue 4887 - Right-shifting by 32 is allowed and broken

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 23 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #10 from github-bugzilla puremagic.com 2012-01-23 23:19:27 PST ---
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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 23 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887



--- Comment #11 from github-bugzilla puremagic.com 2012-01-23 23:25:19 PST ---
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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 23 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4887


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla digitalmars.com
            Version|1.057                       |D1 & D2
         Resolution|                            |FIXED


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 23 2012