diff --git a/druntime/src/core/internal/gc/impl/conservative/gc.d b/druntime/src/core/internal/gc/impl/conservative/gc.d index 64b5bed43b18..5e38348bfb02 100644 --- a/druntime/src/core/internal/gc/impl/conservative/gc.d +++ b/druntime/src/core/internal/gc/impl/conservative/gc.d @@ -5419,7 +5419,7 @@ void undefinedWrite(T)(ref T var, T value) nothrow private void adjustAttrs(ref uint attrs, const TypeInfo ti) nothrow { bool hasContext = ti !is null; - if((attrs & BlkAttr.FINALIZE) && hasContext && typeid(ti) is typeid(TypeInfo_Struct)) + if((attrs & BlkAttr.FINALIZE) && hasContext && (typeid(ti) !is typeid(TypeInfo_Class))) attrs |= BlkAttr.STRUCTFINAL; else // STRUCTFINAL now just means "has a context pointer added to the block" diff --git a/druntime/src/core/internal/newaa.d b/druntime/src/core/internal/newaa.d index 7c858f33522c..31fe0736792e 100644 --- a/druntime/src/core/internal/newaa.d +++ b/druntime/src/core/internal/newaa.d @@ -44,7 +44,7 @@ struct Impl Bucket[] buckets; uint used; uint deleted; - TypeInfo_Struct entryTI; + const TypeInfo entryTI; uint firstUsed; immutable uint keysz; immutable uint valsz; @@ -57,6 +57,7 @@ struct Impl none = 0x0, keyHasPostblit = 0x1, hasPointers = 0x2, + hasDtors = 0x04, } } @@ -147,6 +148,8 @@ AAShell makeAA(K, V)(V[K] src) @trusted flags |= flags.keyHasPostblit; static if (hasIndirections!E) flags |= flags.hasPointers; + static if (hasElaborateDestructor!E) + flags |= flags.hasDtors; return flags; } (); // return the new implementation diff --git a/druntime/src/rt/aaA.d b/druntime/src/rt/aaA.d index 2bddeaf41d7c..e042e9f1322d 100644 --- a/druntime/src/rt/aaA.d +++ b/druntime/src/rt/aaA.d @@ -65,14 +65,18 @@ private: flags |= Flags.keyHasPostblit; if ((ti.key.flags | ti.value.flags) & 1) flags |= Flags.hasPointers; - - entryTI = fakeEntryTI(this, ti.key, ti.value); + if (hasDtor(unqualify(ti.key)) || hasDtor(unqualify(ti.value))) + { + // need the type info for the destructors. + entryTI = ti; + flags |= Flags.hasDtors; + } } Bucket[] buckets; uint used; uint deleted; - TypeInfo_Struct entryTI; + const TypeInfo entryTI; uint firstUsed; immutable uint keysz; immutable uint valsz; @@ -88,6 +92,7 @@ private: none = 0x0, keyHasPostblit = 0x1, hasPointers = 0x2, + hasDtors = 0x4, } @property size_t length() const pure nothrow @nogc @safe @@ -210,32 +215,35 @@ Bucket[] allocBuckets(size_t dim) @trusted pure nothrow private void* allocEntry(scope const Impl* aa, scope const void* pkey) { - import rt.lifetime : _d_newitemU; import core.stdc.string : memcpy, memset; - immutable akeysz = aa.valoff; - void* res = void; - if (aa.entryTI) - res = _d_newitemU(aa.entryTI); - else + immutable akeysz = aa.keysz; + immutable allocsz = aa.valoff + aa.valsz; + auto flags = (aa.flags & Impl.Flags.hasPointers) ? 0 : GC.BlkAttr.NO_SCAN; + if (aa.flags & Impl.Flags.hasDtors) { - auto flags = (aa.flags & Impl.Flags.hasPointers) ? 0 : GC.BlkAttr.NO_SCAN; - res = GC.malloc(akeysz + aa.valsz, flags); + assert(aa.entryTI !is null); + flags |= GC.BlkAttr.FINALIZE; } + auto res = GC.malloc(allocsz, flags, aa.entryTI); - memcpy(res, pkey, aa.keysz); // copy key - memset(res + akeysz, 0, aa.valsz); // zero value + memcpy(res, pkey, akeysz); // copy key + memset(res + akeysz, 0, allocsz - akeysz); // zero everything after return res; } -package void entryDtor(void* p, const TypeInfo_Struct sti) +package void finalizeEntry(void* p, TypeInfo_AssociativeArray ti) { - // key and value type info stored after the TypeInfo_Struct by tiEntry() - auto sizeti = __traits(classInstanceSize, TypeInfo_Struct); - auto extra = cast(const(TypeInfo)*)(cast(void*) sti + sizeti); - extra[0].destroy(p); - extra[1].destroy(p + talign(extra[0].tsize, extra[1].talign)); + import rt.lifetime : unqualify; + + ti.key.destroy(p); + if (hasDtor(unqualify(ti.value))) + { + // unfortunately, need to recalculate this every time. + auto valoff = talign(ti.key.tsize, ti.value.talign); + ti.value.destroy(p + valoff); + } } private bool hasDtor(const TypeInfo ti) pure nothrow @@ -243,149 +251,22 @@ private bool hasDtor(const TypeInfo ti) pure nothrow import rt.lifetime : unqualify; if (typeid(ti) is typeid(TypeInfo_Struct)) - if ((cast(TypeInfo_Struct) cast(void*) ti).xdtor) - return true; + return (cast(TypeInfo_Struct) cast(void*) ti).xdtor !is null; if (typeid(ti) is typeid(TypeInfo_StaticArray)) return hasDtor(unqualify(ti.next)); return false; } -private immutable(void)* getRTInfo(const TypeInfo ti) pure nothrow -{ - // classes are references - const isNoClass = ti && typeid(ti) !is typeid(TypeInfo_Class); - return isNoClass ? ti.rtInfo() : rtinfoHasPointers; -} - -// build type info for Entry with additional key and value fields -TypeInfo_Struct fakeEntryTI(ref Impl aa, const TypeInfo keyti, const TypeInfo valti) nothrow +unittest { - import rt.lifetime : unqualify; - - auto kti = unqualify(keyti); - auto vti = unqualify(valti); - - // figure out whether RTInfo has to be generated (indicated by rtisize > 0) - enum pointersPerWord = 8 * (void*).sizeof * (void*).sizeof; - auto rtinfo = rtinfoNoPointers; - size_t rtisize = 0; - immutable(size_t)* keyinfo = void; - immutable(size_t)* valinfo = void; - if (aa.flags & Impl.Flags.hasPointers) + static immutable(void)* getRTInfo(const TypeInfo ti) pure nothrow { // classes are references - keyinfo = cast(immutable(size_t)*) getRTInfo(keyti); - valinfo = cast(immutable(size_t)*) getRTInfo(valti); - - if (keyinfo is rtinfoHasPointers && valinfo is rtinfoHasPointers) - rtinfo = rtinfoHasPointers; - else - rtisize = 1 + (aa.valoff + aa.valsz + pointersPerWord - 1) / pointersPerWord; - } - bool entryHasDtor = hasDtor(kti) || hasDtor(vti); - if (rtisize == 0 && !entryHasDtor) - return null; - - // save kti and vti after type info for struct - enum sizeti = __traits(classInstanceSize, TypeInfo_Struct); - void* p = GC.malloc(sizeti + (2 + rtisize) * (void*).sizeof); - import core.stdc.string : memcpy; - - memcpy(p, __traits(initSymbol, TypeInfo_Struct).ptr, sizeti); - - auto ti = cast(TypeInfo_Struct) p; - auto extra = cast(TypeInfo*)(p + sizeti); - extra[0] = cast() kti; - extra[1] = cast() vti; - - static immutable tiMangledName = "S2rt3aaA__T5EntryZ"; - ti.mangledName = tiMangledName; - - ti.m_RTInfo = rtisize > 0 ? rtinfoEntry(aa, keyinfo, valinfo, cast(size_t*)(extra + 2), rtisize) : rtinfo; - ti.m_flags = ti.m_RTInfo is rtinfoNoPointers ? cast(TypeInfo_Struct.StructFlags)0 : TypeInfo_Struct.StructFlags.hasPointers; - - // we don't expect the Entry objects to be used outside of this module, so we have control - // over the non-usage of the callback methods and other entries and can keep these null - // xtoHash, xopEquals, xopCmp, xtoString and xpostblit - immutable entrySize = aa.valoff + aa.valsz; - ti.m_init = (cast(ubyte*) null)[0 .. entrySize]; // init length, but not ptr - - if (entryHasDtor) - { - // xdtor needs to be built from the dtors of key and value for the GC - ti.xdtorti = &entryDtor; - ti.m_flags |= TypeInfo_Struct.StructFlags.isDynamicType; - } - - ti.m_align = cast(uint) max(kti.talign, vti.talign); - - return ti; -} - -// build appropriate RTInfo at runtime -immutable(void)* rtinfoEntry(ref Impl aa, immutable(size_t)* keyinfo, - immutable(size_t)* valinfo, size_t* rtinfoData, size_t rtinfoSize) pure nothrow -{ - enum bitsPerWord = 8 * size_t.sizeof; - - rtinfoData[0] = aa.valoff + aa.valsz; - rtinfoData[1..rtinfoSize] = 0; - - void copyKeyInfo(string src)() - { - size_t pos = 1; - size_t keybits = aa.keysz / (void*).sizeof; - while (keybits >= bitsPerWord) - { - rtinfoData[pos] = mixin(src); - keybits -= bitsPerWord; - pos++; - } - if (keybits > 0) - rtinfoData[pos] = mixin(src) & ((size_t(1) << keybits) - 1); + const isNoClass = ti && typeid(ti) !is typeid(TypeInfo_Class); + return isNoClass ? ti.rtInfo() : rtinfoHasPointers; } - if (keyinfo is rtinfoHasPointers) - copyKeyInfo!"~size_t(0)"(); - else if (keyinfo !is rtinfoNoPointers) - copyKeyInfo!"keyinfo[pos]"(); - - void copyValInfo(string src)() - { - size_t bitpos = aa.valoff / (void*).sizeof; - size_t pos = 1; - size_t dstpos = 1 + bitpos / bitsPerWord; - size_t begoff = bitpos % bitsPerWord; - size_t valbits = aa.valsz / (void*).sizeof; - size_t endoff = (bitpos + valbits) % bitsPerWord; - for (;;) - { - const bits = bitsPerWord - begoff; - size_t s = mixin(src); - rtinfoData[dstpos] |= s << begoff; - if (begoff > 0 && valbits > bits) - rtinfoData[dstpos+1] |= s >> bits; - if (valbits < bitsPerWord) - break; - valbits -= bitsPerWord; - dstpos++; - pos++; - } - if (endoff > 0) - rtinfoData[dstpos] &= (size_t(1) << endoff) - 1; - } - - if (valinfo is rtinfoHasPointers) - copyValInfo!"~size_t(0)"(); - else if (valinfo !is rtinfoNoPointers) - copyValInfo!"valinfo[pos]"(); - - return cast(immutable(void)*) rtinfoData; -} - -unittest -{ void test(K, V)() { static struct Entry @@ -404,21 +285,11 @@ unittest assert(!(impl.flags & Impl.Flags.hasPointers)); assert(impl.entryTI is null); } - else if (valrti is rtinfoHasPointers && keyrti is rtinfoHasPointers) + else { assert(impl.flags & Impl.Flags.hasPointers); assert(impl.entryTI is null); } - else - { - auto rtInfo = cast(size_t*) impl.entryTI.rtInfo(); - auto refInfo = cast(size_t*) typeid(Entry).rtInfo(); - assert(rtInfo[0] == refInfo[0]); // size - enum bytesPerWord = 8 * size_t.sizeof * (void*).sizeof; - size_t words = (rtInfo[0] + bytesPerWord - 1) / bytesPerWord; - foreach (i; 0 .. words) - assert(rtInfo[1 + i] == refInfo[i + 1]); - } } test!(long, int)(); test!(string, string); @@ -559,10 +430,10 @@ extern (C) void* _aaGetX(scope AA* paa, const TypeInfo_AssociativeArray ti, immutable hash = calcHash(pkey, aa); // found a value => return it - if (auto p = aa.findSlotLookup(hash, pkey, ti.key)) + if (auto q = aa.findSlotLookup(hash, pkey, ti.key)) { found = true; - return p.entry + aa.valoff; + return q.entry + aa.valoff; } auto p = aa.findSlotInsert(hash); @@ -795,7 +666,7 @@ extern (C) Impl* _d_assocarrayliteralTX(const TypeInfo_AssociativeArray ti, void aa.firstUsed = min(aa.firstUsed, cast(uint)(p - aa.buckets.ptr)); actualLength++; } - else if (aa.entryTI && hasDtor(ti.value)) + else if ((aa.flags & Impl.Flags.hasDtors) && hasDtor(ti.value)) { // destroy existing value before overwriting it ti.value.destroy(p.entry + off); @@ -977,8 +848,11 @@ unittest aa1 = null; aa2 = null; aa3 = null; - GC.runFinalizers((cast(char*)&entryDtor)[0 .. 1]); - assert(T.dtor == 6 && T.postblit == 2); + auto entryDtor = typeid(T).xdtor; + assert(entryDtor !is null); + GC.runFinalizers((cast(char*)entryDtor)[0 .. 1]); + assert(T.postblit == 2); + assert(T.dtor == 6); } // Ensure the newaa struct layout (used for static initialization) is in sync diff --git a/druntime/src/rt/lifetime.d b/druntime/src/rt/lifetime.d index b63a5bf5266e..84e4aaab06bf 100644 --- a/druntime/src/rt/lifetime.d +++ b/druntime/src/rt/lifetime.d @@ -611,12 +611,28 @@ extern (C) int rt_hasFinalizerInSegment(void* p, size_t size, TypeInfo typeInfo, if (!p) return false; - if (typeInfo !is null) + bool checkStructTI(TypeInfo ti) { - assert(typeid(typeInfo) is typeid(TypeInfo_Struct)); + if (typeid(ti) is typeid(TypeInfo_Struct)) + { + auto sti = cast(TypeInfo_Struct)cast(void*)ti; + return cast(size_t)(cast(void*)sti.xdtor - segment.ptr) < segment.length; + } + if (typeid(ti) is typeid(TypeInfo_StaticArray)) + return checkStructTI(ti.next); + return false; + } - auto ti = cast(TypeInfo_Struct)cast(void*)typeInfo; - return cast(size_t)(cast(void*)ti.xdtor - segment.ptr) < segment.length; + if (typeInfo !is null) + { + if (typeid(typeInfo) is typeid(TypeInfo_AssociativeArray)) + { + // this is an AA element. It's in here because we need to run a + // dtor on either the key or the value. + auto tiaa = cast(TypeInfo_AssociativeArray)cast(void*)typeInfo; + return checkStructTI(unqualify(tiaa.key)) || checkStructTI(unqualify(tiaa.value)); + } + return checkStructTI(typeInfo); } // otherwise class @@ -721,22 +737,30 @@ extern (C) void rt_finalizeFromGC(void* p, size_t size, uint attr, TypeInfo type return; } - assert(typeid(typeInfo) is typeid(TypeInfo_Struct)); - - auto si = cast(TypeInfo_Struct)cast(void*)typeInfo; - try { - if (attr & BlkAttr.APPENDABLE) + auto tti = typeid(typeInfo); + + if (tti is typeid(TypeInfo_AssociativeArray)) { - finalize_array(p, size, si); + import rt.aaA : finalizeEntry; + finalizeEntry(p, cast(TypeInfo_AssociativeArray)cast(void*)typeInfo); + } + else if (tti is typeid(TypeInfo_Struct)) + { + auto si = cast(TypeInfo_Struct)cast(void*)typeInfo; + + if (attr & BlkAttr.APPENDABLE) + { + finalize_array(p, size, si); + } + else + finalize_struct(p, si); // struct } - else - finalize_struct(p, si); // struct } catch (Exception e) { - onFinalizeError(si, e); + onFinalizeError(typeInfo, e); } } @@ -1587,16 +1611,13 @@ deprecated unittest GC.free(blkinf.base); // associative arrays - import rt.aaA : entryDtor; - // throw away all existing AA entries with dtor - GC.runFinalizers((cast(char*)&entryDtor)[0..1]); S1[int] aa1; aa1[0] = S1(0); aa1[1] = S1(1); dtorCount = 0; aa1 = null; - GC.runFinalizers((cast(char*)&entryDtor)[0..1]); + GC.runFinalizers((cast(char*)(typeid(S1).xdtor))[0..1]); assert(dtorCount == 2); int[S1] aa2; @@ -1605,7 +1626,7 @@ deprecated unittest aa2[S1(2)] = 2; dtorCount = 0; aa2 = null; - GC.runFinalizers((cast(char*)&entryDtor)[0..1]); + GC.runFinalizers((cast(char*)(typeid(S1).xdtor))[0..1]); assert(dtorCount == 3); S1[2][int] aa3; @@ -1613,7 +1634,7 @@ deprecated unittest aa3[1] = [S1(1),S1(3)]; dtorCount = 0; aa3 = null; - GC.runFinalizers((cast(char*)&entryDtor)[0..1]); + GC.runFinalizers((cast(char*)(typeid(S1).xdtor))[0..1]); assert(dtorCount == 4); } diff --git a/druntime/test/aa/src/test_aa.d b/druntime/test/aa/src/test_aa.d index 5ccb14be1710..f56f1a4b42ef 100644 --- a/druntime/test/aa/src/test_aa.d +++ b/druntime/test/aa/src/test_aa.d @@ -40,6 +40,7 @@ void main() testZeroSizedValue(); testTombstonePurging(); testClear(); + testFakeTypeInfoCollect(); } void testKeysValues1() @@ -905,3 +906,44 @@ void testClear() assert(aa.length == 1); assert(aa[5] == 6); } + +// https://github.com/dlang/dmd/issues/17503 +void testFakeTypeInfoCollect() +{ + import core.memory; + + static struct S + { + int x; + ~this() {} + } + + static struct AAHolder + { + S[int] aa; + } + + static S* getBadS() + { + auto aaholder = new AAHolder; + aaholder.aa[0] = S(); + auto s = 0 in aaholder.aa; // keep a pointer to the entry + GC.free(aaholder); // but not a pointer to the AA. + return s; + } + + static void stackStomp() + { + import core.stdc.string : memset; + ubyte[4 * 4096] x; + memset(x.ptr, 0, x.sizeof); + } + + auto s = getBadS(); + stackStomp(); // destroy any stale references to the AA or s except in the current frame; + GC.collect(); // BUG: this used to invalidate the fake type info, should no longer do this. + foreach(i; 0 .. 1000) // try to reallocate the freed type info + auto p = new void*[1]; + s = null; // clear any reference to the entry + GC.collect(); // used to segfault. +} diff --git a/druntime/test/profile/myprofilegc.log.freebsd.32.exp b/druntime/test/profile/myprofilegc.log.freebsd.32.exp index 8a04f4737acf..712b3a0229ee 100644 --- a/druntime/test/profile/myprofilegc.log.freebsd.32.exp +++ b/druntime/test/profile/myprofilegc.log.freebsd.32.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 256 1 immutable(char)[][int] D main src/profilegc.d:23 + 160 1 immutable(char)[][int] D main src/profilegc.d:23 128 1 float D main src/profilegc.d:18 128 1 int D main src/profilegc.d:15 64 1 double[] profilegc.main src/profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.freebsd.64.exp b/druntime/test/profile/myprofilegc.log.freebsd.64.exp index e3e2476a5a7a..34c223638480 100644 --- a/druntime/test/profile/myprofilegc.log.freebsd.64.exp +++ b/druntime/test/profile/myprofilegc.log.freebsd.64.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 496 1 immutable(char)[][int] D main src/profilegc.d:23 + 320 1 immutable(char)[][int] D main src/profilegc.d:23 160 1 float D main src/profilegc.d:18 160 1 int D main src/profilegc.d:15 64 1 double[] profilegc.main src/profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.linux.32.exp b/druntime/test/profile/myprofilegc.log.linux.32.exp index 9d929c3cca41..8ffc0237a4d0 100644 --- a/druntime/test/profile/myprofilegc.log.linux.32.exp +++ b/druntime/test/profile/myprofilegc.log.linux.32.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 256 1 immutable(char)[][int] D main src/profilegc.d:23 + 160 1 immutable(char)[][int] D main src/profilegc.d:23 128 1 float D main src/profilegc.d:18 128 1 int D main src/profilegc.d:15 64 1 double[] profilegc.main src/profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.linux.64.exp b/druntime/test/profile/myprofilegc.log.linux.64.exp index e3e2476a5a7a..34c223638480 100644 --- a/druntime/test/profile/myprofilegc.log.linux.64.exp +++ b/druntime/test/profile/myprofilegc.log.linux.64.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 496 1 immutable(char)[][int] D main src/profilegc.d:23 + 320 1 immutable(char)[][int] D main src/profilegc.d:23 160 1 float D main src/profilegc.d:18 160 1 int D main src/profilegc.d:15 64 1 double[] profilegc.main src/profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.osx.64.exp b/druntime/test/profile/myprofilegc.log.osx.64.exp index e3e2476a5a7a..34c223638480 100644 --- a/druntime/test/profile/myprofilegc.log.osx.64.exp +++ b/druntime/test/profile/myprofilegc.log.osx.64.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 496 1 immutable(char)[][int] D main src/profilegc.d:23 + 320 1 immutable(char)[][int] D main src/profilegc.d:23 160 1 float D main src/profilegc.d:18 160 1 int D main src/profilegc.d:15 64 1 double[] profilegc.main src/profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.windows.32.exp b/druntime/test/profile/myprofilegc.log.windows.32.exp index 8b4cdf6e7513..8616ec5c64c5 100644 --- a/druntime/test/profile/myprofilegc.log.windows.32.exp +++ b/druntime/test/profile/myprofilegc.log.windows.32.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 256 1 immutable(char)[][int] D main src\profilegc.d:23 + 160 1 immutable(char)[][int] D main src\profilegc.d:23 128 1 float D main src\profilegc.d:18 128 1 int D main src\profilegc.d:15 64 1 double[] profilegc.main src\profilegc.d:56 diff --git a/druntime/test/profile/myprofilegc.log.windows.64.exp b/druntime/test/profile/myprofilegc.log.windows.64.exp index 05408557a848..cc528950c115 100644 --- a/druntime/test/profile/myprofilegc.log.windows.64.exp +++ b/druntime/test/profile/myprofilegc.log.windows.64.exp @@ -1,5 +1,5 @@ bytes allocated, allocations, type, function, file:line - 496 1 immutable(char)[][int] D main src\profilegc.d:23 + 320 1 immutable(char)[][int] D main src\profilegc.d:23 160 1 float D main src\profilegc.d:18 160 1 int D main src\profilegc.d:15 64 1 double[] profilegc.main src\profilegc.d:56