Skip to content

Commit

Permalink
Fix appender code that might not initialize memory that might point a…
Browse files Browse the repository at this point in the history
…t huge allocations (#9084)
  • Loading branch information
schveiguy authored Nov 14, 2024
1 parent 4ef836c commit 567b375
Showing 1 changed file with 58 additions and 1 deletion.
59 changes: 58 additions & 1 deletion std/array.d
Original file line number Diff line number Diff line change
Expand Up @@ -3607,6 +3607,7 @@ if (isDynamicArray!A)
}
else
{
import core.stdc.string : memcpy, memset;
// Time to reallocate.
// We need to almost duplicate what's in druntime, except we
// have better access to the capacity field.
Expand All @@ -3618,6 +3619,15 @@ if (isDynamicArray!A)
if (u)
{
// extend worked, update the capacity
// if the type has indirections, we need to zero any new
// data that we requested, as the existing data may point
// at large unused blocks.
static if (hasIndirections!T)
{
immutable addedSize = u - (_data.capacity * T.sizeof);
() @trusted { memset(_data.arr.ptr + _data.capacity, 0, addedSize); }();
}

_data.capacity = u / T.sizeof;
return;
}
Expand All @@ -3633,10 +3643,20 @@ if (isDynamicArray!A)

auto bi = (() @trusted => GC.qalloc(nbytes, blockAttribute!T))();
_data.capacity = bi.size / T.sizeof;
import core.stdc.string : memcpy;
if (len)
() @trusted { memcpy(bi.base, _data.arr.ptr, len * T.sizeof); }();

_data.arr = (() @trusted => (cast(Unqual!T*) bi.base)[0 .. len])();

// we requested new bytes that are not in the existing
// data. If T has pointers, then this new data could point at stale
// objects from the last time this block was allocated. Zero that
// new data out, it may point at large unused blocks!
static if (hasIndirections!T)
() @trusted {
memset(bi.base + (len * T.sizeof), 0, (newlen - len) * T.sizeof);
}();

_data.tryExtendBlock = true;
// leave the old data, for safety reasons
}
Expand Down Expand Up @@ -4011,6 +4031,43 @@ if (isDynamicArray!A)
app2.toString();
}

// https://issues.dlang.org/show_bug.cgi?id=24856
@system unittest
{
import core.memory : GC;
import std.stdio : writeln;
import std.algorithm.searching : canFind;
GC.disable();
scope(exit) GC.enable();
void*[] freeme;
// generate some poison blocks to allocate from.
auto poison = cast(void*) 0xdeadbeef;
foreach (i; 0 .. 10)
{
auto blk = new void*[7];
blk[] = poison;
freeme ~= blk.ptr;
}

foreach (p; freeme)
GC.free(p);

int tests = 0;
foreach (i; 0 .. 10)
{
Appender!(void*[]) app;
app.put(null);
// if not a realloc of one of the deadbeef pointers, continue
if (!freeme.canFind(app.data.ptr))
continue;
++tests;
assert(!app.data.ptr[0 .. app.capacity].canFind(poison), "Appender not zeroing data!");
}
// just notify in the log whether this test actually could be done.
if (tests == 0)
writeln("WARNING: test of Appender zeroing did not occur");
}

//Calculates an efficient growth scheme based on the old capacity
//of data, and the minimum requested capacity.
//arg curLen: The current length
Expand Down

0 comments on commit 567b375

Please sign in to comment.