Skip to content

Commit

Permalink
Merge pull request #5050 from 9rnsr/fix14140
Browse files Browse the repository at this point in the history 
Issue 14140 - Bad codegen for CTFE union initialisers for immutable structs
  • Loading branch information
@WalterBright
WalterBright committed Sep 13, 2015
2 parents ed85fed + c8c5603 commit 16969dc
Show file tree
Hide file tree
Showing 8 changed files with 548 additions and 455 deletions.
221 changes: 221 additions & 0 deletions src/aggregate.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

module ddmd.aggregate;

import core.stdc.stdio;
import ddmd.access;
import ddmd.arraytypes;
import ddmd.backend;
Expand All @@ -19,6 +20,7 @@ import ddmd.dscope;
import ddmd.dstruct;
import ddmd.dsymbol;
import ddmd.dtemplate;
import ddmd.errors;
import ddmd.expression;
import ddmd.func;
import ddmd.globals;
Expand Down Expand Up @@ -280,6 +282,225 @@ public:

abstract void finalizeSize(Scope* sc);

/***************************************
* Calculate field[i].overlapped, and check that all of explicit
* field initializers have unique memory space on instance.
* Returns:
* true if any errors happen.
*/
final bool checkOverlappedFields()
{
//printf("AggregateDeclaration::checkOverlappedFields() %s\n", toChars());
assert(sizeok == SIZEOKdone);
size_t nfields = fields.dim;
if (isNested())
{
auto cd = isClassDeclaration();
if (!cd || !cd.baseClass || !cd.baseClass.isNested())
nfields--;
}
bool errors = false;

// Fill in missing any elements with default initializers
foreach (i; 0 .. nfields)
{
auto vd = fields[i];
auto vx = vd;
if (vd._init && vd._init.isVoidInitializer())
vx = null;

// Find overlapped fields with the hole [vd->offset .. vd->offset->size()].
foreach (j; 0 .. nfields)
{
if (i == j)
continue;
auto v2 = fields[j];
if (!vd.isOverlappedWith(v2))
continue;

// vd and v2 are overlapping. If either has destructors, postblits, etc., then error
//printf("overlapping fields %s and %s\n", vd->toChars(), v2->toChars());
foreach (k; 0.. 2)
{
auto v = k == 0 ? vd : v2;
Type tv = v.type.baseElemOf();
Dsymbol sv = tv.toDsymbol(null);
if (!sv || errors)
continue;
StructDeclaration sd = sv.isStructDeclaration();
if (sd && (sd.dtor || sd.inv || sd.postblit))
{
error("destructors, postblits and invariants are not allowed in overlapping fields %s and %s",
vd.toChars(), v2.toChars());
errors = true;
break;
}
}
vd.overlapped = true;

if (!vx)
continue;
if (v2._init && v2._init.isVoidInitializer())
continue;

if (vx._init && v2._init)
{
.error(loc, "overlapping default initialization for field %s and %s", v2.toChars(), vd.toChars());
errors = true;
}
}
}
return errors;
}

/***************************************
* Fill out remainder of elements[] with default initializers for fields[].
* Params:
* loc = location
* elements = explicit arguments which given to construct object.
* ctorinit = true if the elements will be used for default initialization.
* Returns:
* false if any errors occur.
* Otherwise, returns true and the missing arguments will be pushed in elements[].
*/
final bool fill(Loc loc, Expressions* elements, bool ctorinit)
{
//printf("AggregateDeclaration::fill() %s\n", toChars());
assert(sizeok == SIZEOKdone);
assert(elements);
size_t nfields = fields.dim - isNested();
bool errors = false;

size_t dim = elements.dim;
elements.setDim(nfields);
foreach (size_t i; dim .. nfields)
(*elements)[i] = null;

// Fill in missing any elements with default initializers
foreach (i; 0 .. nfields)
{
if ((*elements)[i])
continue;

auto vd = fields[i];
auto vx = vd;
if (vd._init && vd._init.isVoidInitializer())
vx = null;

// Find overlapped fields with the hole [vd->offset .. vd->offset->size()].
size_t fieldi = i;
foreach (j; 0 .. nfields)
{
if (i == j)
continue;
auto v2 = fields[j];
if (!vd.isOverlappedWith(v2))
continue;

if ((*elements)[j])
{
vx = null;
break;
}
if (v2._init && v2._init.isVoidInitializer())
continue;

version (all)
{
/* Prefer first found non-void-initialized field
* union U { int a; int b = 2; }
* U u; // Error: overlapping initialization for field a and b
*/
if (!vx)
{
vx = v2;
fieldi = j;
}
else if (v2._init)
{
.error(loc, "overlapping initialization for field %s and %s", v2.toChars(), vd.toChars());
errors = true;
}
}
else
{
// Will fix Bugzilla 1432 by enabling this path always

/* Prefer explicitly initialized field
* union U { int a; int b = 2; }
* U u; // OK (u.b == 2)
*/
if (!vx || !vx._init && v2._init)
{
vx = v2;
fieldi = j;
}
else if (vx != vd && !vx.isOverlappedWith(v2))
{
// Both vx and v2 fills vd, but vx and v2 does not overlap
}
else if (vx._init && v2._init)
{
.error(loc, "overlapping default initialization for field %s and %s",
v2.toChars(), vd.toChars());
errors = true;
}
else
assert(vx._init || !vx._init && !v2._init);
}
}
if (vx)
{
Expression e;
if (vx.type.size() == 0)
{
e = null;
}
else if (vx._init)
{
assert(!vx._init.isVoidInitializer());
e = vx.getConstInitializer(false);
}
else
{
if ((vx.storage_class & STCnodefaultctor) && !ctorinit)
{
.error(loc, "field %s.%s must be initialized because it has no default constructor",
type.toChars(), vx.toChars());
errors = true;
}
/* Bugzilla 12509: Get the element of static array type.
*/
Type telem = vx.type;
if (telem.ty == Tsarray)
{
/* We cannot use Type::baseElemOf() here.
* If the bottom of the Tsarray is an enum type, baseElemOf()
* will return the base of the enum, and its default initializer
* would be different from the enum's.
*/
while (telem.toBasetype().ty == Tsarray)
telem = (cast(TypeSArray)telem.toBasetype()).next;
if (telem.ty == Tvoid)
telem = Type.tuns8.addMod(telem.mod);
}
if (telem.needsNested() && ctorinit)
e = telem.defaultInit(loc);
else
e = telem.defaultInitLiteral(loc);
}
(*elements)[fieldi] = e;
}
}
foreach (e; *elements)
{
if (e && e.op == TOKerror)
return false;
}

return !errors;
}

/****************************
* Do byte or word alignment as necessary.
* Align sizes of 0, as we may not know array sizes yet.
Expand Down
3 changes: 2 additions & 1 deletion src/aggregate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ class AggregateDeclaration : public ScopeDsymbol
void semantic3(Scope *sc);
unsigned size(Loc loc);
virtual void finalizeSize(Scope *sc) = 0;
bool checkOverlappedFields();
bool fill(Loc loc, Expressions *elements, bool ctorinit);
static void alignmember(structalign_t salign, unsigned size, unsigned *poffset);
static unsigned placeField(unsigned *nextoffset,
unsigned memsize, unsigned memalignsize, structalign_t memalign,
Expand Down Expand Up @@ -185,7 +187,6 @@ class StructDeclaration : public AggregateDeclaration
const char *kind();
void finalizeSize(Scope *sc);
bool fit(Loc loc, Scope *sc, Expressions *elements, Type *stype);
bool fill(Loc loc, Expressions *elements, bool ctorinit);
bool isPOD();

StructDeclaration *isStructDeclaration() { return this; }
Expand Down
6 changes: 6 additions & 0 deletions src/dclass.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,7 @@ public:
else
structsize = Target.ptrsize * 2; // allow room for __vptr and __monitor
}

uint offset = structsize;
for (size_t i = 0; i < members.dim; i++)
{
Expand All @@ -1032,9 +1033,14 @@ public:
}
if (sizeok == SIZEOKfwd)
return;

// Add vptr's for any interfaces implemented by this class
structsize += setBaseInterfaceOffsets(structsize);
sizeok = SIZEOKdone;

// Calculate fields[i].overlapped
checkOverlappedFields();

// Look for the constructor
ctor = searchCtor();
if (ctor && ctor.toParent() != this)
Expand Down
72 changes: 54 additions & 18 deletions src/dinterpret.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3629,13 +3629,40 @@ public:
printf("ASSIGN: %s=%s\n", e1.toChars(), newval.toChars());
showCtfeExpr(newval);
}

/* Block assignment or element-wise assignment.
*/
if (e1.op == TOKslice || e1.op == TOKvector || e1.op == TOKarrayliteral || e1.op == TOKstring || e1.op == TOKnull && e1.type.toBasetype().ty == Tarray)
if (e1.op == TOKslice ||
e1.op == TOKvector ||
e1.op == TOKarrayliteral ||
e1.op == TOKstring ||
e1.op == TOKnull && e1.type.toBasetype().ty == Tarray)
{
// Note that slice assignments don't support things like ++, so
// we don't need to remember 'returnValue'.
result = interpretAssignToSlice(e, e1, newval, isBlockAssignment);
if (exceptionOrCant(result))
return;
if (e.e1.op == TOKslice)
{
Expression e1x = interpret((cast(SliceExp)e.e1).e1, istate, ctfeNeedLvalue);
if (e1x.op == TOKdotvar)
{
auto dve = cast(DotVarExp)e1x;
auto ex = dve.e1;
auto sle = ex.op == TOKstructliteral ? (cast(StructLiteralExp)ex)
: ex.op == TOKclassreference ? (cast(ClassReferenceExp)ex).value
: null;
auto v = dve.var.isVarDeclaration();
if (!sle || !v)
{
e.error("CTFE internal error: dotvar slice assignment");
result = CTFEExp.cantexp;
return;
}
stompOverlappedFields(sle, v);
}
}
return;
}
assert(result);
Expand All @@ -3646,6 +3673,22 @@ public:
return;
}

/* Set all sibling fields which overlap with v to VoidExp.
*/
void stompOverlappedFields(StructLiteralExp sle, VarDeclaration v)
{
if (!v.overlapped)
return;
foreach (size_t i, v2; sle.sd.fields)
{
if (v is v2 || !v.isOverlappedWith(v2))
continue;
auto e = (*sle.elements)[i];
if (e.op != TOKvoid)
(*sle.elements)[i] = voidInitLiteral(e.type, v).copy();
}
}

Expression assignToLvalue(BinExp e, Expression e1, Expression newval)
{
VarDeclaration vd = null;
Expand All @@ -3661,9 +3704,11 @@ public:
/* Assignment to member variable of the form:
* e.v = newval
*/
Expression ex = (cast(DotVarExp)e1).e1;
StructLiteralExp sle = ex.op == TOKstructliteral ? (cast(StructLiteralExp)ex) : ex.op == TOKclassreference ? (cast(ClassReferenceExp)ex).value : null;
VarDeclaration v = (cast(DotVarExp)e1).var.isVarDeclaration();
auto ex = (cast(DotVarExp)e1).e1;
auto sle = ex.op == TOKstructliteral ? (cast(StructLiteralExp)ex)
: ex.op == TOKclassreference ? (cast(ClassReferenceExp)ex).value
: null;
auto v = (cast(DotVarExp)e1).var.isVarDeclaration();
if (!sle || !v)
{
e.error("CTFE internal error: dotvar assignment");
Expand All @@ -3674,27 +3719,18 @@ public:
e.error("cannot modify read-only constant %s", sle.toChars());
return CTFEExp.cantexp;
}
int fieldi = ex.op == TOKstructliteral ? findFieldIndexByName(sle.sd, v) : (cast(ClassReferenceExp)ex).findFieldIndexByName(v);
int fieldi = ex.op == TOKstructliteral ? findFieldIndexByName(sle.sd, v)
: (cast(ClassReferenceExp)ex).findFieldIndexByName(v);
if (fieldi == -1)
{
e.error("CTFE internal error: cannot find field %s in %s", v.toChars(), ex.toChars());
return CTFEExp.cantexp;
}
assert(0 <= fieldi && fieldi < sle.elements.dim);

// If it's a union, set all other members of this union to void
if (ex.op == TOKstructliteral && v.overlapped)
{
assert(sle.sd);
for (size_t i = 0; i < sle.sd.fields.dim; i++)
{
VarDeclaration v2 = sle.sd.fields[i];
if (!v.isOverlappedWith(v2))
continue;
Expression* exp = &(*sle.elements)[i];
if ((*exp).op != TOKvoid)
*exp = voidInitLiteral((*exp).type, v).copy();
}
}
stompOverlappedFields(sle, v);

payload = &(*sle.elements)[fieldi];
oldval = *payload;
}
Expand Down
Loading

0 comments on commit 16969dc

Please sign in to comment.