[Issue 14303] New: rt.util.container.array.Array unittest contains invalid code

via Digitalmars-d-bugs digitalmars-d-bugs at puremagic.com
Tue Mar 17 23:07:59 PDT 2015


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 at puremagic.com
          Reporter: braddr at 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);

--


More information about the Digitalmars-d-bugs mailing list