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 pcre.jit on Apple Silicon #9279

Closed
wants to merge 1 commit into from
Closed

Conversation

kelunik
Copy link
Member

@kelunik kelunik commented Aug 8, 2022

This backports zherczeg/sljit#105. Relates to bug 80435, however, it doesn't solve the bus error on PHP 8.0, but PHP 8.1 builds fine now.

This backports zherczeg/sljit#105. Relates to bug #80435, however, it doesn't solve the bus error on PHP 8.0, but PHP 8.1 builds fine now.
@devnexen
Copy link
Member

devnexen commented Aug 8, 2022

Might be the appropriate approach for 8.1, master had been updated to pcre 10.40 cc @cmb69

@kelunik
Copy link
Member Author

kelunik commented Aug 8, 2022

I guess PHP 8.0 also needs zherczeg/sljit#90, but it doesn't apply cleanly to the currently bundled version. Should we upgrade pcre2 there as well or just be happy with it working with jit on PHP 8.1+?

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! This patch looks good for PHP-8.1.

I wouldn't change too much regarding PHP-8.0, since active support for that version ends in a few months. So if we're fine with leaving PHP-8.0 as is, good. Otherwise, it's up for the RMs to decide what to do.

@bwoebi
Copy link
Member

bwoebi commented Aug 9, 2022

@cmb69 I think the point of active support is that such issues are fixed. Unless this is a change with high risk.

@cmb69
Copy link
Member

cmb69 commented Aug 9, 2022

Unless this is a change with high risk.

That's my point. Updating the bundled libpcre from a patched 10.35 to 10.39 or 10.40 might cause all kinds of subtle issues.

@sgolemon, @carusogabriel, thoughts about that?

@devnexen
Copy link
Member

devnexen commented Aug 9, 2022

@cmb69 I think the point of active support is that such issues are fixed. Unless this is a change with high risk.

The other alternative would be for the related os/package manager to patch the source (it s very common homebrew, freebsd pkg ... etc do that all the time)

@bwoebi
Copy link
Member

bwoebi commented Aug 9, 2022

@cmb69 Oh, you're talking about upgrading the bundled version, I thought it would be about applying some patch. Sorry for the confusion then.

@kelunik
Copy link
Member Author

kelunik commented Aug 9, 2022

@bwoebi @cmb69 We can also apply the patch in zherczeg/sljit#90 to the bundled version in 8.0, it just doesn't apply cleanly and I have no idea whether there are other patches missing.

@cmb69
Copy link
Member

cmb69 commented Aug 9, 2022

We can also apply the patch in zherczeg/sljit#90 to the bundled version in 8.0, it just doesn't apply cleanly and I have no idea whether there are other patches missing.

Me neither. And that is exactly the point; we might break other stuff, and that very late in the 8.0 release cycle.

@kelunik
Copy link
Member Author

kelunik commented Aug 30, 2022

@cmb69 So let's merge this into 8.1 and worry about 8.0 if someone wants to build it?

@cmb69 cmb69 closed this in f8b217a Aug 31, 2022
@cmb69
Copy link
Member

cmb69 commented Aug 31, 2022

So let's merge this into 8.1 and worry about 8.0 if someone wants to build it?

Yes. Thank you!

@kelunik kelunik deleted the bug-80435 branch September 17, 2022 06:39
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.

4 participants