From e20c63cb28decb66c792b1dfc10d79aa0aebd7cf Mon Sep 17 00:00:00 2001 From: k-hara Date: Mon, 22 Jun 2015 00:30:33 +0900 Subject: [PATCH 1/2] fix Issue 14708 - destructor for temporary not called during stack unwinding --- src/backend/cod2.c | 14 ++++++++++-- test/runnable/sdtor.d | 50 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/backend/cod2.c b/src/backend/cod2.c index 702225baf084..6dfa60ad7375 100644 --- a/src/backend/cod2.c +++ b/src/backend/cod2.c @@ -5079,9 +5079,14 @@ code *cddctor(elem *e,regm_t *pretregs) */ usednteh |= EHcleanup; if (config.exe == EX_NT) - { usednteh |= NTEHcleanup | NTEH_try; + { + usednteh |= NTEHcleanup | NTEH_try; nteh_usevars(); } + else + { + usednteh |= EHtry; + } assert(*pretregs == 0); code cs; cs.Iop = ESCAPE | ESCdctor; @@ -5117,9 +5122,14 @@ code *cdddtor(elem *e,regm_t *pretregs) */ usednteh |= EHcleanup; if (config.exe == EX_NT) - { usednteh |= NTEHcleanup | NTEH_try; + { + usednteh |= NTEHcleanup | NTEH_try; nteh_usevars(); } + else + { + usednteh |= EHtry; + } code cs; cs.Iop = ESCAPE | ESCddtor; diff --git a/test/runnable/sdtor.d b/test/runnable/sdtor.d index 9f117452865a..1dfbd48185f0 100644 --- a/test/runnable/sdtor.d +++ b/test/runnable/sdtor.d @@ -3942,6 +3942,55 @@ void test14264() assert(dtor == 4); } +/**********************************/ +// 14708 + +bool dtor14708 = false; + +struct S14708 +{ + int n; + + void* get(void* p = null) + { + return null; + } + + ~this() + { + //printf("dtor\n"); + dtor14708 = true; + } +} + +S14708 makeS14708(int n) +{ + return S14708(n); +} + +void foo14708(void* x) +{ + throw new Exception("fail!"); +} + +void bar14708() +{ + foo14708(makeS14708(1).get()); + // A temporary is allocated on stack for the + // return value from makeS(1). + // When foo throws exception, it's dtor should be called + // during unwinding stack, but it does not happen in Win64. +} + +void test14708() +{ + try + { + bar14708(); + } catch (Exception e) {} + assert(dtor14708); // fails! +} + /**********************************/ // 14838 @@ -4111,6 +4160,7 @@ int main() test13669(); test13095(); test14264(); + test14708(); test14838(); printf("Success\n"); From 3bbe72b5fdc278fe1293c20b39af4f94b10070bb Mon Sep 17 00:00:00 2001 From: k-hara Date: Mon, 22 Jun 2015 00:30:34 +0900 Subject: [PATCH 2/2] fix Issue 14696 - destructor for temporary called before statement is complete with conditional operator --- src/e2ir.c | 4 +- src/expression.c | 84 ++++++++++++++++++++++++++++++++ src/expression.h | 1 + test/runnable/sdtor.d | 110 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 2 deletions(-) diff --git a/src/e2ir.c b/src/e2ir.c index 4a873c9e329b..6b9144cabddd 100644 --- a/src/e2ir.c +++ b/src/e2ir.c @@ -3304,12 +3304,12 @@ elem *toElem(Expression *e, IRState *irs) { elem *ec = toElem(ce->econd, irs); - elem *eleft = toElemDtor(ce->e1, irs); + elem *eleft = toElem(ce->e1, irs); tym_t ty = eleft->Ety; if (global.params.cov && ce->e1->loc.linnum) eleft = el_combine(incUsageElem(irs, ce->e1->loc), eleft); - elem *eright = toElemDtor(ce->e2, irs); + elem *eright = toElem(ce->e2, irs); if (global.params.cov && ce->e2->loc.linnum) eright = el_combine(incUsageElem(irs, ce->e2->loc), eright); diff --git a/src/expression.c b/src/expression.c index 33cf9e3d88f6..2e18e639e680 100644 --- a/src/expression.c +++ b/src/expression.c @@ -45,6 +45,7 @@ bool typeMerge(Scope *sc, TOK op, Type **pt, Expression **pe1, Expression **pe2); bool isArrayOpValid(Expression *e); Expression *expandVar(int result, VarDeclaration *v); +bool walkPostorder(Expression *e, StoppableVisitor *v); TypeTuple *toArgTypes(Type *t); bool checkAccess(AggregateDeclaration *ad, Loc loc, Scope *sc, Dsymbol *smember); bool checkFrameAccess(Loc loc, Scope *sc, AggregateDeclaration *ad, size_t istart = 0); @@ -13727,9 +13728,92 @@ Expression *CondExp::semantic(Scope *sc) printf("e1 : %s\n", e1->type->toChars()); printf("e2 : %s\n", e2->type->toChars()); #endif + + /* Bugzilla 14696: If either e1 or e2 contain temporaries which need dtor, + * make them conditional. + * Rewrite: + * cond ? (__tmp1 = ..., __tmp1) : (__tmp2 = ..., __tmp2) + * to: + * (auto __cond = cond) ? (... __tmp1) : (... __tmp2) + * and replace edtors of __tmp1 and __tmp2 with: + * __tmp1->edtor --> __cond && __tmp1.dtor() + * __tmp2->edtor --> __cond || __tmp2.dtor() + */ + hookDtors(sc); + return this; } +void CondExp::hookDtors(Scope *sc) +{ + class DtorVisitor : public StoppableVisitor + { + public: + Scope *sc; + CondExp *ce; + VarDeclaration *vcond; + bool isThen; + + DtorVisitor(Scope *sc, CondExp *ce) + { + this->sc = sc; + this->ce = ce; + this->vcond = NULL; + } + + void visit(Expression *e) + { + //printf("(e = %s)\n", e->toChars()); + } + + void visit(DeclarationExp *e) + { + VarDeclaration *v = e->declaration->isVarDeclaration(); + if (v && !v->noscope && !v->isDataseg()) + { + if (v->init) + { + ExpInitializer *ei = v->init->isExpInitializer(); + if (ei) + ei->exp->accept(this); + } + + if (v->edtor) + { + if (!vcond) + { + vcond = new VarDeclaration(ce->econd->loc, ce->econd->type, + Identifier::generateId("__cond"), new ExpInitializer(ce->econd->loc, ce->econd)); + vcond->storage_class |= STCtemp | STCctfe | STCvolatile; + vcond->semantic(sc); + + Expression *de = new DeclarationExp(ce->econd->loc, vcond); + de = de->semantic(sc); + + Expression *ve = new VarExp(ce->econd->loc, vcond); + ce->econd = Expression::combine(de, ve); + } + + //printf("\t++v = %s, v->edtor = %s\n", v->toChars(), v->edtor->toChars()); + Expression *ve = new VarExp(vcond->loc, vcond); + if (isThen) + v->edtor = new AndAndExp(v->edtor->loc, ve, v->edtor); + else + v->edtor = new OrOrExp(v->edtor->loc, ve, v->edtor); + v->edtor = v->edtor->semantic(sc); + //printf("\t--v = %s, v->edtor = %s\n", v->toChars(), v->edtor->toChars()); + } + } + } + }; + + DtorVisitor v(sc, this); + //printf("+%s\n", toChars()); + v.isThen = true; walkPostorder(e1, &v); + v.isThen = false; walkPostorder(e2, &v); + //printf("-%s\n", toChars()); +} + bool CondExp::isLvalue() { return e1->isLvalue() && e2->isLvalue(); diff --git a/src/expression.h b/src/expression.h index 4e02a1ba813d..5c9c51cfa4b4 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1475,6 +1475,7 @@ class CondExp : public BinExp Expression *toLvalue(Scope *sc, Expression *e); Expression *modifiableLvalue(Scope *sc, Expression *e); Expression *toBoolean(Scope *sc); + void hookDtors(Scope *sc); void accept(Visitor *v) { v->visit(this); } }; diff --git a/test/runnable/sdtor.d b/test/runnable/sdtor.d index 1dfbd48185f0..3044e62b9570 100644 --- a/test/runnable/sdtor.d +++ b/test/runnable/sdtor.d @@ -3942,6 +3942,115 @@ void test14264() assert(dtor == 4); } +/**********************************/ +// 14696 + +void test14696(int len = 2) +{ + string result; + + struct S + { + int n; + + void* get(void* p = null) + { + result ~= "get(" ~ cast(char)(n+'0') ~ ")."; + return null; + } + + ~this() + { + result ~= "dtor(" ~ cast(char)(n+'0') ~ ")."; + } + } + + S makeS(int n) + { + result ~= "makeS(" ~ cast(char)(n+'0') ~ ")."; + return S(n); + } + void foo(void* x, void* y = null) + { + result ~= "foo."; + } + void fooThrow(void* x, void* y = null) + { + result ~= "fooThrow."; + throw new Exception("fail!"); + } + + void check(void delegate() dg, string r, string file = __FILE__, size_t line = __LINE__) + { + import core.exception; + + result = null; + try { dg(); } catch (Exception e) {} + if (result != r) + throw new AssertError(result, file, line); + } + + // temporary in condition + check({ foo(len == 2 ? makeS(1).get() : null); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len == 2 ? null : makeS(1).get() ); }, "foo."); + check({ foo(len != 2 ? makeS(1).get() : null); }, "foo."); + check({ foo(len != 2 ? null : makeS(1).get() ); }, "makeS(1).get(1).foo.dtor(1)."); + + // temporary in nesting conditions + check({ foo(len >= 2 ? (len == 2 ? makeS(1).get() : null) : null); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len >= 2 ? (len == 2 ? null : makeS(1).get() ) : null); }, "foo."); + check({ foo(len >= 2 ? (len != 2 ? makeS(1).get() : null) : null); }, "foo."); + check({ foo(len >= 2 ? (len != 2 ? null : makeS(1).get() ) : null); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len >= 2 ? null : (len == 2 ? makeS(1).get() : null) ); }, "foo."); + check({ foo(len >= 2 ? null : (len == 2 ? null : makeS(1).get() ) ); }, "foo."); + check({ foo(len >= 2 ? null : (len != 2 ? makeS(1).get() : null) ); }, "foo."); + check({ foo(len >= 2 ? null : (len != 2 ? null : makeS(1).get() ) ); }, "foo."); + check({ foo(len > 2 ? (len == 2 ? makeS(1).get() : null) : null); }, "foo."); + check({ foo(len > 2 ? (len == 2 ? null : makeS(1).get() ) : null); }, "foo."); + check({ foo(len > 2 ? (len != 2 ? makeS(1).get() : null) : null); }, "foo."); + check({ foo(len > 2 ? (len != 2 ? null : makeS(1).get() ) : null); }, "foo."); + check({ foo(len > 2 ? null : (len == 2 ? makeS(1).get() : null) ); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len > 2 ? null : (len == 2 ? null : makeS(1).get() ) ); }, "foo."); + check({ foo(len > 2 ? null : (len != 2 ? makeS(1).get() : null) ); }, "foo."); + check({ foo(len > 2 ? null : (len != 2 ? null : makeS(1).get() ) ); }, "makeS(1).get(1).foo.dtor(1)."); + + // temporary in condition and throwing callee + check({ fooThrow(len == 2 ? makeS(1).get() : null); }, "makeS(1).get(1).fooThrow.dtor(1)."); + check({ fooThrow(len == 2 ? null : makeS(1).get() ); }, "fooThrow."); + check({ fooThrow(len != 2 ? makeS(1).get() : null); }, "fooThrow."); + check({ fooThrow(len != 2 ? null : makeS(1).get() ); }, "makeS(1).get(1).fooThrow.dtor(1)."); + + // temporary in nesting condititions and throwing callee + check({ fooThrow(len >= 2 ? (len == 2 ? makeS(1).get() : null) : null); }, "makeS(1).get(1).fooThrow.dtor(1)."); + check({ fooThrow(len >= 2 ? (len == 2 ? null : makeS(1).get() ) : null); }, "fooThrow."); + check({ fooThrow(len >= 2 ? (len != 2 ? makeS(1).get() : null) : null); }, "fooThrow."); + check({ fooThrow(len >= 2 ? (len != 2 ? null : makeS(1).get() ) : null); }, "makeS(1).get(1).fooThrow.dtor(1)."); + check({ fooThrow(len >= 2 ? null : (len == 2 ? makeS(1).get() : null) ); }, "fooThrow."); + check({ fooThrow(len >= 2 ? null : (len == 2 ? null : makeS(1).get() ) ); }, "fooThrow."); + check({ fooThrow(len >= 2 ? null : (len != 2 ? makeS(1).get() : null) ); }, "fooThrow."); + check({ fooThrow(len >= 2 ? null : (len != 2 ? null : makeS(1).get() ) ); }, "fooThrow."); + check({ fooThrow(len > 2 ? (len == 2 ? makeS(1).get() : null) : null); }, "fooThrow."); + check({ fooThrow(len > 2 ? (len == 2 ? null : makeS(1).get() ) : null); }, "fooThrow."); + check({ fooThrow(len > 2 ? (len != 2 ? makeS(1).get() : null) : null); }, "fooThrow."); + check({ fooThrow(len > 2 ? (len != 2 ? null : makeS(1).get() ) : null); }, "fooThrow."); + check({ fooThrow(len > 2 ? null : (len == 2 ? makeS(1).get() : null) ); }, "makeS(1).get(1).fooThrow.dtor(1)."); + check({ fooThrow(len > 2 ? null : (len == 2 ? null : makeS(1).get() ) ); }, "fooThrow."); + check({ fooThrow(len > 2 ? null : (len != 2 ? makeS(1).get() : null) ); }, "fooThrow."); + check({ fooThrow(len > 2 ? null : (len != 2 ? null : makeS(1).get() ) ); }, "makeS(1).get(1).fooThrow.dtor(1)."); + + // temporaries in each conditions + check({ foo(len == 2 ? makeS(1).get() : null, len == 2 ? makeS(2).get() : null); }, "makeS(1).get(1).makeS(2).get(2).foo.dtor(2).dtor(1)."); + check({ foo(len == 2 ? makeS(1).get() : null, len != 2 ? makeS(2).get() : null); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len != 2 ? makeS(1).get() : null, len == 2 ? makeS(2).get() : null); }, "makeS(2).get(2).foo.dtor(2)."); + check({ foo(len != 2 ? makeS(1).get() : null, len != 2 ? makeS(2).get() : null); }, "foo."); + + // nesting temporaries in conditions + check({ foo(len == 2 ? makeS(1).get(len == 2 ? makeS(2).get() : null) : null); }, "makeS(1).makeS(2).get(2).get(1).foo.dtor(2).dtor(1)."); + check({ foo(len == 2 ? makeS(1).get(len != 2 ? makeS(2).get() : null) : null); }, "makeS(1).get(1).foo.dtor(1)."); + check({ foo(len != 2 ? makeS(1).get(len == 2 ? makeS(2).get() : null) : null); }, "foo."); + check({ foo(len != 2 ? makeS(1).get(len != 2 ? makeS(2).get() : null) : null); }, "foo."); +} + /**********************************/ // 14708 @@ -4160,6 +4269,7 @@ int main() test13669(); test13095(); test14264(); + test14696(); test14708(); test14838();