• d-bugmail puremagic.com (66/66) Nov 12 https://issues.dlang.org/show_bug.cgi?id=24856

d-bugmail puremagic.com writes:

https://issues.dlang.org/show_bug.cgi?id=24856

          Issue ID: 24856
           Summary: std.array.Appender.ensureAddable can create stale
                    memory references
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P1
         Component: phobos
          Assignee: nobody puremagic.com
          Reporter: schveiguy gmail.com

The scenario:

1. A large block of memory is allocated, and a small block of memory is
allocated with 8 pointers to the large block.
2. The small block is freed
3. You use std.array.Appender!(void*[]), and append one pointer of `null`

What happens under the hood in step 3:

1. It calls ensureAddable(1).
2. ensureAddable realizes the current capacity (0) is too small, so it
calculates a new capacity (minimum 8).
3. It calls GC.qalloc(8 * elemsize), which tells the GC "I want a block that
holds 8*elemsize bytes at least".
4. The newly freed block is used, because it's the right size.
5. Since T contains pointers (it's a void*), the GC will zero everything
*after* the 8 elements asked for.
6. The Appender adds the one pointer, but does *not* initialize the other 7
asked for.

If the other elements are not appended to, then the stale data which points at
the large block is still there. This keeps the large block pinned, since the GC
will scan the entire block.

In fact, this is exactly what happened to me, and it took an inordinate amount
of resources to track this down.

I was able to reproduce the issue on run.dlang.io with this code, but no
guarantee it always works:

```
void main()
{
    import std.array;
    import core.memory;
    import std.stdio;

    void*[] blk = new void*[7];
    blk[] = cast(void*)0xdeadbeef;
    GC.free(blk.ptr);
    foreach(i; 0 .. 10000) {
        Appender!(void*[]) app;
        app.put(null);
        if(app.data.ptr !is blk.ptr) continue;
            writeln(app.data.ptr[0 .. 7]);
        break;
    }
}
```

The output from this was:

[null, 560745EC8560, DEADBEEF, DEADBEEF, DEADBEEF, DEADBEEF, DEADBEEF]

DEADBEEF represents the large allocation that is kept in memory.

I used a 7-element array, because 8 elements would have grown into the next
size block. But you can clearly see, the stale data is present.

The fix is to zero any new data asked for, but not initialized, inside
Appender. Both in the `extend` call and the `qalloc` call, when the element
type contains pointers.

Reference to the code in question:
https://github.com/dlang/phobos/blob/db798688e4dd492d6f739cac98069518e8290e97/std/array.d#L3613

--