Bug 5370 – Memory corruption in std.outbuffer, attribute NO_SCAN not cleared

OutBuffer class in std.outbuffer used to support storing pointers inside. Now underliying array is not scanned for roots, maybe it has something to do with array appending cache. This shows the problem: import std.outbuffer,std.stdio; import core.memory; int*[] f(){ auto outb = new OutBuffer; for(int i=0;i<1024;++i){ int* p = new int; *p = i; outb.write(cast(size_t)p); } return cast(int*[])outb.toBytes(); } void main(){ GC.disable(); auto arr = f(); assert(arr.length == 1024); assert(*arr[42] == 42); //size: 0 bits 0 - indicates that arr.ptr isn't start of block? writefln("Size %d bits %d",GC.sizeOf(arr.ptr),GC.getAttr(arr.ptr)); //GC.collect();//uncomment to get an assertion failure for(int i =0;i<1024;++i){ auto p = new int; *p = 0xdeadbeef; } assert(*arr[42] == 42); //this fails with GC.collect } Looking into outbuffer.d, the likely culprit is: line(84): GC.clrAttr(data.ptr, GC.BlkAttr.NO_SCAN); which isn't working. Patch for std.outbuffer, changes array type to void[] and removes this hack: @@ -37,7 +37,7 @@ class OutBuffer { - ubyte data[]; + void data[]; uint offset; invariant() @@ -52,10 +52,10 @@ } /********************************* - * Convert to array of bytes. + * Convert to array */ - ubyte[] toBytes() { return data[0 .. offset]; } + void[] toBytes() { return data[0 .. offset]; } /*********************************** * Preallocate nbytes more to the size of the internal buffer. @@ -81,7 +81,6 @@ if (data.length < offset + nbytes) { data.length = (offset + nbytes) * 2; - GC.clrAttr(data.ptr, GC.BlkAttr.NO_SCAN); } } @@ -109,7 +108,7 @@ void write(ubyte b) /// ditto { reserve(ubyte.sizeof); - this.data[offset] = b; + *cast(ubyte *)&data[offset] = b; offset += ubyte.sizeof; } @@ -183,7 +182,7 @@ void write(OutBuffer buf) /// ditto { - write(buf.toBytes()); + write(cast(ubyte[])buf.toBytes()); } /**************************************** @@ -193,7 +192,7 @@ void fill0(uint nbytes) { reserve(nbytes); - data[offset .. offset + nbytes] = 0; + *cast(ubyte*)&data[offset .. offset + nbytes] = 0; offset += nbytes; } @@ -331,7 +330,7 @@ for (uint i = offset; i > index; ) { --i; - data[i + nbytes] = data[i]; + *cast(ubyte*)&data[i + nbytes] = *cast(ubyte*)&data[i]; } offset += nbytes; }

Forgot to mention, that patch _indeed_ fixes the problem ;)

After discussion on the phobos list, this was an intentional behavior change not a regression. Closing.

I fail to see how the fact that GC.clrAttr(data.ptr, GC.BlkAttr.NO_SCAN); no longer clears the desired attribute for arrays is an "intentional behavior change". The only thing discussion proved is that nobody expected buffer to be scanned by GC in the first place, and that I'm the only one concerned with it and I rest my case. At any rate this confusing piece of code that is now effectively a glorified NOP should be removed.