https://issues.dlang.org/show_bug.cgi?id=18734

          Issue ID: 18734
           Summary: bitnum parameter of core.bitop.bt should be signed
           Product: D
           Version: D2
          Hardware: All
                OS: Linux
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P1
         Component: dmd
          Assignee: nob...@puremagic.com
          Reporter: ag0ae...@gmail.com

Closely related to issue 18730. But 18730 is x86-64 only, while this one
affects both x86 and x86-64.

The issue is best demonstrated with 32-bit code:

----
import std.stdio;
void main()
{
    static assert(size_t.sizeof == 4); /* We're in 32-bit code. */

    enum len = 2 ^^ 27;
    assert(len * size_t.sizeof * 8L == 2L ^^ 32);
        /* Exactly enough space for size_t.max bits. */

    auto a = new size_t[len + 1];
        /* Adding one because we're going to start from a[1]. */

    a[$ - 1] = 0x8000_0000; /* Set the very last bit. */
    writeln(bt(&a[1], size_t.max)); /* Try to read it. */
        /* Without -O: 1, correct. With -O: 0, wrong. */

    a[$ - 1] = 0; /* Unset the very last bit. */
    a[0] = 0x8000_0000; /* Set the last bit of the first size_t. */
    writeln(bt(&a[1], size_t.max)); /* Try reading the very last bit again. */
        /* Without -O: "0", correct. With -O: "1", wrong. */
}

/* Copied from core.bitop. */
int bt(in size_t* p, size_t bitnum) pure @system
{
    static if (size_t.sizeof == 8)
        return ((p[bitnum >> 6] & (1L << (bitnum & 63)))) != 0;
    else static if (size_t.sizeof == 4)
        return ((p[bitnum >> 5] & (1  << (bitnum & 31)))) != 0;
    else
        static assert(0);
}
----

The wrong behavior happens because core.bitop.bt is optimized using the bt
instruction. And the bt instruction interprets offset (bitnum) as signed. So
instead of going size_t.max bits up from `a[1]`, it goes 1 bit down.

If core.bitop.bt is supposed to directly map to the bt instruction, the type of
the bitnum parameter should be ptrdiff_t, not size_t. Making that change
probably means that the body of core.bitop.bt has to be changed to accommodate
for negative values of bitnum. And then DMD will have to be updated to
recognize the new body.

x86-64 is affected in the same way, but you can't actually have an array of
2^63 bits, so it can't be hit like on x86.

--

Reply via email to