D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6153 - Inserting to An Array!T inside an Array!(Array!T) causes a segfault.
Summary: Inserting to An Array!T inside an Array!(Array!T) causes a segfault.
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: Andrei Alexandrescu
URL:
Keywords:
: 7901 (view as issue list)
Depends on:
Blocks:
 
Reported: 2011-06-12 21:04 UTC by Victor
Modified: 2013-03-04 15:24 UTC (History)
5 users (show)

See Also:


Attachments
testcase (363 bytes, text/x-dsrc)
2011-06-12 21:04 UTC, Victor
Details

Note You need to log in before you can comment on or make changes to this issue.
Description Victor 2011-06-12 21:04:21 UTC
Created attachment 996 [details]
testcase

See the attached test case. Running it causes a segfault on glibc.
Comment 1 SomeDude 2012-04-24 07:56:49 UTC
The following works correctly:

import std.stdio;
import std.container;

int main() {
    alias Array!int Arr1D;
    alias Array!Arr1D Arr2D;
	
    auto arr1D = Arr1D();
    auto arr2D = Arr2D();
    
    writefln("arr1D %s of size = %d", arr1D, arr1D.length());
    arr1D.insert(1); 
    arr1D.insert(2);
    writefln("arr1D %s of size = %d", arr1D[0], arr1D.length());
    
    arr2D.insert(arr1D);
    writefln("arr2D %s of size = %d", arr2D, arr2D.length());
    writeln(arr2D[0][0], arr2D[0][1]);
    return 0;
}

It produces the output:
PS E:\DigitalMars\dmd2\samples> rdmd a.d
arr1D Array!(int)(RefCounted!(Payload,cast(RefCountedAutoInitialize)0)(_RefCounted(null))) of size = 0
arr1D 1 of size = 2
arr2D Array!(Array!(int))(RefCounted!(Payload,cast(RefCountedAutoInitialize)0)(_RefCounted(A623F0))) of size = 1
12
PS E:\DigitalMars\dmd2\samples>


However inserting ints in arr1D *after* having inserted it in arr2D produces access violations:

import std.stdio;
import std.container;

int main() {
    alias Array!int Arr1D;
    alias Array!Arr1D Arr2D;
	
    auto arr1D = Arr1D();
    auto arr2D = Arr2D();
    arr2D.insert(arr1D);
    writefln("arr2D %s of size %d", arr2D[0], arr2D.length());
	
    arr2D[0].insert(1);
    arr2D[0].insert(2);
    writefln("array size after insert: %d", arr2D[0].length());
    //writeln(arr2D[0][0], arr2D[0][1]);
    return 0;
}

produces:
PS E:\DigitalMars\dmd2\samples> rdmd bug.d
arr2D Array!(int)(RefCounted!(Payload,cast(RefCountedAutoInitialize)0)(_RefCounted(null))) of size 1
array size after insert: 0
PS E:\DigitalMars\dmd2\samples>

It seems that the reason is, as long as arr1D has no elements inserted via insert, it's not allocated and arr2D[0] is null.

See also issue 7901.
Comment 2 SomeDude 2012-04-24 09:05:54 UTC
*** Issue 7901 has been marked as a duplicate of this issue. ***
Comment 3 Matthias Walter 2012-07-18 03:14:20 UTC
The reason for the failure is that for

arr2D[0].insert(1);

the opIndex() method is invoked which returns by value. In theory this is okay since the array is stored by reference and the returned value contains a reference to the payload data of the inner array.

What happens here is that arr2D[0] is uninitialized (the pointer to the RefCounted object is null) and this guy is returned. It is then changed (the insert method initializes the temporary and creates the ref-counted object with the inserted content).

One way to resolve this is to make opIndex return a reference. Or is there an alternative?
Comment 4 Christophe Travert 2012-07-18 06:46:14 UTC
(In reply to comment #3)
> The reason for the failure is that for
> 
> arr2D[0].insert(1);
> 
> the opIndex() method is invoked which returns by value. In theory this is okay
> since the array is stored by reference and the returned value contains a
> reference to the payload data of the inner array.
> 
> What happens here is that arr2D[0] is uninitialized (the pointer to the
> RefCounted object is null) and this guy is returned. It is then changed (the
> insert method initializes the temporary and creates the ref-counted object with
> the inserted content).
> 
> One way to resolve this is to make opIndex return a reference. Or is there an
> alternative?

I think both should be corrected:
 - uninitialised Arrays do not behave as reference values: you can copy them, change the copy, and this does not affect the original. This breaks the intended 'reference value' behavior. The cost of creating a payload for each empty Array may be compensated by not having to check the payload is initialised in each of Array's methods.
 - opIndex should return a reference. Any struct with a non-const method will suffer the same problem when 'array[i].method' is called: the method is called on a copy of the contained data.

Note that Array!bool cannot return a reference to a bool, and can't be corrected the same way. I think this is not a problem, but I may miss something.
Comment 5 Andrei Alexandrescu 2013-03-04 15:24:10 UTC
Works now, probably has been fixed along with other bugs. Feel free to reopen if I missed something.