Thread overview
[Issue 6098] New: CTFE static array corruption of data
Jun 03, 2011
Andrej Mitrovic
[Issue 6098] Static array corruption of data
Jun 03, 2011
Andrej Mitrovic
Jun 03, 2011
Andrej Mitrovic
Jun 03, 2011
Andrej Mitrovic
Jun 03, 2011
Andrej Mitrovic
Jun 03, 2011
kennytm@gmail.com
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098

           Summary: CTFE static array corruption of data
           Product: D
           Version: D2
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Severity: major
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: andrej.mitrovich@gmail.com


--- Comment #0 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2011-06-03 06:15:46 PDT ---
import std.math;
import std.stdio;
import std.range;

auto makeSine(int TableSize)()
{
    double[TableSize] result;
    // double[] result = new double[](TableSize);  // this fixes the issue

    foreach (index, ref sample; result)
    {
        sample = cast(double)sin((cast(double)index / cast(double)TableSize) *
PI * 2.);
    }
    writeln(result);
    return cycle(result);
}

void main()
{
    auto sineTable = makeSine!(10);

    writeln(take(sineTable, 10));
    writeln(take(sineTable, 10));

    double[] arr;
    foreach (sample; take(sineTable, 10))
    {
        arr ~= sample;
    }
    writeln(arr);
}

Results:
[0, 0.587785, 0.951057, 0.951057, 0.587785, -5.42101e-20, -0.587785, -0.951057,
-0.951057, -0.587785]
[6.14906e-318, 2.59842e-307, 2.31044e-317, 0.951057, 0.587785, -5.42101e-20,
2.64149e-308, 2.08636e-317, 1.82805e-307, 2.641e-308]
[6.14906e-318, 2.59842e-307, 2.31044e-317, 2.59842e-307, 2.31044e-317,
-5.42101e-20, 2.64149e-308, 2.08636e-317, 1.82805e-307, 2.641e-308]
[6.14906e-318, 2.64128e-308, 2.49803e-307, 2.2817e-314, 6.3666e-314,
-5.42101e-20, 2.64149e-308, -nan, 1.82805e-307, 2.64126e-308]

If bugzilla makes it hard to read I've pasted the results here: http://paste.pocoo.org/show/400027/

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|DMD                         |Phobos
            Summary|CTFE static array           |Static array corruption of
                   |corruption of data          |data


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2011-06-03 06:25:49 PDT ---
Actually this has nothing to do with CTFE:

import std.math;
import std.stdio;
import std.range;

auto makeSine()
{
    double[10] result;
    //~ double[] result = new double[](TableSize);  // this fixes the issue

    foreach (index, ref sample; result)
    {
        sample = cast(double)sin((cast(double)index / cast(double)10) * PI *
2.);
    }
    writeln(result);
    return cycle(result);
}

void main()
{
    auto sineTable = makeSine();
    writeln(take(sineTable, 10));
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |schveiguy@yahoo.com
         Resolution|                            |INVALID


--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> 2011-06-03 06:34:16 PDT ---
Actually, it's expected :)  result is on the stack, which goes away when you return from the function (this is why the heap-allocating version has no issues).

I'm assuming the whole shenanigans with cycle and take are so the compiler doesn't complain?  That should probably be a hint...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098



--- Comment #3 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2011-06-03 06:42:45 PDT ---
Smiley face, but this is a subtle thing without any warnings from the compiler.

I'm using cycle here for good purpose and not to drown the compiler, take would be used to fill a buffer of some certain length.

I'll just use the heap, it's not a problem. I just assumed using a range on stack-allocated data would copy the data when returning the range from the function.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098



--- Comment #4 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2011-06-03 06:45:00 PDT ---
Actually I can just dup manually instead of using the heap:

return cycle(result.dup);

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098



--- Comment #5 from Steven Schveighoffer <schveiguy@yahoo.com> 2011-06-03 06:58:42 PDT ---
(In reply to comment #3)
> I'm using cycle here for good purpose and not to drown the compiler, take would be used to fill a buffer of some certain length.

OK, it just seemed strange that you were cycling something of length 10, and then only taking 10 elements from it.  I wondered what the purpose of this was.

Sorry for the assumption.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2011-06-03 07:16:40 PDT ---
Here's a more complete example: http://paste.pocoo.org/show/400066/

Basically it's a sinewave that repeats forever. Changing the stepping will increase the pitch, the buffer length depends on the soundcard (for example the soundcard might require a couple of buffers of a certain length to be filled each second), table size basically increases the resolution or smoothness of the curve.

I think that's it, anyway. I'm still new to the whole concept of audio programming. :)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #7 from bearophile_hugs@eml.cc 2011-06-03 10:21:52 PDT ---
(In reply to comment #4)
> Actually I can just dup manually instead of using the heap:
> 
> return cycle(result.dup);

.dup creates a copy on the heap.


This is a safer implementation:

import std.stdio, std.math, std.algorithm, std.range;

auto makeSine() {
    return cycle(map!q{ sin(a * PI) }(iota(0, 2, 0.2)));
}

void main() {
    auto sineTable = makeSine();
    writeln(take(sineTable, 10));
}


Regarding the unsafety of the version that uses a fixed sized array, I'd like the compiler to warn better against such bugs.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6098


kennytm@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kennytm@gmail.com


--- Comment #8 from kennytm@gmail.com 2011-06-03 12:55:56 PDT ---
(In reply to comment #7)
> (In reply to comment #4)
> > Actually I can just dup manually instead of using the heap:
> > 
> > return cycle(result.dup);
> 
> .dup creates a copy on the heap.
> 
> 
> This is a safer implementation:
> 
> import std.stdio, std.math, std.algorithm, std.range;
> 
> auto makeSine() {
>     return cycle(map!q{ sin(a * PI) }(iota(0, 2, 0.2)));
> }
> 
> void main() {
>     auto sineTable = makeSine();
>     writeln(take(sineTable, 10));
> }
> 
> 
> Regarding the unsafety of the version that uses a fixed sized array, I'd like the compiler to warn better against such bugs.

This will evaluate 'sin(a * PI)' N times, which totally goes against the reason of making a sine table. You should 'array' the 'map' before 'cycle'-ing.

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