Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Commit 891eaf7

Browse files
authored
Merge pull request #2755 from rainers/structinfo_lifetime
Issue 20155 - Allocating a struct with dtor on the GC heap can produce false pointers merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
2 parents 3781cab + 441376b commit 891eaf7

File tree

1 file changed

+104
-4
lines changed

1 file changed

+104
-4
lines changed

src/rt/lifetime.d

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,22 @@ size_t __arrayPad(size_t size, const TypeInfo tinext) nothrow pure @trusted
406406
return size > MAXMEDSIZE ? LARGEPAD : ((size > MAXSMALLSIZE ? MEDPAD : SMALLPAD) + structTypeInfoSize(tinext));
407407
}
408408

409+
/**
410+
clear padding that might not be zeroed by the GC (it assumes it is within the
411+
requested size from the start, but it is actually at the end of the allocated block)
412+
*/
413+
private void __arrayClearPad(ref BlkInfo info, size_t arrsize, size_t padsize) nothrow pure
414+
{
415+
import core.stdc.string;
416+
if (padsize > MEDPAD && !(info.attr & BlkAttr.NO_SCAN) && info.base)
417+
{
418+
if (info.size < PAGESIZE)
419+
memset(info.base + arrsize, 0, padsize);
420+
else
421+
memset(info.base, 0, LARGEPREFIX);
422+
}
423+
}
424+
409425
/**
410426
allocate an array memory block by applying the proper padding and
411427
assigning block attributes if not inherited from the existing block
@@ -426,7 +442,10 @@ BlkInfo __arrayAlloc(size_t arrsize, const TypeInfo ti, const TypeInfo tinext) n
426442
uint attr = (!(tinext.flags & 1) ? BlkAttr.NO_SCAN : 0) | BlkAttr.APPENDABLE;
427443
if (typeInfoSize)
428444
attr |= BlkAttr.STRUCTFINAL | BlkAttr.FINALIZE;
429-
return GC.qalloc(padded_size, attr, tinext);
445+
446+
auto bi = GC.qalloc(padded_size, attr, tinext);
447+
__arrayClearPad(bi, arrsize, padsize);
448+
return bi;
430449
}
431450

432451
BlkInfo __arrayAlloc(size_t arrsize, ref BlkInfo info, const TypeInfo ti, const TypeInfo tinext)
@@ -436,14 +455,17 @@ BlkInfo __arrayAlloc(size_t arrsize, ref BlkInfo info, const TypeInfo ti, const
436455
if (!info.base)
437456
return __arrayAlloc(arrsize, ti, tinext);
438457

458+
immutable padsize = __arrayPad(arrsize, tinext);
439459
bool overflow;
440-
auto padded_size = addu(arrsize, __arrayPad(arrsize, tinext), overflow);
460+
auto padded_size = addu(arrsize, padsize, overflow);
441461
if (overflow)
442462
{
443463
return BlkInfo();
444464
}
445465

446-
return GC.qalloc(padded_size, info.attr, tinext);
466+
auto bi = GC.qalloc(padded_size, info.attr, tinext);
467+
__arrayClearPad(bi, arrsize, padsize);
468+
return bi;
447469
}
448470

449471
/**
@@ -1096,15 +1118,19 @@ extern (C) void* _d_newitemU(in TypeInfo _ti)
10961118
auto ti = unqualify(_ti);
10971119
auto flags = !(ti.flags & 1) ? BlkAttr.NO_SCAN : 0;
10981120
immutable tiSize = structTypeInfoSize(ti);
1099-
immutable size = ti.tsize + tiSize;
1121+
immutable itemSize = ti.tsize;
1122+
immutable size = itemSize + tiSize;
11001123
if (tiSize)
11011124
flags |= BlkAttr.STRUCTFINAL | BlkAttr.FINALIZE;
11021125

11031126
auto blkInf = GC.qalloc(size, flags, ti);
11041127
auto p = blkInf.base;
11051128

11061129
if (tiSize)
1130+
{
1131+
*cast(TypeInfo*)(p + itemSize) = null; // the GC might not have cleared this area
11071132
*cast(TypeInfo*)(p + blkInf.size - tiSize) = cast() ti;
1133+
}
11081134

11091135
return p;
11101136
}
@@ -2678,6 +2704,80 @@ deprecated unittest
26782704
}
26792705
}
26802706

2707+
// test struct dtor handling not causing false pointers
2708+
unittest
2709+
{
2710+
if (!callStructDtorsDuringGC)
2711+
return;
2712+
2713+
// for 64-bit, allocate a struct of size 40
2714+
static struct S
2715+
{
2716+
size_t[4] data;
2717+
S* ptr4;
2718+
}
2719+
auto p1 = new S;
2720+
auto p2 = new S;
2721+
p2.ptr4 = p1;
2722+
2723+
// a struct with a dtor with size 32, but the dtor will cause
2724+
// allocation to be larger by a pointer
2725+
static struct A
2726+
{
2727+
size_t[3] data;
2728+
S* ptr3;
2729+
2730+
~this() {}
2731+
}
2732+
2733+
GC.free(p2);
2734+
auto a = new A; // reuse same memory
2735+
if (cast(void*)a is cast(void*)p2) // reusage not guaranteed
2736+
{
2737+
auto ptr = cast(S**)(a + 1);
2738+
assert(*ptr != p1); // still same data as p2.ptr4?
2739+
}
2740+
2741+
// small array
2742+
static struct SArr
2743+
{
2744+
void*[10] data;
2745+
}
2746+
auto arr1 = new SArr;
2747+
arr1.data[] = p1;
2748+
GC.free(arr1);
2749+
2750+
// allocates 2*A.sizeof + (void*).sizeof (TypeInfo) + 1 (array length)
2751+
auto arr2 = new A[2];
2752+
if (cast(void*)arr1 is cast(void*)arr2.ptr) // reusage not guaranteed
2753+
{
2754+
auto ptr = cast(S**)(arr2.ptr + 2);
2755+
assert(*ptr != p1); // still same data as p2.ptr4?
2756+
}
2757+
2758+
// large array
2759+
static struct LArr
2760+
{
2761+
void*[1023] data;
2762+
}
2763+
auto larr1 = new LArr;
2764+
larr1.data[] = p1;
2765+
GC.free(larr1);
2766+
2767+
auto larr2 = new S[255];
2768+
if (cast(void*)larr1 is cast(void*)larr2.ptr - LARGEPREFIX) // reusage not guaranteed
2769+
{
2770+
auto ptr = cast(S**)larr1;
2771+
assert(ptr[0] != p1); // 16 bytes array header
2772+
assert(ptr[1] != p1);
2773+
version (D_LP64) {} else
2774+
{
2775+
assert(ptr[2] != p1);
2776+
assert(ptr[3] != p1);
2777+
}
2778+
}
2779+
}
2780+
26812781
// test class finalizers exception handling
26822782
unittest
26832783
{

0 commit comments

Comments
 (0)