Skip to content

Commit

Permalink
Merge pull request #4765 from 9rnsr/fix14730
Browse files Browse the repository at this point in the history 
Issue 14730 - Wrong closure var access with -inline
  • Loading branch information
@ibuclaw
ibuclaw committed Feb 7, 2016
2 parents 8388211 + 2a1fe8c commit 2d0bcf1
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 51 deletions.
9 changes: 9 additions & 0 deletions src/e2ir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Symbol* toSymbol(ClassReferenceExp *cre);
elem *filelinefunction(IRState *irs, Expression *e);
void toTraceGC(IRState *irs, elem *e, Loc *loc);
void genTypeInfo(Type *t, Scope *sc);
void setClosureVarOffset(FuncDeclaration *fd);

int callSideEffectLevel(FuncDeclaration *f);
int callSideEffectLevel(Type *t);
Expand Down Expand Up @@ -3418,6 +3419,14 @@ elem *toElem(Expression *e, IRState *irs)
#endif
assert(txb->ty == tyb->ty);

// Bugzilla 14730
if (global.params.useInline && v->offset == 0)
{
FuncDeclaration *fd = v->parent->isFuncDeclaration();
if (fd && fd->semanticRun < PASSobj)
setClosureVarOffset(fd);
}

elem *e = toElem(dve->e1, irs);
Type *tb1 = dve->e1->type->toBasetype();
if (tb1->ty != Tclass && tb1->ty != Tpointer)
Expand Down
125 changes: 74 additions & 51 deletions src/toir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,61 @@ elem *resolveLengthVar(VarDeclaration *lengthVar, elem **pe, Type *t1)
return einit;
}

void setClosureVarOffset(FuncDeclaration *fd)
{
if (fd->needsClosure())
{
unsigned offset = Target::ptrsize; // leave room for previous sthis

for (size_t i = 0; i < fd->closureVars.dim; i++)
{
VarDeclaration *v = fd->closureVars[i];

/* Align and allocate space for v in the closure
* just like AggregateDeclaration::addField() does.
*/
unsigned memsize;
unsigned memalignsize;
structalign_t xalign;
if (v->storage_class & STClazy)
{
/* Lazy variables are really delegates,
* so give same answers that TypeDelegate would
*/
memsize = Target::ptrsize * 2;
memalignsize = memsize;
xalign = STRUCTALIGN_DEFAULT;
}
else if (v->storage_class & (STCout | STCref))
{
// reference parameters are just pointers
memsize = Target::ptrsize;
memalignsize = memsize;
xalign = STRUCTALIGN_DEFAULT;
}
else
{
memsize = v->type->size();
memalignsize = v->type->alignsize();
xalign = v->alignment;
}
AggregateDeclaration::alignmember(xalign, memalignsize, &offset);
v->offset = offset;
//printf("closure var %s, offset = %d\n", v->toChars(), v->offset);

offset += memsize;

/* Can't do nrvo if the variable is put in a closure, since
* what the shidden points to may no longer exist.
*/
if (fd->nrvo_can && fd->nrvo_var == v)
{
fd->nrvo_can = 0;
}
}
}
}

/*************************************
* Closures are implemented by taking the local variables that
* need to survive the scope of the function, and copying them
Expand All @@ -656,6 +711,8 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
{
if (fd->needsClosure())
{
setClosureVarOffset(fd);

// Generate closure on the heap
// BUG: doesn't capture variadic arguments passed to this function

Expand All @@ -671,7 +728,7 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
* ~this() { call destructor }
* }
*/
//printf("FuncDeclaration::buildClosure() %s\n", toChars());
//printf("FuncDeclaration::buildClosure() %s\n", fd->toChars());

/* Generate type name for closure struct */
const char *name1 = "CLOSURE.";
Expand All @@ -690,12 +747,12 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
symbol_add(sclosure);
irs->sclosure = sclosure;

unsigned offset = Target::ptrsize; // leave room for previous sthis
assert(fd->closureVars.dim);
assert(fd->closureVars[0]->offset >= Target::ptrsize);
for (size_t i = 0; i < fd->closureVars.dim; i++)
{
VarDeclaration *v = fd->closureVars[i];
//printf("closure var %s\n", v->toChars());
assert(v->isVarDeclaration());

if (v->needsAutoDtor())
{
Expand All @@ -711,43 +768,6 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
*/
v->error("cannot reference variadic arguments from closure");
}
/* Align and allocate space for v in the closure
* just like AggregateDeclaration::addField() does.
*/
unsigned memsize;
unsigned memalignsize;
structalign_t xalign;
if (v->storage_class & STClazy)
{
/* Lazy variables are really delegates,
* so give same answers that TypeDelegate would
*/
memsize = Target::ptrsize * 2;
memalignsize = memsize;
xalign = STRUCTALIGN_DEFAULT;
}
else if (ISWIN64REF(v))
{
memsize = v->type->size();
memalignsize = v->type->alignsize();
xalign = v->alignment;
}
else if (ISREF(v, NULL))
{
// reference parameters are just pointers
memsize = Target::ptrsize;
memalignsize = memsize;
xalign = STRUCTALIGN_DEFAULT;
}
else
{
memsize = v->type->size();
memalignsize = v->type->alignsize();
xalign = v->alignment;
}
AggregateDeclaration::alignmember(xalign, memalignsize, &offset);
v->offset = offset;
offset += memsize;

/* Set Sscope to closure */
Symbol *vsym = toSymbol(v);
Expand All @@ -757,20 +777,23 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
/* Add variable as closure type member */
symbol_struct_addField(Closstru->Ttag, vsym->Sident, vsym->Stype, v->offset);
//printf("closure field %s: memalignsize: %i, offset: %i\n", vsym->Sident, memalignsize, v->offset);

/* Can't do nrvo if the variable is put in a closure, since
* what the shidden points to may no longer exist.
*/
if (fd->nrvo_can && fd->nrvo_var == v)
{
fd->nrvo_can = 0;
}
}
// offset is now the size of the closure
Closstru->Ttag->Sstruct->Sstructsize = offset;

// Calculate the size of the closure
VarDeclaration *vlast = fd->closureVars[fd->closureVars.dim - 1];
unsigned structsize;
if (vlast->storage_class & STClazy)
structsize = vlast->offset + Target::ptrsize * 2;
else if (vlast->isRef() || vlast->isOut())
structsize = vlast->offset + Target::ptrsize;
else
structsize = vlast->offset + vlast->type->size();
//printf("structsize = %d\n", structsize);

Closstru->Ttag->Sstruct->Sstructsize = structsize;

// Allocate memory for the closure
elem *e = el_long(TYsize_t, offset);
elem *e = el_long(TYsize_t, structsize);
e = el_bin(OPcall, TYnptr, el_var(getRtlsym(RTLSYM_ALLOCMEMORY)), e);
toTraceGC(irs, e, &fd->loc);

Expand Down
53 changes: 53 additions & 0 deletions test/runnable/closure.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,57 @@ void test12406()
}
}

/************************************/
// 14730

void test14730()
{
static auto makeS(int x)
{
struct S
{
int n;
int get() { return x; } // x will be a closure variable
}
return S(x);
}
auto s = makeS(1);
assert(s.get() == 1);
// By inlining get() function call, it's rewritten to:
// assert(*(s.tupleof[$-1] + x.offset) == 1);
// --> In DotVarExp::toElem(), x->offset should be already nonzero.
}

// ----

// This is questionable case. Currently it works without any errors,
// but not sure it's really intentional

struct S14730x(alias f)
{
auto foo()() { return f(0); }

void dummy() {}
}

auto makeS14730x() //@nogc
{
int x = 10;
S14730x!(a => x) s;
//assert(s.foo() == 10);
return s;
}

void test14730x()
{
auto s = makeS14730x();
assert(s.tupleof[$-1] !is null);

// instantiationg foo outside of makeS will place the variable x in closure
// *after* the semantic3 completion of makeS() function.
assert(s.foo() == 10);
}

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

int main()
Expand Down Expand Up @@ -929,6 +980,8 @@ int main()
test9685a();
test9685b();
test12406();
test14730();
test14730x();

printf("Success\n");
return 0;
Expand Down

0 comments on commit 2d0bcf1

Please sign in to comment.