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

Ensure proper alignment of zend_accel_shared_globals.interned_strings #15359

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

arnaud-lb
Copy link
Member

Fixes #15348

#13676 adds a assertion to check that zend_accel_shared_globals.interned_strings.start is 8 bytes aligned, which in hindsight was not guaranteed. What is guaranteed is that the size of zend_strings in the buffer is a multiple of 8.

Here I ensure that zend_accel_shared_globals.interned_strings, and thus zend_accel_shared_globals.interned_strings.start, are 8-bytes aligned.

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 solves the issue for me locally, and generally looks good to me.

Maybe we should have a nightly CI run for Windows x86 (ZTS only might be sufficient).

@cmb69
Copy link
Member

cmb69 commented Aug 12, 2024

Maybe we should have a nightly CI run for Windows x86 (ZTS only might be sufficient).

Shouldn't be hard to set up; only a couple of lines for the push workflow would need to be adapted (PR #15361). On the other hand, this issue would probably not have been detected. Maybe we should set up a debug build; maybe ASan would have found it. Maybe more local x86 builds would help. Not sure; I'll try a bit.

@arnaud-lb
Copy link
Member Author

One possible option is to build with --enable-debug-assertions

@arnaud-lb arnaud-lb marked this pull request as ready for review August 12, 2024 19:43
@arnaud-lb arnaud-lb merged commit 8115018 into php:master Aug 12, 2024
10 checks passed
arnaud-lb added a commit to arnaud-lb/php-src that referenced this pull request Aug 12, 2024
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.

Assertion violation in ZendAccelerator.c
2 participants