Skip to content
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

Jump to bottom

[REG2.069] Issue 15524 - 64bit app with anon-class crashes in contract #5420

Merged
merged 1 commit into from Feb 23, 2016
Merged

[REG2.069] Issue 15524 - 64bit app with anon-class crashes in contract #5420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/e2ir.c
View file
Open in desktop
Expand Up @@ -994,8 +994,7 @@ elem *toElem(Expression *e, IRState *irs)
* contract, instead look at the original stack data.
*/
bool forceStackAccess = false;
if (s->Sclass == SCparameter &&
fd->isVirtual() && (fd->fdrequire || fd->fdensure))
if (fd->isVirtual() && (fd->fdrequire || fd->fdensure))
{
Dsymbol *sx = irs->getFunc();
while (sx != fd)
Expand Down Expand Up @@ -1035,7 +1034,7 @@ elem *toElem(Expression *e, IRState *irs)
e = el_bin(OPadd, TYnptr, ethis, el_long(TYnptr, soffset));
if (se->op == TOKvar)
e = el_una(OPind, TYnptr, e);
if (ISREF(se->var, tb) && !(ISWIN64REF(se->var) && v && v->offset))
if (ISREF(se->var, tb) && !(ISWIN64REF(se->var) && v && v->offset && !forceStackAccess))
e = el_una(OPind, s->ty(), e);
else if (se->op == TOKsymoff && nrvo)
{
Expand Down
3 changes: 2 additions & 1 deletion src/toir.c
View file
Open in desktop
Expand Up @@ -671,7 +671,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 Down Expand Up @@ -711,6 +711,7 @@ 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.
*/
Expand Down
135 changes: 135 additions & 0 deletions test/runnable/testcontracts.d
View file
Open in desktop
Expand Up @@ -697,6 +697,139 @@ void test9383()
a.bar3(103); assert(A9383.val == 103); // closure [parameter]
}

/*******************************************/
// 15524 - Different from issue 9383 cases, closed variable size is bigger than REGSIZE.

class A15524
{
static void delegate() dg;
static string val;

void failInBase() { assert(typeid(this) is typeid(A15524)); }

// in-contract tests
void foo1(string s) in { A15524.val = s; failInBase; } body { } // no closure
void foo2(string s) in { A15524.val = s; failInBase; } body { string x; dg = { x = null; }; } // closure [local]
void foo3(string s) in { A15524.val = s; failInBase; } body { dg = { s = null; }; } // closure [parameter]
void foo4(string s) in { A15524.val = s; failInBase; } body { } // no closure
void foo5(string s) in { A15524.val = s; failInBase; } body { } // no closure
void foo6(string s) in { A15524.val = s; failInBase; } body { string x; dg = { x = null; }; } // closure [local]
void foo7(string s) in { A15524.val = s; failInBase; } body { dg = { s = null; }; } // closure [parameter]

// out-contract tests
void bar1(string s) out { A15524.val = s; } body { } // no closure
void bar2(string s) out { A15524.val = s; } body { string x; dg = { x = null; }; } // closure [local]
void bar3(string s) out { A15524.val = s; } body { dg = { s = null; }; } // closure [parameter]
void bar4(string s) out { A15524.val = s; } body { } // no closure
void bar5(string s) out { A15524.val = s; } body { } // no closure
void bar6(string s) out { A15524.val = s; } body { string x; dg = { x = null; }; } // closure [local]
void bar7(string s) out { A15524.val = s; } body { dg = { s = null; }; } // closure [parameter]
}

class B15524 : A15524
{
static string val;

// in-contract tests
override void foo1(string s) in { B15524.val = s; } body { } // -> no closure
override void foo2(string s) in { B15524.val = s; } body { string x; dg = { x = null; }; } // -> closure [local] appears
override void foo3(string s) in { B15524.val = s; } body { dg = { s = null; }; } // -> closure [parameter]
override void foo4(string s) in { B15524.val = s; } body { string x; dg = { x = null; }; } // -> closure [local] appears
override void foo5(string s) in { B15524.val = s; } body { dg = { s = null; }; } // -> closure [parameter] appears
override void foo6(string s) in { B15524.val = s; } body { } // -> closure [local] disappears
override void foo7(string s) in { B15524.val = s; } body { } // -> closure [parameter] disappears

// out-contract tests
override void bar1(string s) out { B15524.val = s; } body { } // -> no closure
override void bar2(string s) out { B15524.val = s; } body { string x; dg = { x = null; }; } // -> closure [local] appears
override void bar3(string s) out { B15524.val = s; } body { dg = { s = null; }; } // -> closure [parameter]
override void bar4(string s) out { B15524.val = s; } body { string x; dg = { x = null; }; } // -> closure [local] appears
override void bar5(string s) out { B15524.val = s; } body { dg = { s = null; }; } // -> closure [parameter] appears
override void bar6(string s) out { B15524.val = s; } body { } // -> closure [local] disappears
override void bar7(string s) out { B15524.val = s; } body { } // -> closure [parameter] disappears
}

void test15524()
{
auto a = new A15524();
auto b = new B15524();

// base class in-contract is used from derived class. // base derived
b.foo1("1"); assert(A15524.val == "1" && B15524.val == "1"); // no closure -> no closure
b.foo2("2"); assert(A15524.val == "2" && B15524.val == "2"); // closure [local] -> closure [local] appears
b.foo3("3"); assert(A15524.val == "3" && B15524.val == "3"); // closure [parameter] -> closure [parameter]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests look good, however the change doesn't pass this assert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that currently all 64bit platforms has problems with the issue 15524 case - closure allocation + contracts with virtual class methods. This PR works for Win64, but I think other Posix platforms need other fix.

But, I'm mainly working on Windows platform. So I'll pending this PR till finishing other possible regression fixes done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I fixed the problem in 64bit posix platforms. Ready to go.

b.foo4("4"); assert(A15524.val == "4" && B15524.val == "4"); // no closure -> closure [local] appears
b.foo5("5"); assert(A15524.val == "5" && B15524.val == "5"); // no closure -> closure [parameter] appears
b.foo6("6"); assert(A15524.val == "6" && B15524.val == "6"); // closure [local] -> closure [local] disappears
b.foo7("7"); assert(A15524.val == "7" && B15524.val == "7"); // closure [parameter] -> closure [parameter] disappears

// base class out-contract is used from derived class. // base derived
b.bar1("1"); assert(A15524.val == "1" && B15524.val == "1"); // no closure -> no closure
b.bar2("2"); assert(A15524.val == "2" && B15524.val == "2"); // closure [local] -> closure [local] appears
b.bar3("3"); assert(A15524.val == "3" && B15524.val == "3"); // closure [parameter] -> closure [parameter]
b.bar4("4"); assert(A15524.val == "4" && B15524.val == "4"); // no closure -> closure [local] appears
b.bar5("5"); assert(A15524.val == "5" && B15524.val == "5"); // no closure -> closure [parameter] appears
b.bar6("6"); assert(A15524.val == "6" && B15524.val == "6"); // closure [local] -> closure [local] disappears
b.bar7("7"); assert(A15524.val == "7" && B15524.val == "7"); // closure [parameter] -> closure [parameter] disappears

// in-contract in base class.
a.foo1("1"); assert(A15524.val == "1"); // no closure
a.foo2("2"); assert(A15524.val == "2"); // closure [local]
a.foo3("3"); assert(A15524.val == "3"); // closure [parameter]

// out-contract in base class.
a.bar1("1"); assert(A15524.val == "1"); // no closure
a.bar2("2"); assert(A15524.val == "2"); // closure [local]
a.bar3("3"); assert(A15524.val == "3"); // closure [parameter]
}

void test15524a()
{
auto t1 = new Test15524a();
t1.infos["first"] = 0; //t1.add("first");
t1.add("second");

auto t2 = new Test15524b();
t2.add("second");
}

class Test15524a
{
int[string] infos;

void add(string key)
in
{
assert(key !in infos); // @@@ crash here at second
}
body
{
auto item = new class
{
void notCalled()
{
infos[key] = 0;
// affects, key parameter is made a closure variable.
}
};
}
}

class Test15524b
{
void add(string key)
in
{
assert(key == "second"); // @@@ fails
}
body
{
static void delegate() dg;
dg = { auto x = key; };
// affects, key parameter is made a closure variable.
}
}

/*******************************************/
// 10479

Expand Down Expand Up @@ -791,6 +924,8 @@ int main()
test8073();
test8093();
test9383();
test15524();
test15524a();
test14779();

printf("Success\n");
Expand Down