www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10966] New: Leaked destruction in static array postblit

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10966

           Summary: Leaked destruction in static array postblit
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: monarchdodra gmail.com



This is a study of what happens in a static array, if a postblit throws. Tested
with 2.063 and 2.063.2:

The "caught" issue is that if we are doing "1-element to N-element" postblit,
then the "so-far contructed" objects will not be destroyed:

//----
import std.stdio;

int k;

struct S
{
    int a;
    this(this)
    {
        writef("postblitting %s... ", a);
        if (k++ == 2)
        {
            writefln("FAILED!");
            throw new Exception("BOOM!");
        }
        a += 10;
        writefln("OK => %s", a);
    }
    ~this()
    {
        writefln("destroying %s", a);
    }
}

void main()
{
    S s0     = S(0);
    S[4] ss1 = [S(1), S(2), S(3), S(4)];
    S[4] ss2  =
        ss1; //Case N to N
        //s0; //Case 1 to N
}
//----

Version N to N:
//----
ostblitting 1... OK => 11
postblitting 2... OK => 12
postblitting 3... FAILED!
destroying 12
destroying 11
destroying 4
destroying 3
destroying 2
destroying 1
destroying 0
//----
Comment: This behavior is correct: Two items have been constructed, they are
both destructed.

Version 1 to N:
//----
ostblitting 1... OK => 11
postblitting 2... OK => 12
postblitting 3... FAILED!
destroying 4
destroying 3
destroying 2
destroying 1
destroying 0
//----
Comment: This behavior is IN-correct: Two items have been constructed (11 and
12), yet neither gets destroyed.



If I may, I think the "root" issue is that the way static array postblit is
implemented is wrong ("eg, the call to typeid(S[4]).postblit"). Currently, all
it does is "call postblit 1 by 1, until it succeeds or fails". If it fails
though, it then relies on the compiler to know which single items in the array
have been constructed, and then destroy them individually.

It should be the postblit itself that deconstructs the "so-far-built" items in
the array.

For the compiler, there should be 1 and only 1 item: The array. It's state
should be binary: Built or not built. Period. A related issue (I'm about to
file) is how static array assignment works.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 05 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10966


Kenji Hara <k.hara.pg gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Severity|normal                      |critical



This is -O switch issue with dmd.
Test following reduced case.

--------
extern(C) int printf(const char*, ...);

int k;

struct S
{
    int a;
    this(this)
    {
        printf("postblitting %d... ", a);
        if (k++ == 2)
        {
            printf("FAILED!\n");
            throw new Exception("BOOM!");
        }
        a += 10;
        printf("OK => %d\n", a);
    }
    ~this()
    {
        printf("destroying %d\n", a);
    }
}

void main()
{
    S s0     = S(0);
    S[4] ss1 = [S(1), S(2), S(3), S(4)];
    S[4] ss2 = s0;  //Case 1 to N
    // call _d_arraysetctor defined in this module
    // instead of the one stored in druntime
}

// copy from druntime/src/rt/arrayassign.d
extern (C) void* _d_arraysetctor(void* p, void* value, int count, TypeInfo ti)
{
    import core.stdc.string : memcpy;

    void* pstart = p;   // with -O, variable pstart is wrongly made an alias of
p
    auto element_size = ti.tsize;

    try
    {
        foreach (i; 0 .. count)
        {
            // Copy construction is defined as bit copy followed by postblit.
            memcpy(p, value, element_size);
            ti.postblit(p);
            p += element_size;
        }
    }
    catch (Throwable o)
    {
        // Destroy, in reverse order, what we've constructed so far
        while (p > pstart)
        {
            p -= element_size;
            ti.destroy(p);
        }
        printf("--end\n");

        throw o;
    }
    return pstart;
}
--------

Without -O switch, _d_arraysetctor works as expected.
With -O switch, it doesn't work correctly.

Today druntime is normally compiled with -O switch, therefore this problem
occurs always.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 05 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10966




More reduced test case:

extern(C) int printf(const char*, ...);

void main()
{
    int s;
    int[4] ss;
    bug10966(&ss[0], &s, ss.length, int.sizeof);
}

void* bug10966(void* p, void* value, int count, size_t element_size)
{
    // with -O, variable pstart is wrongly made an alias of p
    void* pstart = p;

    try
    {
        printf("+pstart = %p, p = %p\n", pstart, p);
        foreach (i; 0 .. count)
        {
            if (i == 2) throw new Exception("dummy");
            printf("postblit p = %p\n", p);
            p += element_size;
        }
    }
    catch (Throwable o)
    {
        printf("-pstart = %p, p = %p\n", pstart, p);
        assert(p != pstart);    // asserts with -O
        while (p > pstart)
        {
            p -= element_size;
            printf("destroy p = %p\n", p);
        }
    }
    return pstart;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 05 2013