Bug 18730 – dmd miscompiles core.bitop.bt with -O

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2018-04-04T23:52:51Z
Last change time
2018-04-30T17:17:54Z
Keywords
pull, wrong-code
Assigned to
No Owner
Creator
ag0aep6g
Blocks
18750
See also
https://issues.dlang.org/show_bug.cgi?id=18734, https://issues.dlang.org/show_bug.cgi?id=18717

Comments

Comment #0 by ag0aep6g — 2018-04-04T23:52:51Z
Spin-off from issue 18717 which can be closed as a duplicate when this one gets fixed. ---- void main() { enum bitsPerSizeT = size_t.sizeof * 8; enum bitIndex = int.max + 1L; auto a = new size_t[](bitIndex / bitsPerSizeT + 1); bt(a.ptr, bitIndex); } /* 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); } ---- Compile with `-O`. Resulting program segfaults. Generated code for the bt function: ---- 0: 55 push rbp 1: 48 8b ec mov rbp,rsp 4: 0f a3 3e bt DWORD PTR [rsi],edi 7: 19 c0 sbb eax,eax 9: f7 d8 neg eax b: 5d pop rbp c: c3 ret ---- edi should be rdi in the bt instruction. It's hard to find information on this, but this page says that bt interprets the offset as signed: <http://faydoc.tripod.com/cpu/bt.htm>. That explains the segfault, even though `bitIndex` fits into a uint.
Comment #1 by ketmar — 2018-04-05T15:45:14Z
it segfaults on x86 too. but i won't change "hardware" field, 'cause it seems that with x86_64 there is a codegen bug too, and fixing segfault for x86_64 should automatically fix it for x86.
Comment #2 by ag0aep6g — 2018-04-05T19:08:26Z
Comment #3 by ag0aep6g — 2018-04-05T19:25:54Z
(In reply to Ketmar Dark from comment #1) > it segfaults on x86 too. but i won't change "hardware" field, 'cause it > seems that with x86_64 there is a codegen bug too, and fixing segfault for > x86_64 should automatically fix it for x86. I think we can split this in two: 1) Fix the x86-64 issue presented here by fixing the register size. This is the easy part. It doesn't fix the situation on x86. 2) File a new issue for the larger problem: core.bitop.bt's bitnum parameter is a size_t when it should be a ptrdiff_t. Then bt's body has to be adjusted for that, because D doesn't have negatives indices. And dmd has to be adjusted to recognize the new body.
Comment #4 by ketmar — 2018-04-05T19:33:23Z
yeah. would you do it, please?
Comment #5 by ag0aep6g — 2018-04-05T21:23:31Z
(In reply to ag0aep6g from comment #3) > 2) File a new issue for the larger problem: core.bitop.bt's bitnum parameter > is a size_t when it should be a ptrdiff_t. Then bt's body has to be adjusted > for that, because D doesn't have negatives indices. And dmd has to be > adjusted to recognize the new body. Filed that as issue 18734.
Comment #6 by ketmar — 2018-04-05T22:15:50Z
tnank you.
Comment #7 by bugzilla — 2018-04-06T20:28:36Z
This covers 64 bit operations too: http://www.felixcloutier.com/x86/BT.html
Comment #8 by github-bugzilla — 2018-04-25T00:53:12Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/dbbb00a2055c074004bf294b71ffe1b090ff984a fix issue 18730 - dmd miscompiles core.bitop.bt with -O https://github.com/dlang/dmd/commit/4a0437981a2754ca74d47a2371d80072e02d1e2c fixup! fix issue 18730 - dmd miscompiles core.bitop.bt with -O https://github.com/dlang/dmd/commit/61e96ecf532e5480d609402a5b8e0e4c24cae89d run tests for issue 18730 only on x86-64 https://github.com/dlang/dmd/commit/7f30e528231de450d5491278b408ddfb1e02061c Merge pull request #8142 from aG0aep6G/18730 fix issue 18730 - dmd miscompiles core.bitop.bt with -O merged-on-behalf-of: Walter Bright <[email protected]>
Comment #9 by ag0aep6g — 2018-04-30T17:17:54Z
*** Issue 18717 has been marked as a duplicate of this issue. ***