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

mingw 32-bit: realign the stack in our callbacks #21324

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Aug 3, 2023

A default 32-bit mingw build assumes the stack is 16 byte aligned, which appears to be a problem with gcc.

With quadmath enabled, libgcc includes instructions that require 16-byte alignment and access relative to the stack pointer, and when the stack isn't aligned, results in a crash.

To prevent that add the force_align_arg_pointer attribute to our callbacks for 32-bit gcc on Windows.

Fixes #21313

@sisyphus
Copy link
Contributor

sisyphus commented Aug 3, 2023

Fixes #21313

Indeed - nice work.
It works fine for me with both mcf-threads and posix-threads builds, irrespective of gcc vendor.
Thanks @tonycoz, @xenu,
I look forward to seeing this PR pushed in time for the 5.39.2 release.

There's a comment in the patch (in perl.h) that it might pose a problem for XS code that performs a callback, so I built a couple of modules of mine that do that, and found no issue.
The opening sentence of that comment seems a bit clumsy. IIUC it would be better rewritten as:
When generating code for 32-bit windows gcc assumes the stack is 16 byte aligned, though the system doesn't guarantee that.

Cheers,
Rob

Copy link
Contributor

@khwilliamson khwilliamson left a comment

Choose a reason for hiding this comment

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

Though I'm not an expert on this

Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

Though I agree with your comment - there may still be XS modules invoking callbacks and the like which will want this adding. If that becomes too many to manage we may have to think about other options.

perl.h Outdated

https://github.com/Perl/perl5/issues/21313

Where gcc when generating code for 32-bit windows assumes the stack
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiniest of nits: There's a double space in this line of comment

A default 32-bit mingw build assumes the stack is 16 byte aligned,
which appears to be a problem with gcc.

With quadmath enabled, libgcc includes instructions that require
16-byte alignment and access relative to the stack pointer, and
when the stack isn't aligned, results in a crash.

To prevent that add the force_align_arg_pointer attribute to our
callbacks for 32-bit gcc on Windows.
@tonycoz tonycoz force-pushed the 21313-gcc-i686-stack-alignment branch from 6ea6210 to 0803cb7 Compare August 6, 2023 23:56
@tonycoz
Copy link
Contributor Author

tonycoz commented Aug 7, 2023

There's a comment in the patch (in perl.h) that it might pose a problem for XS code that performs a callback, so I built a couple of modules of mine that do that, and found no issue.

It's only going to be a problem when the callback is called through something that doesn't preserve the 16-byte alignment (gcc produced code does), and then the callback does something that requires the 16 byte alignment, like comparing a __float128 as my crashing example does, or calling into perl codes that does that. Specifically SSE code could be a problem too, or code built with a sufficiently high -march=... which might be autovectorized.

I could see callback via qsort being a problem, or many Win32 API functions, but in most cases the callback is going to be fairly trivial and won't execute an of the SSE ops that cause the crash.

@tonycoz tonycoz merged commit 8133ff9 into Perl:blead Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MSWindows] 32-bit quadmath builds fail multiple tests.
4 participants