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

fix issue 19497 - the program crash using dmd with -O, it works fine … #9113

Merged
merged 2 commits into from Dec 29, 2018
Merged

fix issue 19497 - the program crash using dmd with -O, it works fine … #9113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ede66c
fix issue 19497 - the program crash using dmd with -O, it works fine …
aG0aep6G Dec 21, 2018
9d31e0d
fix another `getlvalue` call, remove workaround
aG0aep6G Dec 25, 2018
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
6 changes: 2 additions & 4 deletions src/dmd/backend/cod4.d
View file
Open in desktop
Expand Up @@ -443,7 +443,7 @@ void cdeq(ref CodeBuilder cdb,elem *e,regm_t *pretregs)
postinc = e11.EV.E2.EV.Vint;
if (e11.Eoper == OPpostdec)
postinc = -postinc;
getlvalue(cdb,&cs,e11,RMstore);
getlvalue(cdb,&cs,e1,RMstore);
Copy link
Member

Choose a reason for hiding this comment

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

dmd/src/dmd/backend/cod4.d

Line 713 in 2ede66c

getlvalue(cdb,&cs,e11,RMstore | retregs);
and
https://github.com/dlang/dmd/blob/master/src/dmd/backend/cgxmm.d#L257
have more or less the same code, should they be changed, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first one seems to have a workaround following it:

dmd/src/dmd/backend/cod4.d

Lines 715 to 716 in 5cb179f

if (I64 && sz < 8)
cs.Irex &= ~REX_W; // incorrectly set by getlvalue()

Blame leads to this commit: 521e68c. Unfortunately, there's hardly a hint as to what issue was fixed there. It looks like removing the workaround, and fixing the getlvalue call instead, could work. But maybe we should first find/add a test that breaks without the workaround.

Regarding the one in cgxmm.d, I got nothing.

Copy link
Member

Choose a reason for hiding this comment

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

The first one seems to have a workaround following it:

This code triggers the workaround (Win64):

// REQUIRES: -O

int toStringz(string s) { return s.length > 0 ? s[0] : 0; }

private void toAStringz(in string[] a, int*az)
{
    foreach (string s; a)
    {
        *az++ = toStringz(s);
    }
}

void main()
{
    string[1] sa = [ "abc" ];
    int[2] tgt = 0x2a;
    toAStringz(sa[], tgt.ptr);
    assert(tgt[0] == 'a');
    assert(tgt[1] == 0x2a);
}

It also passes if e11 is replaced by e1 instead of the workaround.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding the one in cgxmm.d, I got nothing.

It is hit by the same code when using float, double or __vector(T) instead of int, but doesn't seem to be affected by the issue.

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've added a commit that removes the workaround and adds your test.

freenode(e11.EV.E2);
}
else
Expand Down Expand Up @@ -710,10 +710,8 @@ void cdeq(ref CodeBuilder cdb,elem *e,regm_t *pretregs)
postinc = e11.EV.E2.EV.Vint;
if (e11.Eoper == OPpostdec)
postinc = -postinc;
getlvalue(cdb,&cs,e11,RMstore | retregs);
getlvalue(cdb,&cs,e1,RMstore | retregs);
freenode(e11.EV.E2);
if (I64 && sz < 8)
cs.Irex &= ~REX_W; // incorrectly set by getlvalue()
}
else
{
Expand Down
39 changes: 39 additions & 0 deletions test/runnable/mars1.d
View file
Open in desktop
Expand Up @@ -1838,6 +1838,44 @@ void test18730() // https://issues.dlang.org/show_bug.cgi?id=18730

////////////////////////////////////////////////////////////////////////

void test19497() // https://issues.dlang.org/show_bug.cgi?id=19497
{
{
ubyte[1024] data;
ushort* ushortPtr = cast(ushort*) data.ptr;
*ushortPtr++ = 0xfe00;
printf("ushortPtr(%p)\n", ushortPtr);
fflush(stdout);
}

alias Seq(stuff ...) = stuff;
static foreach (T; Seq!(ubyte, ushort, uint, ulong, byte, short, int, long))
{{
T[2] data = 0x2A;
T* q = &data[0];
*q++ = cast(T) 0x1122334455667788;
if (*q != 0x2A) assert(false);
}}

{
static int toStringz(string s) { return s.length > 0 ? s[0] : 0; }
static void toAStringz(in string[] a, int* az)
{
foreach (string s; a)
{
*az++ = toStringz(s);
}
}
string[1] sa = ["abc"];
int[2] tgt = 0x2a;
toAStringz(sa[], tgt.ptr);
if (tgt[0] != 'a') assert(false);
if (tgt[1] != 0x2a) assert(false);
}
}

////////////////////////////////////////////////////////////////////////

int main()
{
testgoto();
Expand Down Expand Up @@ -1903,6 +1941,7 @@ int main()
test18315();
test18461();
test18730();
test19497();

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