• via Digitalmars-d-bugs (64/64) Mar 17 2015 https://issues.dlang.org/show_bug.cgi?id=14303

via Digitalmars-d-bugs <digitalmars-d-bugs puremagic.com> writes:

https://issues.dlang.org/show_bug.cgi?id=14303

          Issue ID: 14303
           Summary: rt.util.container.array.Array unittest contains
                    invalid code
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: critical
          Priority: P1
         Component: druntime
          Assignee: nobody puremagic.com
          Reporter: braddr puremagic.com

The last block of unit tests:

unittest
{
    alias RC = common.RC;
    Array!RC ary;

    size_t cnt;
    assert(cnt == 0);
    ary.insertBack(RC(&cnt));
    assert(cnt == 1);
    ary.insertBack(ary.front); // <---
    assert(cnt == 2);
    ary.popBack();
    assert(cnt == 1);
    ary.popBack();
    assert(cnt == 0);
}

The marked line takes the front element of array and passes it by reference to
ary.insertBack.  The problem is that insertBack can realloc the array which can
cause it to be moved, invalidating the reference.  I'm seeing this happen and
resulting in a segfault.  For evidence of the problem, changing
Array.insertBack to:

    void insertBack()(auto ref T val)
    {
        static if (is(T == common.RC)) printf("val._cnt: %p\n", val._cnt);
.fflush(.stdout);
        length = length + 1;
        static if (is(T == common.RC)) printf("val._cnt: %p\n", val._cnt);
.fflush(.stdout);
        back = val;
    }

val._cnt: 00000000001AF960 (matches &cnt)
val._cnt: 00000000001A003C (doesn't match)

Adding more printf's to clarify the flow between those two points:

Array.length _ptr before xrealloc: 0000000000312900
Array.length _ptr after xrealloc:  0000000000312A60
RC.length grow
common.initialize &t: 0000000000312A70, t._cnt: 0000000000000000
common.initialize memset

Not sure what caused the old _cnt pointer to get overwritten with a new value,
but it obviously was, thankfully exposing the problem.  The code certainly
_looks_ innocent enough.  It took way more digging than it should have for me
to realize what the problem is.  I hate to just remove the test case though.

Maybe replace:
    ary.insertBack(ary.front);

with:
    ary.insertBack(RC(&cnt));
    assert(cnt == 2);
    ary.back = ary.front;
    assert(cnt == 2);

--