D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 18996 - Inserting a type containing indirections into an std.container Array causes SIGILL(4). Illegal Instruction.
Summary: Inserting a type containing indirections into an std.container Array causes S...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: druntime (show other issues)
Version: D2
Hardware: All All
: P1 regression
Assignee: Steven Schveighoffer
URL:
Keywords: pull
Depends on:
Blocks:
 
Reported: 2018-06-16 06:42 UTC by Paul Crane
Modified: 2020-03-21 03:56 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Paul Crane 2018-06-16 06:42:12 UTC
The following code causes a SIGILL or signal 4 on Linux.

import std.container;

struct Record
{
	string name;
}

struct Foo(T)
{
	alias RecordArray = Array!T;

	void test()
	{
		T value;
		array_.insert(value);
	}

	RecordArray array_;
}

void main(string[] arguments)
{
	Foo!Record foo;
	foo.test();
}

I don't have much experience with gdb so this is the best I could do:

(gdb) run
Starting program: /home/soulsbane/Projects/D/ssloc/ssloc 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".

Program received signal SIGILL, Illegal instruction.
0x0000555555614204 in _D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv ()
(gdb) bt
#0  0x0000555555614204 in _D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv ()
#1  0x00005555555f52c3 in gc_removeRange ()
#2  0x00005555555f4a05 in core.memory.GC.removeRange(const(void*)) ()
#3  0x00005555555e119e in _D3std9container5array__T5ArrayTS3app6RecordZQu7Payload7reserveMFNbNimZv (this=..., elements=1) at /usr/include/dlang/dmd/std/container/array.d:382
#4  0x00005555555e43f0 in _D3std9container5array__T5ArrayTS3app6RecordZQu7Payload__T10insertBackTQBnZQrMFNbNiQBzZm (this=..., elem=...) at /usr/include/dlang/dmd/std/container/array.d:404
#5  0x00005555555e433a in _D3std9container5array__T5ArrayTS3app6RecordZQu__T10insertBackTQBfZQrMFNbNiQBrZm (this=..., stuff=...) at /usr/include/dlang/dmd/std/container/array.d:831
#6  0x00005555555e0d2a in _D3app__T3FooTSQn6RecordZQq4testMFNbNiZv (this=...) at source/app.d:73
#7  0x00005555555e0c31 in D main (arguments=...) at source/app.d:82
(gdb) disas
Dump of assembler code for function _D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv:
   0x0000555555614044 <+0>:     push   %rbp
   0x0000555555614045 <+1>:     mov    %rsp,%rbp
   0x0000555555614048 <+4>:     sub    $0x40,%rsp
   0x000055555561404c <+8>:     mov    %rbx,-0x40(%rbp)
   0x0000555555614050 <+12>:    mov    %r12,-0x38(%rbp)
   0x0000555555614054 <+16>:    mov    %r13,-0x30(%rbp)
   0x0000555555614058 <+20>:    mov    %r14,-0x28(%rbp)
   0x000055555561405c <+24>:    mov    %rdi,-0x8(%rbp)
   0x0000555555614060 <+28>:    mov    %rsi,%rdx
   0x0000555555614063 <+31>:    mov    %rdi,%rcx
   0x0000555555614066 <+34>:    add    $0x28,%rcx
   0x000055555561406a <+38>:    mov    0x8(%rcx),%r8
   0x000055555561406e <+42>:    mov    (%rcx),%r9
   0x0000555555614071 <+45>:    test   %r8,%r8
   0x0000555555614074 <+48>:    je     0x555555614204 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+448>
   0x000055555561407a <+54>:    mov    %r9,%rcx
   0x000055555561407d <+57>:    cmp    %rdx,(%rcx)
   0x0000555555614080 <+60>:    jne    0x5555556141e4 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+416>
   0x0000555555614086 <+66>:    mov    -0x8(%rbp),%r8
   0x000055555561408a <+70>:    add    $0x28,%r8
   0x000055555561408e <+74>:    mov    %r8,-0x20(%rbp)
   0x0000555555614092 <+78>:    mov    0x8(%r8),%rsi
   0x0000555555614096 <+82>:    mov    (%r8),%rdx
   0x0000555555614099 <+85>:    lea    (%rsi,%rsi,2),%rsi
   0x000055555561409d <+89>:    lea    -0x18(%rdx,%rsi,8),%rsi
   0x00005555556140a5 <+97>:    mov    %rcx,%rdi
   0x00005555556140a8 <+100>:   movsq  %ds:(%rsi),%es:(%rdi)
   0x00005555556140aa <+102>:   movsq  %ds:(%rsi),%es:(%rdi)
   0x00005555556140ac <+104>:   movsq  %ds:(%rsi),%es:(%rdi)
   0x00005555556140ae <+106>:   xor    %eax,%eax
   0x00005555556140b0 <+108>:   mov    %al,-0x10(%rbp)
   0x00005555556140b3 <+111>:   mov    -0x20(%rbp),%r14
   0x00005555556140b7 <+115>:   mov    0x8(%r14),%r13
   0x00005555556140bb <+119>:   dec    %r13
   0x00005555556140be <+122>:   mov    %r13,%r9
   0x00005555556140c1 <+125>:   lea    (%r9,%r9,2),%rbx
   0x00005555556140c5 <+129>:   lea    0x0(,%rbx,8),%rbx
   0x00005555556140cd <+137>:   mov    %r13,%rdx
   0x00005555556140d0 <+140>:   or     $0x18,%rdx
   0x00005555556140d7 <+147>:   shr    $0x20,%rdx
   0x00005555556140db <+151>:   je     0x5555556140fa <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+182>
   0x00005555556140dd <+153>:   mov    %rbx,%rax
   0x00005555556140e0 <+156>:   movabs $0xaaaaaaaaaaaaaaab,%rdx
   0x00005555556140ea <+166>:   mul    %rdx
   0x00005555556140ed <+169>:   shr    $0x4,%rdx
   0x00005555556140f1 <+173>:   cmp    %r13,%rdx
   0x00005555556140f4 <+176>:   je     0x5555556140fa <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+182>
   0x00005555556140f6 <+178>:   movb   $0x1,-0x10(%rbp)
   0x00005555556140fa <+182>:   rex movsbl -0x10(%rbp),%esi
   0x00005555556140ff <+187>:   xor    $0x1,%sil
   0x0000555555614103 <+191>:   je     0x5555556141a7 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+355>
   0x0000555555614109 <+197>:   mov    %rbx,-0x18(%rbp)
   0x000055555561410d <+201>:   mov    0x8(%r14),%rcx
   0x0000555555614111 <+205>:   cmp    %rcx,%r13
   0x0000555555614114 <+208>:   jae    0x555555614153 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+271>
---Type <return> to continue, or q <return> to quit---
   0x0000555555614116 <+210>:   mov    %r13,%rax
   0x0000555555614119 <+213>:   lea    (%rax,%rax,2),%rsi
   0x000055555561411d <+217>:   lea    0x0(,%rsi,8),%rsi
   0x0000555555614125 <+225>:   add    (%r14),%rsi
   0x0000555555614128 <+228>:   mov    %rcx,%rdx
   0x000055555561412b <+231>:   sub    %rax,%rdx
   0x000055555561412e <+234>:   je     0x555555614153 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+271>
   0x0000555555614130 <+236>:   mov    %rsi,%rbx
   0x0000555555614133 <+239>:   lea    (%rdx,%rdx,2),%r12
   0x0000555555614137 <+243>:   lea    0x0(,%r12,8),%r12
   0x000055555561413f <+251>:   add    %rsi,%r12
   0x0000555555614142 <+254>:   mov    %rbx,%rdi
   0x0000555555614145 <+257>:   callq  0x555555618944 <_D2rt4util9container6common__T7destroyTS2gc11gcinterface5RangeZQBhFNaNbNiNfKQBlZv>
   0x000055555561414a <+262>:   add    $0x18,%rbx
   0x000055555561414e <+266>:   cmp    %r12,%rbx
   0x0000555555614151 <+269>:   jb     0x555555614142 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+254>
   0x0000555555614153 <+271>:   mov    (%r14),%rsi
   0x0000555555614156 <+274>:   mov    -0x18(%rbp),%rdi
   0x000055555561415a <+278>:   callq  0x5555555fa40c <_D2rt4util9container6common8xreallocFNbNiPvmZQe>
   0x000055555561415f <+283>:   mov    %rax,(%r14)
   0x0000555555614162 <+286>:   mov    0x8(%r14),%rcx
   0x0000555555614166 <+290>:   cmp    %r13,%rcx
   0x0000555555614169 <+293>:   jae    0x5555556141cb <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+391>
   0x000055555561416b <+295>:   lea    (%rcx,%rcx,2),%rsi
   0x000055555561416f <+299>:   lea    0x0(,%rsi,8),%rsi
   0x0000555555614177 <+307>:   add    %rax,%rsi
   0x000055555561417a <+310>:   mov    %r13,%rdx
   0x000055555561417d <+313>:   sub    %rcx,%rdx
   0x0000555555614180 <+316>:   je     0x5555556141cb <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+391>
   0x0000555555614182 <+318>:   mov    %rsi,%rbx
   0x0000555555614185 <+321>:   lea    (%rdx,%rdx,2),%r12
   0x0000555555614189 <+325>:   lea    0x0(,%r12,8),%r12
   0x0000555555614191 <+333>:   add    %rsi,%r12
   0x0000555555614194 <+336>:   mov    %rbx,%rdi
   0x0000555555614197 <+339>:   callq  0x555555618980 <_D2rt4util9container6common__T10initializeTS2gc11gcinterface5RangeZQBlFNaNbNiKQBjZv>
   0x000055555561419c <+344>:   add    $0x18,%rbx
   0x00005555556141a0 <+348>:   cmp    %r12,%rbx
   0x00005555556141a3 <+351>:   jb     0x555555614194 <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+336>
   0x00005555556141a5 <+353>:   jmp    0x5555556141cb <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+391>
   0x00005555556141a7 <+355>:   xor    %edi,%edi
   0x00005555556141a9 <+357>:   callq  0x555555607b1c <_D4core9exception__T11staticErrorTCQBhQBf16OutOfMemoryErrorTbZQBqFNaNbNibZQBo>
   0x00005555556141ae <+362>:   mov    %rax,%rdi
   0x00005555556141b1 <+365>:   callq  0x5555555f5af4 <_d_throwdwarf>
   0x00005555556141b6 <+370>:   mov    -0x40(%rbp),%rbx
   0x00005555556141ba <+374>:   mov    -0x38(%rbp),%r12
   0x00005555556141be <+378>:   mov    -0x30(%rbp),%r13
   0x00005555556141c2 <+382>:   mov    -0x28(%rbp),%r14
   0x00005555556141c6 <+386>:   mov    %rbp,%rsp
   0x00005555556141c9 <+389>:   pop    %rbp
   0x00005555556141ca <+390>:   retq   
   0x00005555556141cb <+391>:   mov    %r13,0x8(%r14)
   0x00005555556141cf <+395>:   mov    -0x40(%rbp),%rbx
   0x00005555556141d3 <+399>:   mov    -0x38(%rbp),%r12
   0x00005555556141d7 <+403>:   mov    -0x30(%rbp),%r13
   0x00005555556141db <+407>:   mov    -0x28(%rbp),%r14
   0x00005555556141df <+411>:   mov    %rbp,%rsp
---Type <return> to continue, or q <return> to quit---
   0x00005555556141e2 <+414>:   pop    %rbp
   0x00005555556141e3 <+415>:   retq   
   0x00005555556141e4 <+416>:   mov    $0x18,%eax
   0x00005555556141e9 <+421>:   add    %rax,%rcx
   0x00005555556141ec <+424>:   lea    (%r8,%r8,2),%rbx
   0x00005555556141f0 <+428>:   lea    0x0(,%rbx,8),%rbx
   0x00005555556141f8 <+436>:   add    %r9,%rbx
   0x00005555556141fb <+439>:   cmp    %rbx,%rcx
   0x00005555556141fe <+442>:   jb     0x55555561407d <_D2gc4impl5protoQo7ProtoGC11removeRangeMFNbNiPvZv+57>
=> 0x0000555555614204 <+448>:   ud2    
   0x0000555555614206 <+450>:   mov    -0x40(%rbp),%rbx
   0x000055555561420a <+454>:   mov    -0x38(%rbp),%r12
   0x000055555561420e <+458>:   mov    -0x30(%rbp),%r13
   0x0000555555614212 <+462>:   mov    -0x28(%rbp),%r14
   0x0000555555614216 <+466>:   mov    %rbp,%rsp
   0x0000555555614219 <+469>:   pop    %rbp
   0x000055555561421a <+470>:   retq   
End of assembler dump.

My system:
DMD: DMD64 D Compiler v2.080.1-dirty
Manjaro Linux
Kernel 4.14.48-2
Processor: core i7

I don't think it's my system since run.dlang.io gives this output:

2.061   to 2.078.1: Success and no output
Since      2.079.0: Failure with output: Error: program killed by signal 4

So it looks like 2.079 that this issue cropped up.
Thanks!
Comment 1 Paul Crane 2018-06-18 04:57:05 UTC
Actually, all it takes is:

import std.container;

void main(string[] arguments)
{
    Array!string str;
    str.insert("hello");
}

Up to      2.078.1: Success and no output
Since      2.079.0: Failure with output: Error: program killed by signal 4

Either I'm missing something or this is a really bad bug. I would think if it was a bug how could it have gone unnoticed?
Comment 2 Paul Crane 2018-06-18 05:32:18 UTC
After even more testing it looks like only Array's with strings are causing this issue:

struct Test
{
    string value;
}

void main(string[] arguments)
{
    //Cause SIGILL
    Array!string str;
    str.insert("hello");

    //Works
    Array!bool boolValues;
    boolValues.insert(true);

    //Works
    Array!int intValues;
    intValues.insert(10);

    //Causes SIGILL if struct contains a string. Works otherwise.
    Test value;
    Array!Test test;
    test.insert(value);
    
}

All string types(wstring, dstring, string) will fail.
Comment 3 basile-z 2018-06-18 05:45:56 UTC
This is caused by an operation on a string literal data, which are known to be read-only.

For example:


---
void main(string[] arguments)
{
    Array!string str;
    str.insert("hello".idup);
}
---

works finely because string data are now  on the heap, not in the segment where the literal resides.
Comment 4 basile-z 2018-06-18 05:51:11 UTC
Spotted the bug.

The problem is actually that the garbage collector is not initialized. 2079 introduced lazy initialization.

---
void main(string[] arguments)
{
    auto a = "init gc".dup; // force GC init...
    Array!string str;
    str.insert("hello"); 
}
---

no more SIGILL.
Comment 5 basile-z 2018-06-18 07:07:41 UTC
Problem is confirmed: Adding `gc_init()` in the Array `shared static this()` makes the problem going away. However for now `gc_init()` is private. 

One way to fix the issue is make `gc_init` public in druntime and add an explicit init call in `Array`. The other would be to make a dummy alloc if `Array` element has indirections.
Comment 6 Paul Crane 2018-06-18 07:53:40 UTC
Thanks a lot for looking into this. Your fix does work for me as well. Thanks again it's much appreciated!
Comment 7 Steven Schveighoffer 2018-06-18 13:53:01 UTC
The issue is that Array!string (or Array!T where T has indirections) is going to add its array as a range to the GC whenever it allocates one.

What it does AFTER though is remove it's previous array as a range. When the Array has not been initialized yet, this is effectively calling GC.removeRange(null).

The ProtoGC was asserting on that, generating the illegal instruction. However, the normal GC handled it just fine, which is why initializing the GC solved the problem.

The ProtoGC should act exactly like the normal GC until the normal GC takes over, so this was a bug in the ProtoGC.

PR: https://github.com/dlang/druntime/pull/2220
Comment 8 Paul Crane 2018-06-18 20:37:50 UTC
Thanks Steven for finding the bug and fixing it! It is appreciated!
Comment 9 github-bugzilla 2018-06-25 18:33:11 UTC
Commits pushed to master at https://github.com/dlang/druntime

https://github.com/dlang/druntime/commit/1b0af1ee6030b233eba7d29629c5ea995d5df944
Fix issue 18996 - ProtoGC should support removing roots and ranges
that were not originally added. Behavior is now consistent with
conservative GC.

https://github.com/dlang/druntime/commit/90c140f439e3ee719d8a01455dee5dc976e1f9d1
Merge pull request #2220 from schveiguy/fix18996

Fix issue 18996 - ProtoGC should support removing roots and ranges that were not originally added