Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #17503 - Associative Arrays improperly register a GC-allocated TypeInfo for element cleanup #20863

Merged
merged 7 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/src/dmd/dcast.d
Original file line number Diff line number Diff line change
Expand Up @@ -2832,6 +2832,7 @@ Expression castTo(Expression e, Scope* sc, Type t, Type att = null)
(*ae.keys)[i] = ex;
}
ae.type = t;
semanticTypeInfo(sc, ae.type);
return ae;
}
return visit(e);
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/declaration.d
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,8 @@ extern (C++) final class TypeInfoStaticArrayDeclaration : TypeInfoDeclaration
*/
extern (C++) final class TypeInfoAssociativeArrayDeclaration : TypeInfoDeclaration
{
Type entry; // type of TypeInfo_AssociativeArray.Entry!(t.index, t.next)

extern (D) this(Type tinfo)
{
super(tinfo);
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/declaration.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ class TypeInfoStaticArrayDeclaration final : public TypeInfoDeclaration
class TypeInfoAssociativeArrayDeclaration final : public TypeInfoDeclaration
{
public:
Type* entry;

static TypeInfoAssociativeArrayDeclaration *create(Type *tinfo);

void accept(Visitor *v) override { v->visit(this); }
Expand Down
16 changes: 14 additions & 2 deletions compiler/src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -11168,6 +11168,11 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
exp.e2 = e2x;
t1 = e1x.type.toBasetype();
}
else if (t1.ty == Taarray)
{
// when assigning a constant, the need for TypeInfo might change
semanticTypeInfo(sc, t1);
}
/* Check the mutability of e1.
*/
if (auto ale = exp.e1.isArrayLengthExp())
Expand Down Expand Up @@ -13070,7 +13075,10 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
exp.e1 = exp.e1.implicitCastTo(sc, ta.index);
}

semanticTypeInfo(sc, ta.index);
// even though the glue layer only needs the type info of the index,
// this might be the first time an AA literal is accessed, so check
// the full type info
semanticTypeInfo(sc, ta);

// Return type is pointer to value
exp.type = ta.nextOf().pointerTo();
Expand Down Expand Up @@ -13307,8 +13315,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
}

if (exp.e1.type.toBasetype().ty == Taarray)
{
semanticTypeInfo(sc, exp.e1.type.toBasetype());

}

if (!target.isVectorOpSupported(t1, exp.op, t2))
{
Expand Down Expand Up @@ -16849,6 +16858,9 @@ void semanticTypeInfo(Scope* sc, Type t)
{
semanticTypeInfo(sc, t.index);
semanticTypeInfo(sc, t.next);

if (global.params.useTypeInfo)
getTypeInfoType(t.loc, t, sc);
}

void visitStruct(TypeStruct t)
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -6912,6 +6912,7 @@ class TypeInfoStaticArrayDeclaration final : public TypeInfoDeclaration
class TypeInfoAssociativeArrayDeclaration final : public TypeInfoDeclaration
{
public:
Type* entry;
static TypeInfoAssociativeArrayDeclaration* create(Type* tinfo);
void accept(Visitor* v) override;
};
Expand Down Expand Up @@ -8791,6 +8792,7 @@ struct Id final
static Identifier* xopEquals;
static Identifier* xopCmp;
static Identifier* xtoHash;
static Identifier* Entry;
static Identifier* LINE;
static Identifier* FILE;
static Identifier* MODULE;
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/id.d
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ immutable Msgtable[] msgtable =
{ "xopCmp", "__xopCmp" },
{ "xtoHash", "__xtoHash" },
{ "__tmpfordtor" },
{ "Entry" },

{ "LINE", "__LINE__" },
{ "FILE", "__FILE__" },
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/semantic2.d
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,8 @@ private extern(C++) final class StaticAAVisitor : SemanticTimeTransitiveVisitor
loweredExp = loweredExp.ctfeInterpret();

aaExp.lowering = loweredExp;

semanticTypeInfo(sc, loweredExp.type);
}

// https://issues.dlang.org/show_bug.cgi?id=24602
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dmd/todt.d
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ private extern (C++) class TypeInfoDtVisitor : Visitor
override void visit(TypeInfoAssociativeArrayDeclaration d)
{
//printf("TypeInfoAssociativeArrayDeclaration.toDt()\n");
verifyStructSize(Type.typeinfoassociativearray, 4 * target.ptrsize);
verifyStructSize(Type.typeinfoassociativearray, 5 * target.ptrsize);

dtb.xoff(toVtblSymbol(Type.typeinfoassociativearray), 0); // vtbl for TypeInfo_AssociativeArray
if (Type.typeinfoassociativearray.hasMonitor())
Expand All @@ -1361,6 +1361,9 @@ private extern (C++) class TypeInfoDtVisitor : Visitor

TypeInfo_toObjFile(null, d.loc, tc.index);
dtb.xoff(toSymbol(tc.index.vtinfo), 0); // TypeInfo for array of type

TypeInfo_toObjFile(null, d.loc, d.entry);
dtb.xoff(toSymbol(d.entry.vtinfo), 0); // TypeInfo for key,value-pair
}

override void visit(TypeInfoFunctionDeclaration d)
Expand Down
33 changes: 33 additions & 0 deletions compiler/src/dmd/typesem.d
Original file line number Diff line number Diff line change
Expand Up @@ -7023,6 +7023,39 @@
return t;
}

Type nakedOf(Type type)
{
//printf("Type::nakedOf() %p, %s\n", type, type.toChars());
if (type.mod == 0)
return type;
if (type.mcache) with(type.mcache)
{
// the cache has the naked type at the "identity" position, try to find it
if (cto && cto.mod == 0)
return cto;
if (ito && ito.mod == 0)
return ito;

Check warning on line 7037 in compiler/src/dmd/typesem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/typesem.d#L7037

Added line #L7037 was not covered by tests
if (sto && sto.mod == 0)
return sto;

Check warning on line 7039 in compiler/src/dmd/typesem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/typesem.d#L7039

Added line #L7039 was not covered by tests
if (scto && scto.mod == 0)
return scto;
if (wto && wto.mod == 0)
return wto;
if (wcto && wcto.mod == 0)
return wcto;

Check warning on line 7045 in compiler/src/dmd/typesem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/typesem.d#L7045

Added line #L7045 was not covered by tests
if (swto && swto.mod == 0)
return swto;

Check warning on line 7047 in compiler/src/dmd/typesem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/typesem.d#L7047

Added line #L7047 was not covered by tests
if (swcto && swcto.mod == 0)
return swcto;

Check warning on line 7049 in compiler/src/dmd/typesem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/typesem.d#L7049

Added line #L7049 was not covered by tests
}
Type t = type.nullAttributes();
t.mod = 0;
t = t.merge();
t.fixTo(type);
//printf("\t%p %s\n", t, t.toChars());
return t;
}

Type unqualify(Type type, uint m)
{
Type t = type.mutableOf().unSharedOf();
Expand Down
74 changes: 71 additions & 3 deletions compiler/src/dmd/typinf.d
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import dmd.expression;
import dmd.globals;
import dmd.location;
import dmd.mtype;
import dmd.typesem;
import core.stdc.stdio;

/****************************************************
Expand Down Expand Up @@ -67,6 +68,9 @@ bool genTypeInfo(Expression e, Loc loc, Type torig, Scope* sc)

import dmd.typesem : merge2;
Type t = torig.merge2(); // do this since not all Type's are merge'd
if (t.ty == Taarray)
t = makeNakedAssociativeArray(cast(TypeAArray)t);

bool needsCodegen = false;
if (!t.vtinfo)
{
Expand All @@ -79,7 +83,7 @@ bool genTypeInfo(Expression e, Loc loc, Type torig, Scope* sc)
else if (t.isWild())
t.vtinfo = TypeInfoWildDeclaration.create(t);
else
t.vtinfo = getTypeInfoDeclaration(t);
t.vtinfo = getTypeInfoDeclaration(t, sc);
assert(t.vtinfo);

// ClassInfos are generated as part of ClassDeclaration codegen
Expand Down Expand Up @@ -115,7 +119,7 @@ extern (C++) Type getTypeInfoType(Loc loc, Type t, Scope* sc)
return t.vtinfo.type;
}

private TypeInfoDeclaration getTypeInfoDeclaration(Type t)
private TypeInfoDeclaration getTypeInfoDeclaration(Type t, Scope* sc)
{
//printf("Type::getTypeInfoDeclaration() %s\n", t.toChars());
switch (t.ty)
Expand All @@ -127,7 +131,7 @@ private TypeInfoDeclaration getTypeInfoDeclaration(Type t)
case Tsarray:
return TypeInfoStaticArrayDeclaration.create(t);
case Taarray:
return TypeInfoAssociativeArrayDeclaration.create(t);
return getTypeInfoAssocArrayDeclaration(cast(TypeAArray)t, sc);
case Tstruct:
return TypeInfoStructDeclaration.create(t);
case Tvector:
Expand All @@ -151,6 +155,70 @@ private TypeInfoDeclaration getTypeInfoDeclaration(Type t)
}
}

/******************************************
* Instantiate TypeInfoAssociativeArrayDeclaration and fill
* the entry with TypeInfo_AssociativeArray.Entry!(t.index, t.next)
*
* Params:
* t = TypeAArray to generate TypeInfo_AssociativeArray for
* sc = context
* Returns:
* a TypeInfoAssociativeArrayDeclaration with field entry initialized
*/
TypeInfoDeclaration getTypeInfoAssocArrayDeclaration(TypeAArray t, Scope* sc)
{
import dmd.arraytypes;
import dmd.expressionsem;
import dmd.id;

assert(sc); // must not be called in the code generation phase

auto ti = TypeInfoAssociativeArrayDeclaration.create(t);
t.vtinfo = ti; // assign it early to avoid recursion in expressionSemantic
Loc loc = t.loc;
auto tiargs = new Objects();
tiargs.push(t.index); // always called with naked types
tiargs.push(t.next);

Expression id = new IdentifierExp(loc, Id.empty);
id = new DotIdExp(loc, id, Id.object);
id = new DotIdExp(loc, id, Id.TypeInfo_AssociativeArray);
auto tempinst = new DotTemplateInstanceExp(loc, id, Id.Entry, tiargs);
auto e = expressionSemantic(tempinst, sc);
assert(e.type);
ti.entry = e.type;
if (auto ts = ti.entry.isTypeStruct())
{
ts.sym.requestTypeInfo = true;
if (auto tmpl = ts.sym.isInstantiated())
tmpl.minst = sc._module.importedFrom; // ensure it get's emitted
}
getTypeInfoType(loc, ti.entry, sc);
assert(ti.entry.vtinfo);

return ti;
}

/******************************************
* Find or create a TypeAArray with index and next without
* any head modifiers, tail `inout` is replaced with `const`
*
* Params:
* t = TypeAArray to convert
* Returns:
* t = found type
*/
Type makeNakedAssociativeArray(TypeAArray t)
{
Type tindex = t.index.toBasetype().nakedOf().substWildTo(MODFlags.const_);
Type tnext = t.next.toBasetype().nakedOf().substWildTo(MODFlags.const_);
if (tindex == t.index && tnext == t.next)
return t;

t = new TypeAArray(tnext, tindex);
return t.merge();
}

/**************************************************
* Returns:
* true if any part of type t is speculative.
Expand Down
23 changes: 23 additions & 0 deletions compiler/test/runnable/testaa2.d
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,27 @@ struct PropertyTable10106
CodepointSet10106[string] table;
}

/************************************************/
// strip inout in key and value types
void testinout()
{
inout(long) func1(inout(long[][int]) aa)
{
return aa[0][0];
}
long[][int] a = [ 0 : [42] ];
long b = func1(a);
assert(b == 42);
}

/************************************************/
// type info generated for enum creation in InExp?
void testinenum()
{
enum string[string] aa = [ "one" : "un", "two" : "deux" ];
assert("one" in aa);
}

/************************************************/

int main()
Expand All @@ -295,6 +316,8 @@ int main()
test4523();
test3825();
test3825x();
testinout();
testinenum();

printf("Success\n");
return 0;
Expand Down
2 changes: 1 addition & 1 deletion druntime/src/core/internal/newaa.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions druntime/src/object.d
Original file line number Diff line number Diff line change
Expand Up @@ -1324,8 +1324,16 @@ class TypeInfo_AssociativeArray : TypeInfo
override @property inout(TypeInfo) next() nothrow pure inout { return value; }
override @property uint flags() nothrow pure const { return 1; }

// TypeInfo entry is generated from the type of this template to help rt/aaA.d
static struct Entry(K, V)
{
K key;
V value;
}

TypeInfo value;
TypeInfo key;
TypeInfo entry;

override @property size_t talign() nothrow pure const
{
Expand Down
Loading
Loading