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.

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

Fix issue 16555 - Generate correct code for pushing scalar parameters. #6171

Closed
wants to merge 1 commit into from
Closed

Conversation

LemonBoy
Copy link
Contributor

@LemonBoy LemonBoy commented Oct 3, 2016

This is my try at fixing this issue, bear with me as it's my first time playing around with the dmd internals.
The snippet below shows a major codegen problem, the first 8 parameters are correctly passed into XMM0-XMM7 but for the last one a push rax is generated, which is clearly wrong.
The patch corrects this issue by exiting the huge switch and using the code that's already there to correctly push fp values and vectors (the vector path is untested, I just assumed we'd hit the same problem).
There's a further change that attempts to use the XMM registers whenever possible to avoid the using the x87 instructions, it does work fine in this case but I'm not 100% sure about this.

import std.stdio;

void main()
{
    void inner(double x)
    {
        if(x == 999) // to show that x itself isn't corrupt
        {
            writefln("x=%s a=%s b=%s c=%s d=%s e=%s f=%s g=%s h=%s", x, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0);
            //outer(x, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0);
        }
    }

    inner(999.0);
}

Cheers

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
16555 Stack corruption when calling multi-parameter outer function from nested function

@ghost
Copy link

ghost commented Oct 5, 2016

unit test needed for this one. Take alook at https://github.com/dlang/dmd/tree/master/test.

@WalterBright
Copy link
Member

This definitely needs a test case. How about adapting the example in the bug report?

@JinShil
Copy link
Contributor

JinShil commented Feb 14, 2018

Revived at #7890

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants