Bug 11139 – malloc/emplace resulting in memory corruption

Status
RESOLVED
Resolution
INVALID
Severity
blocker
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
Linux
Creation time
2013-09-29T06:42:00Z
Last change time
2013-11-04T01:10:24Z
Assigned to
nobody
Creator
puneet

Comments

Comment #0 by puneet — 2013-09-29T06:42:22Z
On my linux box I get: $ rdmd test.d 4000 $ rdmd -version=EMPLACE test.d 4000 Segmentation fault (core dumped) The code runs fine if I comment out line 19 or line 23 -- both of which are producing only a side effect. class Frop { // 01 int[] _var; // 02 int[] var() { // 03 return _var; // 04 } // 05 this() { // 06 this._var.length = 1; // 07 } // 08 } // 09 class Foo { // 10 long[] nodes; // 11 Frop[] frops; // 12 long[] table; // 13 this() { // 14 nodes.length = 120000; // 15 frops.length = 1; // 16 frops[0] = new Frop(); // 17 initTable(); // 18 zoo(frops[0].var); // 19 } // 20 void initTable() { // 21 import std.stdio; // 22 writeln(4000); // 23 table.length = 40000; // 24 } // 25 void zoo(int[] varset) {} // 26 } // 27 class Bar { // 28 Foo _foo; // 29 this() { // 30 version(EMPLACE) { // 31 import std.conv, core.stdc.stdlib; // 32 enum size_t size = __traits(classInstanceSize, Foo); // 33 // static assert(size is 32); // 64 on x64 // 34 void* tmp = core.stdc.stdlib.malloc(size); // 35 if (!tmp) // 36 throw new Exception("Memory allocation failed"); // 37 void[] mem = tmp[0..size]; // 38 _foo = emplace!(Foo)(mem); // 39 } // 40 else { // 41 _foo = new Foo(); // 42 } // 43 } // 44 } // 45 void main() { // 46 auto bar = new Bar; // 47 } // 48
Comment #1 by puneet — 2013-09-29T19:33:05Z
Another reduced testcase. This one does not have dependency on writeln. class Frop { // 01 void zoo(int ) { } // 02 } // 03 class Foo { // 04 int[] table; // 05 int[] table2; // 06 Frop frop; // 07 this() { // 08 frop = new Frop(); // 09 table2.length = 1; // 10 table.length = 320000; // 11 frop.zoo(0); // 12 } // 13 } // 14 class Bar { // 15 Foo foo; // 16 this() { // 17 version(EMPLACE) { // 18 import std.conv, core.stdc.stdlib; // 19 enum size_t size = __traits(classInstanceSize, Foo); // 20 static assert (size % size_t.sizeof is 0); // 21 void* tmp = core.stdc.stdlib.malloc(size); // 22 if (!tmp) // 23 throw new Exception("Memory allocation failed"); // 24 void[] mem = tmp[0..size]; // 25 foo = emplace!(Foo)(mem); // 26 } // 27 else { // 28 foo = new Foo(); // 29 } // 30 } // 31 } // 32 void main() { // 33 auto bar = new Bar; // 34 } // 35
Comment #2 by ibuclaw — 2013-10-13T06:51:53Z
This is what happens when you have GC'd memory only being referenced by non-GC'd memory.
Comment #3 by monarchdodra — 2013-10-13T07:48:43Z
(In reply to comment #2) > This is what happens when you have GC'd memory only being referenced by > non-GC'd memory. According to the original thread, this would be solved by GC.addRange. Apparently, Andrei tried it, but it didn't solve the problem. I haven't tried to double check. http://forum.dlang.org/thread/[email protected]#post-bpcfwijqpmlxozplsalx:40forum.dlang.org
Comment #4 by ibuclaw — 2013-10-13T07:52:28Z
(In reply to comment #3) > (In reply to comment #2) > > This is what happens when you have GC'd memory only being referenced by > > non-GC'd memory. > > According to the original thread, this would be solved by GC.addRange. > Apparently, Andrei tried it, but it didn't solve the problem. I haven't tried > to double check. > > http://forum.dlang.org/thread/[email protected]#post-bpcfwijqpmlxozplsalx:40forum.dlang.org Don't you mean GC.addRoot ?
Comment #5 by monarchdodra — 2013-10-13T08:01:09Z
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > This is what happens when you have GC'd memory only being referenced by > > > non-GC'd memory. > > > > According to the original thread, this would be solved by GC.addRange. > > Apparently, Andrei tried it, but it didn't solve the problem. I haven't tried > > to double check. > > > > http://forum.dlang.org/thread/[email protected]#post-bpcfwijqpmlxozplsalx:40forum.dlang.org > > Don't you mean GC.addRoot ? The thread made mention of addRange. AFAIK: addRoot is a way to say "this pointer exists even if you don't see it". addRange is "this memory location which isn't yours, scan it anyways". So I think in this context, addRange is correct. But I'll try some with both.
Comment #6 by ibuclaw — 2013-10-13T08:15:32Z
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (In reply to comment #2) > > > > This is what happens when you have GC'd memory only being referenced by > > > > non-GC'd memory. > > > > > > According to the original thread, this would be solved by GC.addRange. > > > Apparently, Andrei tried it, but it didn't solve the problem. I haven't tried > > > to double check. > > > > > > http://forum.dlang.org/thread/[email protected]#post-bpcfwijqpmlxozplsalx:40forum.dlang.org > > > > Don't you mean GC.addRoot ? > > The thread made mention of addRange. AFAIK: > > addRoot is a way to say "this pointer exists even if you don't see it". > > addRange is "this memory location which isn't yours, scan it anyways". > > So I think in this context, addRange is correct. But I'll try some with both. Both are correct. Having a look, GC.addRange seems more correct for struct/classes. And I can't re-produce the SEGV if I add in GC.addRange(tmp, size) - unlike what Andrei claims...
Comment #7 by ibuclaw — 2013-10-13T08:17:06Z
(In reply to comment #6) > > Both are correct. Having a look, GC.addRange seems more correct for > struct/classes. > See collection routine in the GC. --- // Scan roots[] mark(roots, roots + nroots); // Scan ranges[] for (n = 0; n < nranges; n++) mark(ranges[n].pbot, ranges[n].ptop); ---
Comment #8 by safety0ff.bugz — 2013-10-25T17:26:47Z
Debugging this it looks like it is calling/jumping into a heap location ([rcx+40] instead of static call,) instead of calling Frop.zoo.
Comment #9 by safety0ff.bugz — 2013-11-03T11:36:08Z
Ignore the above comment, if I add GC.addRange(tmp, size) after line 24 it stops segfaulting.
Comment #10 by ibuclaw — 2013-11-04T00:55:16Z
(In reply to comment #9) > Ignore the above comment, if I add GC.addRange(tmp, size) after line 24 it > stops segfaulting. (In reply to comment #8) > Debugging this it looks like it is calling/jumping into a heap location > ([rcx+40] instead of static call,) instead of calling Frop.zoo. It's called virtual. :~)
Comment #11 by ibuclaw — 2013-11-04T01:09:06Z
(In reply to comment #9) > Ignore the above comment, if I add GC.addRange(tmp, size) after line 24 it > stops segfaulting. No need to debug this, it's quite obvious what's going on (see my first comment). Don't think there's any real *fix* here other than using GC.malloc to allocate the memory instead of cstdlib.malloc. The GC does not track memory allocated outside of the GC, and it is unwise to think that GC'd memory is safe from collection if it's only being referenced by raw malloc'd memory.