Skip to content

Issue 11510 - Relax restriction for overlapped pointer field access in safe code/during CTFE #2757

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.

Sign up for GitHub

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

Merged
WalterBright merged 4 commits into from
Dec 16, 2013
Merged

Issue 11510 - Relax restriction for overlapped pointer field access in safe code/during CTFE #2757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d4810c0
[style] Remove trailing spaces
9rnsr Nov 13, 2013
115d98c
[Refactoring] Move StructLiteralExp::fill into StructDeclaration
9rnsr Nov 13, 2013
a293298
fix Issue 11510 - Relax restriction for overlapped pointer field acce…
9rnsr Nov 13, 2013
090abe1
fix Issue 11427 - anonymous unions break structs in @safe code
9rnsr Nov 13, 2013
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
2 changes: 1 addition & 1 deletion src/aggregate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class AggregateDeclaration : public ScopeDsymbol
Type *handle; // 'this' type
unsigned structsize; // size of struct
unsigned alignsize; // size of struct for alignment purposes
int hasUnions; // set if aggregate has overlapping fields
VarDeclarations fields; // VarDeclaration fields
Sizeok sizeok; // set when structsize contains valid data
Dsymbol *deferred; // any deferred semantic2() or semantic3() symbol
Expand Down Expand Up @@ -166,6 +165,7 @@ class StructDeclaration : public AggregateDeclaration
const char *mangle(bool isv = false);
const char *kind();
void finalizeSize(Scope *sc);
bool fill(Loc loc, Expressions *elements, bool ctorinit);
bool isPOD();
int needOpAssign();
int needOpEquals();
Expand Down
21 changes: 6 additions & 15 deletions src/declaration.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ VarDeclaration::VarDeclaration(Loc loc, Type *type, Identifier *id, Initializer
aliassym = NULL;
onstack = 0;
canassign = 0;
overlapped = false;
lastVar = NULL;
ctfeAdrOnStack = -1;
rundtor = NULL;
Expand Down Expand Up @@ -899,30 +900,20 @@ void VarDeclaration::semantic(Scope *sc)
//printf("sc->stc = %x\n", sc->stc);
//printf("storage_class = x%x\n", storage_class);

// Safety checks
// Calculate type size + safety checks
if (sc->func && !sc->intypeof)
{
if (storage_class & STCgshared)
{
if (sc->func->setUnsafe())
error("__gshared not allowed in safe functions; use shared");
}
if (init && init->isVoidInitializer() && type->hasPointers())
if (type->hasPointers()) // get type size
{
if (sc->func->setUnsafe())
error("void initializers for pointers not allowed in safe functions");
}
if (type->hasPointers() && type->toDsymbol(sc))
{
Dsymbol *s = type->toDsymbol(sc);
if (s)
if (init && init->isVoidInitializer())
{
AggregateDeclaration *ad2 = s->isAggregateDeclaration();
if (ad2 && ad2->hasUnions)
{
if (sc->func->setUnsafe())
error("unions containing pointers are not allowed in @safe functions");
}
if (sc->func->setUnsafe())
error("void initializers for pointers not allowed in safe functions");
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/declaration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ class VarDeclaration : public Declaration
short onstack; // 1: it has been allocated on the stack
// 2: on stack, run destructor anyway
int canassign; // it can be assigned to
bool overlapped; // if it is a field and has overlapping
Dsymbol *aliassym; // if redone as alias to another symbol
VarDeclaration *lastVar; // Linked list of variables for goto-skips-init detection

Expand Down
153 changes: 23 additions & 130 deletions src/expression.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,23 @@ Expression *resolvePropertiesX(Scope *sc, Expression *e1, Expression *e2 = NULL)
return e->semantic(sc);
}
}
else if (e1->op == TOKdotvar)
{
// Check for reading overlapped pointer field in @safe code.
VarDeclaration *v = ((DotVarExp *)e1)->var->isVarDeclaration();
if (v && v->overlapped &&
sc->func && !sc->intypeof)
{
AggregateDeclaration *ad = v->toParent2()->isAggregateDeclaration();
if (ad && e1->type->hasPointers() &&
sc->func->setUnsafe())
{
e1->error("field %s.%s cannot be accessed in @safe code because it overlaps with a pointer",
ad->toChars(), v->toChars());
return new ErrorExp();
}
}
}
else if (e1->op == TOKdotexp)
{
e1->error("expression has no value");
Expand Down Expand Up @@ -4606,17 +4623,15 @@ Expression *StructLiteralExp::semantic(Scope *sc)

/* Fill out remainder of elements[] with default initializers for fields[]
*/
Expression *e = fill(false);
if (e->op == TOKerror)
if (!sd->fill(loc, elements, false))
{
/* An error in the initializer needs to be recorded as an error
* in the enclosing function or template, since the initializer
* will be part of the stuct declaration.
*/
global.increaseErrorCount();
return e;
return new ErrorExp();
}
assert(e == this);
type = stype ? stype : sd->type;
return this;
}
Expand All @@ -4640,115 +4655,6 @@ Expression *StructLiteralExp::addDtorHook(Scope *sc)
return this;
}

Expression *StructLiteralExp::fill(bool ctorinit)
{
assert(sd && sd->sizeok == SIZEOKdone);
size_t nfields = sd->fields.dim - sd->isNested();

size_t dim = elements->dim;
elements->setDim(nfields);
for (size_t i = dim; i < nfields; i++)
(*elements)[i] = NULL;

// Fill in missing any elements with default initializers
for (size_t i = 0; i < nfields; i++)
{
if ((*elements)[i])
continue;
VarDeclaration *vd = sd->fields[i];
VarDeclaration *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;
for (size_t j = 0; j < nfields; j++)
{
if (i == j)
continue;
VarDeclaration *v2 = sd->fields[j];
if (v2->init && v2->init->isVoidInitializer())
continue;

bool overlap = (vd->offset < v2->offset + v2->type->size() &&
v2->offset < vd->offset + vd->type->size());
if (!overlap)
continue;

sd->hasUnions = 1; // note that directly unrelated...

if ((*elements)[j])
{
vx = NULL;
break;
}

#if 1
/* 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("overlapping initialization for field %s and %s",
v2->toChars(), vd->toChars());
}
#else // fix Bugzilla 1432
/* 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->offset < v2->offset + v2->type->size() &&
v2->offset < vx->offset + vx->type->size()))
{
// Both vx and v2 fills vd, but vx and v2 does not overlap
}
else if (vx->init && v2->init)
{
error("overlapping default initialization for field %s and %s",
v2->toChars(), vd->toChars());
}
else
assert(vx->init || !vx->init && !v2->init);
#endif
}
if (vx)
{
Expression *e;
if (vx->init)
{
assert(!vx->init->isVoidInitializer());
e = vx->getConstInitializer(false);
}
else
{
if ((vx->storage_class & STCnodefaultctor) && !ctorinit)
{
error("field %s.%s must be initialized because it has no default constructor",
sd->type->toChars(), vx->toChars());
}
if (vx->type->needsNested() && ctorinit)
e = vx->type->defaultInit(loc);
else
e = vx->type->defaultInitLiteral(loc);
}
(*elements)[fieldi] = e;
}
}

for (size_t i = 0; i < elements->dim; i++)
{
Expression *e = (*elements)[i];
if (e && e->op == TOKerror)
return e;
}
return this;
}

/**************************************
* Gets expression at offset of type.
* Returns NULL if not found.
Expand Down Expand Up @@ -7925,19 +7831,6 @@ Expression *DotVarExp::semantic(Scope *sc)
return e;
}
}
Dsymbol *s;
if (sc->func && !sc->intypeof && t1->hasPointers() &&
(s = t1->toDsymbol(sc)) != NULL)
{
AggregateDeclaration *ad = s->isAggregateDeclaration();
if (ad && ad->hasUnions)
{
if (sc->func->setUnsafe())
{ error("union %s containing pointers are not allowed in @safe functions", t1->toChars());
goto Lerr;
}
}
}
}

//printf("-DotVarExp::semantic('%s')\n", toChars());
Expand Down Expand Up @@ -8705,10 +8598,10 @@ Expression *CallExp::semantic(Scope *sc)
ExpInitializer *ei = NULL;
if (t1->needsNested())
{
StructLiteralExp *sle = new StructLiteralExp(loc, (StructDeclaration *)ad, NULL, e1->type);
Expression *e = sle->fill(true);
if (e->op == TOKerror)
return e;
StructDeclaration *sd = (StructDeclaration *)ad;
StructLiteralExp *sle = new StructLiteralExp(loc, sd, NULL, e1->type);
if (!sd->fill(loc, sle->elements, true))
return new ErrorExp();
sle->type = type;
ei = new ExpInitializer(loc, sle);
}
Expand Down
1 change: 0 additions & 1 deletion src/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,6 @@ class StructLiteralExp : public Expression
Expression *syntaxCopy();
int apply(apply_fp_t fp, void *param);
Expression *semantic(Scope *sc);
Expression *fill(bool ctorinit);
Expression *getField(Type *type, unsigned offset);
int getFieldIndex(Type *type, unsigned offset);
elem *toElem(IRState *irs);
Expand Down
8 changes: 3 additions & 5 deletions src/init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,11 @@ Initializer *StructInitializer::semantic(Scope *sc, Type *t, NeedInterpret needI
return new ErrorInitializer();

StructLiteralExp *sle = new StructLiteralExp(loc, sd, elements, t);
Expression *e = sle->fill(false);
if (e->op == TOKerror)
if (!sd->fill(loc, elements, false))
return new ErrorInitializer();
sle->type = t;

e->type = t;

ExpInitializer *ie = new ExpInitializer(loc, e);
ExpInitializer *ie = new ExpInitializer(loc, sle);
return ie->semantic(sc, t, needInterpret);
}
else if ((t->ty == Tdelegate || t->ty == Tpointer && t->nextOf()->ty == Tfunction) && value.dim == 0)
Expand Down
65 changes: 42 additions & 23 deletions src/interpret.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5882,41 +5882,56 @@ Expression *DotVarExp::interpret(InterState *istate, CtfeGoal goal)
#endif
if (ex->op == TOKaddress)
ex = ((AddrExp *)ex)->e1;

VarDeclaration *v = var->isVarDeclaration();
if (!v)
{
error("CTFE internal error: %s", toChars());
return EXP_CANT_INTERPRET;
}
if (ex->op == TOKnull && ex->type->toBasetype()->ty == Tclass)
{ error("class '%s' is null and cannot be dereferenced", e1->toChars());
return EXP_CANT_INTERPRET;
}
if (ex->op == TOKnull)
{ error("dereference of null pointer '%s'", e1->toChars());
{
if (ex->type->toBasetype()->ty == Tclass)
error("class '%s' is null and cannot be dereferenced", e1->toChars());
else
error("dereference of null pointer '%s'", e1->toChars());
return EXP_CANT_INTERPRET;
}
if (ex->op == TOKstructliteral || ex->op == TOKclassreference)
{
StructLiteralExp *se = ex->op == TOKclassreference ? ((ClassReferenceExp *)ex)->value : (StructLiteralExp *)ex;
/* We don't know how to deal with overlapping fields
*/
if (se->sd->hasUnions)
{ error("Unions with overlapping fields are not yet supported in CTFE");
return EXP_CANT_INTERPRET;
}
StructLiteralExp *se;
int i;

// We can't use getField, because it makes a copy
int i = -1;
if (ex->op == TOKclassreference)
i = ((ClassReferenceExp *)ex)->findFieldIndexByName(v);
{
se = ((ClassReferenceExp *)ex)->value;
i = ((ClassReferenceExp *)ex)->findFieldIndexByName(v);
}
else
i = findFieldIndexByName(se->sd, v);
{
se = (StructLiteralExp *)ex;
i = findFieldIndexByName(se->sd, v);
}
#if DMDV1
if (se->sd->hasUnions)
{
error("Unions with overlapping fields are not yet supported in CTFE");
return EXP_CANT_INTERPRET;
}
#endif
if (i == -1)
{
error("couldn't find field %s of type %s in %s", v->toChars(), type->toChars(), se->toChars());
return EXP_CANT_INTERPRET;
}
e = (*se->elements)[i];
if (!e)
{
error("Internal Compiler Error: Null field %s", v->toChars());
return EXP_CANT_INTERPRET;
}

if (goal == ctfeNeedLvalue || goal == ctfeNeedLvalueRef)
{
// If it is an lvalue literal, return it...
Expand All @@ -5939,27 +5954,31 @@ Expression *DotVarExp::interpret(InterState *istate, CtfeGoal goal)
e->type = type;
return e;
}
if (!e)
{
error("Internal Compiler Error: Null field %s", v->toChars());
return EXP_CANT_INTERPRET;
}
// If it is an rvalue literal, return it...
if (e->op == TOKstructliteral || e->op == TOKarrayliteral ||
e->op == TOKassocarrayliteral || e->op == TOKstring)
return e;
if (e->op == TOKvoid)
{
VoidInitExp *ve = (VoidInitExp *)e;
error("cannot read uninitialized variable %s in CTFE", ve->var->toChars());
const char *s = ve->var->toChars();
#if DMDV2
if (v->overlapped)
{
error("Reinterpretation through overlapped field %s is not allowed in CTFE", s);
return EXP_CANT_INTERPRET;
}
#endif
error("cannot read uninitialized variable %s in CTFE", s);
return EXP_CANT_INTERPRET;
}
if ( isPointer(type) )
if (isPointer(type))
{
return paintTypeOntoLiteral(type, e);
}
if (e->op == TOKvar)
{ // Don't typepaint twice, since that might cause an erroneous copy
{
// Don't typepaint twice, since that might cause an erroneous copy
e = getVarExp(loc, istate, ((VarExp *)e)->var, goal);
if (e != EXP_CANT_INTERPRET && e->op != TOKthrownexception)
e = paintTypeOntoLiteral(type, e);
Expand Down
Loading