Bug 9449 – Static arrays of 128bit types segfault on initialization. Incorrect calling of memset128ii.

This small code crashs. ---------------------------------- import core.simd; void main() { ubyte16 table[1]; } ---------------------------------- It crashes in: void[] *_memset128ii(void[] *p, void[] value, size_t count); It seems that a wrong "count" is passed in by the _Dmain() function. Details: DMD64 D Compiler v2.061 Linux Ubuntu x86_64 AMD Phenom II

Something is very screwy with the executable that DMD produces for this code. For example: $ cat test.d import core.simd; void main() { ubyte16 table[1]; } $ dmd -L--export-dynamic -g -m64 test.d $ gdb test GNU gdb (GDB) 7.4.1-debian Copyright (C) 2012 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-linux-gnu". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /tmp/test...done. (gdb) break Dmain Segmentation fault $ Commenting out the ubyte16 line makes the problem go away (it will correctly set the breakpoint). Looks like the codegen is screwed up somewhere?

Actually, this looks like a compiler bug. The ubyte16 alias translates to __vector(ubyte[16]), which is a compiler built-in magic type. Here's the disassembly of Dmain: 0000000000418620 <_Dmain>: 418620: 55 push %rbp 418621: 48 8b ec mov %rsp,%rbp 418624: 48 83 ec 10 sub $0x10,%rsp 418628: 48 be 01 00 00 00 00 movabs $0x1,%rsi 41862f: 00 00 00 418632: 66 0f 6f 05 e6 77 01 movdqa 0x177e6(%rip),%xmm0 # 42fe20 <_IO_stdin_used+0x10> 418639: 00 41863a: 48 8d 7d f0 lea -0x10(%rbp),%rdi 41863e: e8 a9 07 00 00 callq 418dec <_memset128ii> 418643: 31 c0 xor %eax,%eax 418645: c9 leaveq 418646: c3 retq Here's the disassembly of _memset128ii: 0000000000418dec <_memset128ii>: 418dec: 55 push %rbp 418ded: 48 8b ec mov %rsp,%rbp 418df0: 48 83 ec 20 sub $0x20,%rsp 418df4: 48 89 75 e8 mov %rsi,-0x18(%rbp) 418df8: 48 89 55 f0 mov %rdx,-0x10(%rbp) 418dfc: 49 89 f8 mov %rdi,%r8 418dff: 49 89 fb mov %rdi,%r11 418e02: 49 89 c9 mov %rcx,%r9 418e05: 49 c1 e1 04 shl $0x4,%r9 418e09: 4c 03 cf add %rdi,%r9 418e0c: 4d 3b c1 cmp %r9,%r8 418e0f: 73 18 jae 418e29 <_memset128ii+0x3d> 418e11: 48 8b 55 f0 mov -0x10(%rbp),%rdx 418e15: 48 8b 45 e8 mov -0x18(%rbp),%rax 418e19: 49 89 00 mov %rax,(%r8) 418e1c: 49 89 50 08 mov %rdx,0x8(%r8) 418e20: 49 83 c0 10 add $0x10,%r8 418e24: 4d 39 c8 cmp %r9,%r8 418e27: 72 e8 jb 418e11 <_memset128ii+0x25> 418e29: 49 8b c3 mov %r11,%rax 418e2c: 48 8b e5 mov %rbp,%rsp 418e2f: 5d pop %rbp 418e30: c3 retq Note that the expected parameters to memset128ii appear to not be passed by Dmain; I traced the execution into memset128ii and found that it was trying to memset an unreasonably large range of memory (2e+15 bytes), probably because the wrong arguments were passed to it. Since the only druntime code involved is template wrapper around the compiler magic type __vector, the fault must lie with the compiler SIMD intrinsics.

_memset128ii expects: %rcx => size_t count %rdx => .ptr of value array $rsi => .length of value array %rdi => pointer to first argument array what _Dmain passes: %rcx => nothing (garbage) %rdx => nothing (garbage) %rsi => size_t count %rdi => pointer to 16 byte object %xmm0 => ubyte16[1] array

Seems to be related to bug 8518.

(In reply to comment #4) > Seems to be related to bug 8518. Thanks for founding. The root of the problem (at least this one) is when dmd frontend parses and generates list of expressions, it does not create "hidden" expression which calls _memset128ii. Instead it does this when it executes AssignExpression::toElem() and later calls setArray() which issues call to _memset128ii. However it does not convert ubyte16[1] from static array to dynamic array and passes it as a static array. Since _memset128ii expects dynamic array, the program goes off the rails.

It may be a regression. 8 month ago Walter introduced two commits to e2ir.c (https://github.com/D-Programming-Language/dmd/commit/6c2a2878200e0df1c73db976a747abf61b6a5e1a) and src/memset.d (https://github.com/D-Programming-Language/druntime/commit/a405a02394e2c26c6a66c3fc5ef3777bb86cd973) to fix reg pair. However there is no crosstalk between how e2ir.c pass arguments and how _memset128ii takes it. I do not know whether original code was supported before these changes, but if it was, this is a regression.

(In reply to comment #5) > (In reply to comment #4) > > Seems to be related to bug 8518. > > Thanks for founding. The root of the problem (at least this one) is when dmd > frontend parses and generates list of expressions, it does not create "hidden" > expression which calls _memset128ii. Instead it does this when it executes > AssignExpression::toElem() and later calls setArray() which issues call to > _memset128ii. However it does not convert ubyte16[1] from static array to > dynamic array and passes it as a static array. Since _memset128ii expects > dynamic array, the program goes off the rails. I'm pretty sure the use of void[] in _memset128ii is simply so as to have a 128bit data type. It's never used as, or expected to be, an array. _memset128ii doesn't care whether it's being passed a static or dynamic array, it just blindly increments a pointer and writes to it "count" times. (In reply to comment #3) > _memset128ii expects: > > %rcx => size_t count > %rdx => .ptr of value array > $rsi => .length of value array > %rdi => pointer to first argument array This is incorrect. _memset128 expects: RCX: size_t count RDX: higher 64 bits of value RSI: lower 64 bits of value RDI: pointer to the 1st element of the destination array.

(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #4) > > > Seems to be related to bug 8518. > > > > Thanks for founding. The root of the problem (at least this one) is when dmd > > frontend parses and generates list of expressions, it does not create "hidden" > > expression which calls _memset128ii. Instead it does this when it executes > > AssignExpression::toElem() and later calls setArray() which issues call to > > _memset128ii. However it does not convert ubyte16[1] from static array to > > dynamic array and passes it as a static array. Since _memset128ii expects > > dynamic array, the program goes off the rails. > > I'm pretty sure the use of void[] in _memset128ii is simply so as to have a > 128bit data type. It's never used as, or expected to be, an array. > > _memset128ii doesn't care whether it's being passed a static or dynamic array, > it just blindly increments a pointer and writes to it "count" times. > > (In reply to comment #3) > > _memset128ii expects: > > > > %rcx => size_t count > > %rdx => .ptr of value array > > $rsi => .length of value array > > %rdi => pointer to first argument array > > This is incorrect. _memset128 expects: > > RCX: size_t count > RDX: higher 64 bits of value > RSI: lower 64 bits of value > RDI: pointer to the 1st element of the destination array. /s/_memset128/_memset128ii

(In reply to comment #7) > I'm pretty sure the use of void[] in _memset128ii is simply so as to have a > 128bit data type. It's never used as, or expected to be, an array. > > _memset128ii doesn't care whether it's being passed a static or dynamic array, > it just blindly increments a pointer and writes to it "count" times. I think it does matter whether dynamic array was passed or a static one due to how arguments are passed. > (In reply to comment #3) > > _memset128ii expects: > > > > %rcx => size_t count > > %rdx => .ptr of value array > > $rsi => .length of value array > > %rdi => pointer to first argument array > > This is incorrect. _memset128 expects: > > RCX: size_t count > RDX: higher 64 bits of value > RSI: lower 64 bits of value > RDI: pointer to the 1st element of the destination array. I see no difference between length dynamic array property and your "lower 64 bits of value" (also between ptr and "higher 64 bits of value"). And passing a pointer to dynamic array is not the same thing as passing pointer to the first element: http://dpaste.dzfl.pl/8f91aed8

(In reply to comment #9) > (In reply to comment #7) > > I'm pretty sure the use of void[] in _memset128ii is simply so as to have a > > 128bit data type. It's never used as, or expected to be, an array. > > > > _memset128ii doesn't care whether it's being passed a static or dynamic array, > > it just blindly increments a pointer and writes to it "count" times. > > I think it does matter whether dynamic array was passed or a static one due to > how arguments are passed. It doesn't matter in this case because it is a pointer being passed, not an array at all. > > (In reply to comment #3) > > > _memset128ii expects: > > > > > > %rcx => size_t count > > > %rdx => .ptr of value array > > > $rsi => .length of value array > > > %rdi => pointer to first argument array > > > > This is incorrect. _memset128 expects: > > > > RCX: size_t count > > RDX: higher 64 bits of value > > RSI: lower 64 bits of value > > RDI: pointer to the 1st element of the destination array. > > I see no difference between length dynamic array property and your "lower 64 > bits of value" (also between ptr and "higher 64 bits of value"). because value is not an array. As I said before, void[] is just used because conveniently void[].sizeof == 16 (128 bits) on x64 (the check for x64 is done inside e2ir.c) > And passing a > pointer to dynamic array is not the same thing as passing pointer to the first > element: http://dpaste.dzfl.pl/8f91aed8 See my comment above. Each *element* of the array is being represented by a void[]. There is no D style array passing happening here, static or otherwise, it's just pointers. Imagine replacing void[] with a hypothetical _128BitType and you'll see what I mean. I'm currently re-implementing memset.d and updating the compiler to interact with the new functions. This should hopefully fix this bug and maybe 9969 also, if there isn't a nasty backend bug hiding behind it all.

(In reply to comment #10) > There is no D style array passing happening here, static or otherwise, > it's just pointers. Sorry, mistake. There is an array being passed as "value", but from the callers point of view (generated in dmd) it's not an array at all, it's just a 128 bit type.

(In reply to comment #10) > > I see no difference between length dynamic array property and your "lower 64 > > bits of value" (also between ptr and "higher 64 bits of value"). > > because value is not an array. As I said before, void[] is just used because > conveniently void[].sizeof == 16 (128 bits) on x64 (the check for x64 is done > inside e2ir.c) Value is actually accepted as an array due to passing conversions and unusual usage inside memset function is no excuse for changing ABI interpretation. Clearly, anyone can pass many different things through some inappropriate parameter but it does not mean that each time callee would adjust passing convention for different types. I don't see point in this dispute further . I argue that value is technically accepted as dynamic array and you argue that it is treated as 128 bit element. These points don't contradict and however arguments are called (lower value or length property) does change the picture - there is no correspondence between what is passed and what is received. > I'm currently re-implementing memset.d and updating the compiler to interact > with the new functions. This should hopefully fix this bug and maybe 9969 also, > if there isn't a nasty backend bug hiding behind it all. I doubt that it is possible without dmd hacking but good luck.

I hit the same bug without any obvious SIMD operations: struct Point { double f; double g; } void main() { Point[1] arr; } Ali

I just ran in to this again with something equivalent to Ali's case above. My previous attempts to fix this never got anywhere. I could get it working but then other things broke, rinse and repeat... Does anyone have any clue how to do this? It's a pretty glaring hole to segfault any time someone declares a static array of 128 bit types (excluding cdouble, which has it's own memset overload). I'm marking this a regression as, whatever the reason, it worked with 2.065.0 Walter I've assigning you because it seems likely that https://github.com/D-Programming-Language/dmd/commit/6c2a2878200e0df1c73db976a747abf61b6a5e1a) and (https://github.com/D-Programming-Language/druntime/commit/a405a02394e2c26c6a66c3fc5ef3777bb86cd973 caused it, but I'm not certain.

*** Issue 8518 has been marked as a duplicate of this issue. ***

(In reply to John Colvin from comment #14) > I'm marking this a regression as, whatever the reason, it worked with 2.065.0 > > Walter I've assigning you because it seems likely that > https://github.com/D-Programming-Language/dmd/commit/ > 6c2a2878200e0df1c73db976a747abf61b6a5e1a) and > (https://github.com/D-Programming-Language/druntime/commit/ > a405a02394e2c26c6a66c3fc5ef3777bb86cd973 caused it, but I'm not certain. That seems very unlikely, as those changes were dated June 2012, long before 2.065.

(In reply to Walter Bright from comment #16) > That seems very unlikely, as those changes were dated June 2012, long before > 2.065. The issue existed before 2.065, but was somehow masked for a while.

This is not a regression, it never worked. Nevertheless, I'm working on a fix.

partial fix: https://github.com/D-Programming-Language/druntime/pull/901

Rest of the fix: https://github.com/D-Programming-Language/dmd/pull/3784

Commits pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/4a8ca65688d2f5be290087c4b1f078c508fc7272 parial Issue 9449 - Static arrays of 128bit types segfault on initialization. Incorrect calling of memset128ii https://github.com/D-Programming-Language/druntime/commit/3626a88e1dee58a88969f6bbfc5a1b1c74d0dee5 Merge pull request #901 from WalterBright/fix9449 [critical] partial Issue 9449 - Static arrays of 128bit types segfault on initializa...

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/d9e770828558b9ad5578e6d02bbe5ee54b493326 fix Issue 9449 - Static arrays of 128bit types segfault on initialization. Incorrect calling of memset128ii https://github.com/D-Programming-Language/dmd/commit/c3ccc4168594b049e3619c2bb5c3016592f8a454 Merge pull request #3784 from WalterBright/fix9449 [critical] fix Issue 9449 - Static arrays of 128bit types segfault on initializatio...

Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/eaef472a9e4ded15360a8a8de9c36b8b3cd07059 fix Issue 9449 - Static arrays of 128bit types segfault on initialization. Incorrect calling of memset128ii

Commit pushed to 2.066 at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/ff80144f49f7ca8dd376a8ce671c1600d033506a Merge pull request #901 from WalterBright/fix9449 [critical] partial Issue 9449 - Static arrays of 128bit types segfault on initializa...

Commit pushed to 2.066 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/7a12ad9615931b5247c7c1d5fa29d969cc6aa1cc Merge pull request #3784 from WalterBright/fix9449 [critical] fix Issue 9449 - Static arrays of 128bit types segfault on initializatio...

Commit pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/ff80144f49f7ca8dd376a8ce671c1600d033506a Merge pull request #901 from WalterBright/fix9449

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/7a12ad9615931b5247c7c1d5fa29d969cc6aa1cc Merge pull request #3784 from WalterBright/fix9449

This still segfaults with dmd 2.066 (64-bit linux): void main() { float[4][2] m = [[2.0, 1, 3, 4], [2.0, 1, 3, 4]]; } Dump of assembler code for function _Dmain: 0x000000000041a160 <+0>: push rbp 0x000000000041a161 <+1>: mov rbp,rsp 0x000000000041a164 <+4>: sub rsp,0x40 0x000000000041a168 <+8>: lea rax,[rbp-0x30] 0x000000000041a16c <+12>: movabs rsi,0x2 0x000000000041a176 <+22>: movss xmm0,DWORD PTR [rip+0x183e2] # 0x432560 0x000000000041a17e <+30>: movss DWORD PTR [rbp-0x10],xmm0 0x000000000041a183 <+35>: mov ecx,0x3f800000 0x000000000041a188 <+40>: mov DWORD PTR [rbp-0x40],ecx 0x000000000041a18b <+43>: movss xmm1,DWORD PTR [rbp-0x40] 0x000000000041a190 <+48>: movss DWORD PTR [rbp-0xc],xmm1 0x000000000041a195 <+53>: movss xmm2,DWORD PTR [rip+0x183c7] # 0x432564 0x000000000041a19d <+61>: movss DWORD PTR [rbp-0x8],xmm2 0x000000000041a1a2 <+66>: movss xmm3,DWORD PTR [rip+0x183be] # 0x432568 0x000000000041a1aa <+74>: movss DWORD PTR [rbp-0x4],xmm3 0x000000000041a1af <+79>: lea rdi,[rbp-0x10] 0x000000000041a1b3 <+83>: push QWORD PTR [rdi+0x8] 0x000000000041a1b6 <+86>: push QWORD PTR [rdi] 0x000000000041a1b8 <+88>: mov rdi,rax 0x000000000041a1bb <+91>: call 0x41ad5c <_memset128ii> 0x000000000041a1c0 <+96>: add rsp,0x10 0x000000000041a1c4 <+100>: xor eax,eax 0x000000000041a1c6 <+102>: leave 0x000000000041a1c7 <+103>: ret

Reopened for this: void main() { float[4][2] m = [[2.0, 1, 3, 4], [2.0, 1, 3, 4]]; // segfault }

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/30b689c4ddeefe73bff05caba181447329621d57 add test case for Issue 9449 https://github.com/D-Programming-Language/dmd/commit/e308d06df5cb6447728f3a3e05259fc835acf677 Merge pull request #4049 from WalterBright/fix9449 add test case for Issue 9449

Can't reproduce on the auto-tester (Git master). Please open a new issue with more details about your system configuration if the problem still persists.

Commits pushed to 2.067 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/30b689c4ddeefe73bff05caba181447329621d57 add test case for Issue 9449 https://github.com/D-Programming-Language/dmd/commit/e308d06df5cb6447728f3a3e05259fc835acf677 Merge pull request #4049 from WalterBright/fix9449

Hi ! I've hit this bug with GIT HEAD dmd and vibe.d built with win32driver. I've disassembled .exe and it crashes with movdqa, the address is misaligned. I've changed movdqa opcode to movdqu in dmd's code and all works OK now.

(In reply to Temtaime from comment #34) > Hi ! > I've hit this bug with GIT HEAD dmd and vibe.d built with win32driver. > > I've disassembled .exe and it crashes with movdqa, the address is misaligned. > I've changed movdqa opcode to movdqu in dmd's code and all works OK now. Works with 2.067.0-b4, closing this. Please open a new ticket if you have this problem on a win32 machine.