From aea25d6b6ea8383058f4987fe11ae16bbda18852 Mon Sep 17 00:00:00 2001 From: Walter Bright Date: Mon, 20 Oct 2014 13:25:27 -0700 Subject: [PATCH] fix Issue 13586 - Destructors not run when argument list evaluation throws --- src/backend/cod2.c | 6 ++ src/backend/cod3.c | 5 +- src/backend/gdag.c | 1 + src/backend/gflow.c | 1 + src/declaration.h | 3 +- src/e2ir.c | 15 ++++ src/expression.c | 168 +++++++++++++++++++++++++++++++++++++++--- src/expression.h | 2 + src/interpret.c | 8 ++ src/tocsym.c | 2 +- test/runnable/sdtor.d | 115 +++++++++++++++++++++++++---- 11 files changed, 297 insertions(+), 29 deletions(-) diff --git a/src/backend/cod2.c b/src/backend/cod2.c index bfa036f61202..553fb35676f0 100644 --- a/src/backend/cod2.c +++ b/src/backend/cod2.c @@ -5108,6 +5108,12 @@ code *cdddtor(elem *e,regm_t *pretregs) cd = cat(cd, nteh_gensindex(0)); // the actual index will be patched in later // by except_fillInEHTable() + // Mark all registers as destroyed + { + code *cy = getregs(allregs); + assert(!cy); + } + assert(*pretregs == 0); code *c = codelem(e->E1,pretregs,FALSE); gen1(c,0xC3); // RET diff --git a/src/backend/cod3.c b/src/backend/cod3.c index 082ad455fefa..21deb112d2be 100644 --- a/src/backend/cod3.c +++ b/src/backend/cod3.c @@ -1416,6 +1416,8 @@ void doswitch(block *b) gen2sib(ce,LEA,(REX_W << 16) | modregxrm(0,r1,4),modregxrmx(0,r1,r2)); // LEA R1,[R1][R2] gen2(ce,0xFF,modregrmx(3,4,r1)); // JMP R1 + pinholeopt(ce, NULL); + b->Btablesize = (int) (vmax - vmin + 1) * 4; } else @@ -5243,7 +5245,8 @@ void pinholeopt(code *c,block *b) } // Replace [R13] with 0[R13] - if (c->Irex & REX_B && (c->Irm & modregrm(3,0,5)) == modregrm(0,0,5)) + if (c->Irex & REX_B && ((c->Irm & modregrm(3,0,7)) == modregrm(0,0,BP) || + issib(c->Irm) && (c->Irm & modregrm(3,0,0)) == 0 && (c->Isib & 7) == BP)) { c->Irm |= modregrm(1,0,0); c->IFL1 = FLconst; diff --git a/src/backend/gdag.c b/src/backend/gdag.c index e2c89cf357fe..776e25d462dc 100644 --- a/src/backend/gdag.c +++ b/src/backend/gdag.c @@ -258,6 +258,7 @@ STATIC void aewalk(register elem **pn,register vec_t ae) case OPdctor: break; case OPasm: + case OPddtor: vec_clear(ae); // kill everything return; diff --git a/src/backend/gflow.c b/src/backend/gflow.c index eedf3fb818bf..61c17537cb09 100644 --- a/src/backend/gflow.c +++ b/src/backend/gflow.c @@ -974,6 +974,7 @@ STATIC void accumaecpx(elem *n) vec_free(Kr); break; } + case OPddtor: case OPasm: assert(!n->Eexp); // no ASM available expressions vec_set(KILL); // KILL everything diff --git a/src/declaration.h b/src/declaration.h index d21d67bd1c0c..7b1b9e9765e7 100644 --- a/src/declaration.h +++ b/src/declaration.h @@ -83,7 +83,8 @@ enum PURE; #define STCnodefaultctor 0x8000000000LL // must be set inside constructor #define STCtemp 0x10000000000LL // temporary variable #define STCrvalue 0x20000000000LL // force rvalue for variables -#define STCnogc 0x40000000000LL // @nogc +#define STCnogc 0x40000000000LL // @nogc +#define STCvolatile 0x80000000000LL // destined for volatile in the back end const StorageClass STCStorageClass = (STCauto | STCscope | STCstatic | STCextern | STCconst | STCfinal | STCabstract | STCsynchronized | STCdeprecated | STCoverride | STClazy | STCalias | diff --git a/src/e2ir.c b/src/e2ir.c index 376a051e71cf..830d43aaec08 100644 --- a/src/e2ir.c +++ b/src/e2ir.c @@ -1383,6 +1383,7 @@ elem *toElem(Expression *e, IRState *irs) elem *ex = NULL; elem *ey = NULL; + elem *ezprefix = NULL; elem *ez = NULL; if (ne->allocator || ne->onstack) @@ -1489,11 +1490,14 @@ elem *toElem(Expression *e, IRState *irs) if (ne->member) { + if (ne->argprefix) + ezprefix = toElem(ne->argprefix, irs); // Call constructor ez = callfunc(ne->loc, irs, 1, ne->type, ez, ectype, ne->member, ne->member->type, NULL, ne->arguments); } e = el_combine(ex, ey); + e = el_combine(e, ezprefix); e = el_combine(e, ez); } else if (t->ty == Tpointer && t->nextOf()->toBasetype()->ty == Tstruct) @@ -1511,6 +1515,7 @@ elem *toElem(Expression *e, IRState *irs) elem *ex = NULL; elem *ey = NULL; + elem *ezprefix = NULL; elem *ez = NULL; if (ne->allocator) @@ -1537,6 +1542,8 @@ elem *toElem(Expression *e, IRState *irs) elem *ev = el_same(&ex); + if (ne->argprefix) + ezprefix = toElem(ne->argprefix, irs); if (ne->member) { if (sd->isNested()) @@ -1579,12 +1586,15 @@ elem *toElem(Expression *e, IRState *irs) //elem_print(ez); e = el_combine(ex, ey); + e = el_combine(e, ezprefix); e = el_combine(e, ez); } else if (t->ty == Tarray) { TypeDArray *tda = (TypeDArray *)t; + elem *ezprefix = ne->argprefix ? toElem(ne->argprefix, irs) : NULL; + assert(ne->arguments && ne->arguments->dim >= 1); if (ne->arguments->dim == 1) { @@ -1616,11 +1626,13 @@ elem *toElem(Expression *e, IRState *irs) e = el_bin(OPcall,TYdarray,el_var(rtlsym[rtl]),e); e->Eflags |= EFLAGS_variadic; } + e = el_combine(ezprefix, e); } else if (t->ty == Tpointer) { TypePointer *tp = (TypePointer *)t; Expression *di = tp->next->defaultInit(); + elem *ezprefix = ne->argprefix ? toElem(ne->argprefix, irs) : NULL; // call _d_newitemT(ti) e = toElem(ne->newtype->getTypeInfo(NULL), irs); @@ -1630,6 +1642,8 @@ elem *toElem(Expression *e, IRState *irs) if (ne->arguments && ne->arguments->dim == 1) { + /* ezprefix, ts=_d_newitemT(ti), *ts=arguments[0], ts + */ elem *e2 = toElem((*ne->arguments)[0], irs); symbol *ts = symbol_genauto(Type_toCtype(tp)); @@ -1642,6 +1656,7 @@ elem *toElem(Expression *e, IRState *irs) e = el_combine(e, el_var(ts)); //elem_print(e); } + e = el_combine(ezprefix, e); } else { diff --git a/src/expression.c b/src/expression.c index c4f2ba390ba3..78b36ad9f84c 100644 --- a/src/expression.c +++ b/src/expression.c @@ -1403,12 +1403,13 @@ Expression *callCpCtor(Scope *sc, Expression *e) * fd the function being called, NULL if called indirectly * Output: * *prettype return type of function + * *peprefix expression to execute before arguments[] are evaluated, NULL if none * Returns: * true errors happened */ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, - Type *tthis, Expressions *arguments, FuncDeclaration *fd, Type **prettype) + Type *tthis, Expressions *arguments, FuncDeclaration *fd, Type **prettype, Expression **peprefix) { //printf("functionParameters()\n"); assert(arguments); @@ -1418,6 +1419,8 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, unsigned olderrors = global.errors; bool err = false; *prettype = Type::terror; + Expression *eprefix = NULL; + *peprefix = NULL; if (nargs > nparams && tf->varargs == 0) { @@ -1685,7 +1688,7 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, } else { - arg = arg->isLvalue() ? callCpCtor(sc, arg) : valueNoDtor(arg); +// arg = arg->isLvalue() ? callCpCtor(sc, arg) : valueNoDtor(arg); } //printf("arg: %s\n", arg->toChars()); @@ -1785,7 +1788,7 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, } if (tb->ty == Tstruct) { - arg = callCpCtor(sc, arg); +// arg = callCpCtor(sc, arg); } // Give error for overloaded function addresses @@ -1800,10 +1803,147 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, arg->rvalue(); arg = arg->optimize(WANTvalue); } - L3: (*arguments)[i] = arg; } + /* Remaining problems: + * 1. order of evaluation - some function push L-to-R, others R-to-L. Until we resolve what array assignment does (which is + * implemented by calling a function) we'll defer this for now. + * 2. value structs (or static arrays of them) that need to be copy constructed + * 3. value structs (or static arrays of them) that have destructors, and subsequent arguments that may throw before the + * function gets called (functions normally destroy their parameters) + * 2 and 3 are handled by doing the argument construction in 'eprefix' so that if a later argument throws, they are cleaned + * up properly. Pushing arguments on the stack then cannot fail. + */ + if (1) + { + /* Compute indices of first and last throwing argument. + * Used to not set up destructors unless a throw can happen in a later argument. + */ + bool anythrow = false; + size_t firstthrow = ~0; + size_t lastthrow = ~0; + for (size_t i = 0; i < arguments->dim; ++i) + { + Expression *arg = (*arguments)[i]; + if (canThrow(arg, sc->func, false)) + { + if (!anythrow) + { + anythrow = true; + firstthrow = i; + } + lastthrow = i; + } + } + + bool appendToPrefix = false; + VarDeclaration *gate = NULL; + for (size_t i = 0; i < arguments->dim; ++i) + { + Expression *arg = (*arguments)[i]; + + /* Skip reference parameters + */ + if (i < nparams) + { + Parameter *p = Parameter::getNth(tf->parameters, i); + if (p->storageClass & (STClazy | STCref | STCout)) + continue; + } + + TypeStruct *ts = NULL; + Type *tv = arg->type->baseElemOf(); + if (tv->ty == Tstruct) + ts = (TypeStruct *)tv; + + if (anythrow && i < lastthrow) // if there are throws after this arg + { + if (ts && ts->sym->dtor) + { + appendToPrefix = true; + + // Need the gate because throws may occur after this arg is constructed + if (!gate) + { + Identifier *idtmp = Lexer::uniqueId("__gate"); + gate = new VarDeclaration(loc, Type::tbool, idtmp, NULL); + gate->storage_class |= STCtemp | STCctfe | STCvolatile; + gate->semantic(sc); + + Expression *ae = new DeclarationExp(loc, gate); + ae = ae->semantic(sc); + eprefix = Expression::combine(eprefix, ae); + } + } + } + if (anythrow && i == lastthrow) + { + appendToPrefix = false; + } + if (appendToPrefix) // don't need to add to prefix until there's something to destruct + { + Identifier *idtmp = Lexer::uniqueId("__pfx"); + VarDeclaration *tmp = new VarDeclaration(loc, arg->type, idtmp, new ExpInitializer(loc, arg)); + tmp->storage_class |= STCtemp | STCctfe; + tmp->semantic(sc); + + /* Modify the destructor so it only runs if gate==false + */ + if (tmp->edtor) + { + Expression *e = tmp->edtor; + e = new OrOrExp(e->loc, new VarExp(e->loc, gate), e); // (gate || destructor) + tmp->edtor = e->semantic(sc); + //printf("edtor: %s\n", tmp->edtor->toChars()); + } + + // auto __pfx = arg + Expression *ae = new DeclarationExp(loc, tmp); + ae = ae->semantic(sc); + eprefix = Expression::combine(eprefix, ae); + + arg = new VarExp(loc, tmp); + arg = arg->semantic(sc); + } + else if (ts) + { + arg = arg->isLvalue() ? callCpCtor(sc, arg) : valueNoDtor(arg); + } + else if (anythrow && firstthrow <= i && i <= lastthrow && gate) + { + Identifier *id = Lexer::uniqueId("__pfy"); + VarDeclaration *tmp = new VarDeclaration(loc, arg->type, id, new ExpInitializer(loc, arg)); + tmp->storage_class |= STCtemp | STCctfe; + tmp->semantic(sc); + + Expression *ae = new DeclarationExp(loc, tmp); + ae = ae->semantic(sc); + eprefix = Expression::combine(eprefix, ae); + + arg = new VarExp(loc, tmp); + arg = arg->semantic(sc); + } + + if (anythrow && i == lastthrow) + { + /* Set gate to true after prefix runs + */ + if (eprefix) + { + assert(gate); + // (gate = true) + Expression *e = new AssignExp(gate->loc, new VarExp(gate->loc, gate), new IntegerExp(gate->loc, 1, Type::tbool)); + e = e->semantic(sc); + eprefix = Expression::combine(eprefix, e); + gate = NULL; + } + } + (*arguments)[i] = arg; + } + } + //if (eprefix) printf("eprefix: %s\n", eprefix->toChars()); + // If D linkage and variadic, add _arguments[] as first argument if (tf->linkage == LINKd && tf->varargs == 1) { @@ -1844,6 +1984,7 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf, tret = tret->substWildTo(wildmatch); } *prettype = tret; + *peprefix = eprefix; return (err || olderrors != global.errors); } @@ -4471,6 +4612,7 @@ NewExp::NewExp(Loc loc, Expression *thisexp, Expressions *newargs, this->newargs = newargs; this->newtype = newtype; this->arguments = arguments; + argprefix = NULL; member = NULL; allocator = NULL; onstack = 0; @@ -4498,6 +4640,7 @@ Expression *NewExp::semantic(Scope *sc) Type *tb; ClassDeclaration *cdthis = NULL; size_t nargs; + Expression *newprefix = NULL; Lagain: if (thisexp) @@ -4678,7 +4821,7 @@ Expression *NewExp::semantic(Scope *sc) if (!arguments) arguments = new Expressions(); - if (functionParameters(loc, sc, tf, type, arguments, f, &type)) + if (functionParameters(loc, sc, tf, type, arguments, f, &type, &argprefix)) return new ErrorExp(); } else @@ -4706,7 +4849,7 @@ Expression *NewExp::semantic(Scope *sc) TypeFunction *tf = (TypeFunction *)f->type; Type *rettype; - if (functionParameters(loc, sc, tf, NULL, newargs, f, &rettype)) + if (functionParameters(loc, sc, tf, NULL, newargs, f, &rettype, &newprefix)) return new ErrorExp(); } else @@ -4747,7 +4890,7 @@ Expression *NewExp::semantic(Scope *sc) TypeFunction *tf = (TypeFunction *)f->type; Type *rettype; - if (functionParameters(loc, sc, tf, NULL, newargs, f, &rettype)) + if (functionParameters(loc, sc, tf, NULL, newargs, f, &rettype, &newprefix)) return new ErrorExp(); } else @@ -4777,7 +4920,7 @@ Expression *NewExp::semantic(Scope *sc) if (!arguments) arguments = new Expressions(); - if (functionParameters(loc, sc, tf, type, arguments, f, &type)) + if (functionParameters(loc, sc, tf, type, arguments, f, &type, &argprefix)) return new ErrorExp(); } else @@ -4851,6 +4994,8 @@ Expression *NewExp::semantic(Scope *sc) //printf("NewExp:type '%s'\n", type->toChars()); semanticTypeInfo(sc, type); + if (newprefix) + return combine(newprefix, this); return this; Lerr: @@ -8750,9 +8895,10 @@ Expression *CallExp::semantic(Scope *sc) } assert(t1->ty == Tfunction); + Expression *argprefix; if (!arguments) arguments = new Expressions(); - if (functionParameters(loc, sc, (TypeFunction *)(t1), tthis, arguments, f, &type)) + if (functionParameters(loc, sc, (TypeFunction *)(t1), tthis, arguments, f, &type, &argprefix)) return new ErrorExp(); if (!type) @@ -8771,7 +8917,7 @@ Expression *CallExp::semantic(Scope *sc) if (tf->next->isBaseOf(t, &offset) && offset) { type = tf->next; - return castTo(sc, t); + return combine(argprefix, castTo(sc, t)); } } @@ -8782,7 +8928,7 @@ Expression *CallExp::semantic(Scope *sc) f->tookAddressOf = 0; } - return this; + return combine(argprefix, this); } diff --git a/src/expression.h b/src/expression.h index 0f93833eee62..a739dae8c010 100644 --- a/src/expression.h +++ b/src/expression.h @@ -554,6 +554,8 @@ class NewExp : public Expression Type *newtype; Expressions *arguments; // Array of Expression's + Expression *argprefix; // expression to be evaluated just before arguments[] + CtorDeclaration *member; // constructor function NewDeclaration *allocator; // allocator function int onstack; // allocate on stack diff --git a/src/interpret.c b/src/interpret.c index 641f62c0b009..74a6b1a619f6 100644 --- a/src/interpret.c +++ b/src/interpret.c @@ -3006,6 +3006,14 @@ class Interpreter : public Visitor #if LOG printf("%s NewExp::interpret() %s\n", e->loc.toChars(), e->toChars()); #endif + + if (e->argprefix) + { + result = e->argprefix->interpret(istate, ctfeNeedNothing); + if (exceptionOrCantInterpret(result)) + return; + } + if (e->newtype->ty == Tarray && e->arguments) { result = recursivelyCreateArrayLiteral(e->loc, e->newtype, istate, e->arguments, 0); diff --git a/src/tocsym.c b/src/tocsym.c index 47e7c55c70df..6fbc51868d92 100644 --- a/src/tocsym.c +++ b/src/tocsym.c @@ -210,7 +210,7 @@ Symbol *toSymbol(Dsymbol *s) } } - if (vd->ident == Id::va_argsave) + if (vd->ident == Id::va_argsave || vd->storage_class & STCvolatile) { /* __va_argsave is set outside of the realm of the optimizer, * so we tell the optimizer to leave it alone diff --git a/test/runnable/sdtor.d b/test/runnable/sdtor.d index b4ad4053fbcb..1defe87edc93 100644 --- a/test/runnable/sdtor.d +++ b/test/runnable/sdtor.d @@ -3315,21 +3315,21 @@ void test13089() nothrow /**********************************/ -struct NoDtortest11763 {} - -struct HasDtortest11763 -{ - NoDtortest11763 func() - { - return NoDtortest11763(); - } - ~this() {} -} - -void test11763() -{ - HasDtortest11763().func(); -} +struct NoDtortest11763 {} + +struct HasDtortest11763 +{ + NoDtortest11763 func() + { + return NoDtortest11763(); + } + ~this() {} +} + +void test11763() +{ + HasDtortest11763().func(); +} /**********************************/ @@ -3351,6 +3351,90 @@ void test13303() /**********************************/ +void test13586() +{ + static struct S { + __gshared int count; + ~this() { ++count; printf("~S\n"); } + } + + static struct T { + __gshared int count; + ~this() { ++count; printf("~T\n"); } + } + + static int foo(bool flag) + { + if (flag) + throw new Exception("hello"); + return 1; + } + + static void func(S s, int f, T t) + { + printf("func()\n"); + } + + static class C + { + this(S s, int f, T t) + { + printf("C()\n"); + } + } + + { + bool threw = false; + try + { + func(S(), foo(true), T()); + printf("not reach\n"); + } + catch (Exception e) + { + threw = true; + } + printf("threw %d S %d T %d\n", threw, S.count, T.count); + assert(threw && S.count == 1 && T.count == 0); + S.count = 0; + T.count = 0; + } + { + bool threw = false; + try + { + func(S(), foo(false), T()); + printf("reached\n"); + } + catch (Exception e) + { + threw = true; + } + printf("threw %d S %d T %d\n", threw, S.count, T.count); + assert(!threw && S.count == 1 && T.count == 1); + S.count = 0; + T.count = 0; + } + { + bool threw = false; + try + { + new C(S(), foo(true), T()); + printf("not reach\n"); + } + catch (Exception e) + { + threw = true; + } + printf("threw %d S %d T %d\n", threw, S.count, T.count); + assert(threw && S.count == 1 && T.count == 0); + S.count = 0; + T.count = 0; + } +} + +/**********************************/ + int main() { test1(); @@ -3454,6 +3538,7 @@ int main() test13089(); test11763(); test13303(); + test13586(); printf("Success\n"); return 0;