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

bodies_by_type[SVt_PVNV]: handle __float128 NV alignment on 32-bit #22609

Open
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Sep 19, 2024

On i686 systems with -msse __float128 requires 16 byte alignment, but for XPVNV bodies the hack used by new_body_allocated() to avoid allocating the unused xmg_stash and xmg_u fields means that the base of the XPVNV body ends up mis-aligned on 32-bit systems.

One 64-bit systems the combined size of those fields is 16-bytes so the modified pointer is still properly aligned.

Fixes #22577

perldelta something like the following in Selected bug fixes:

Builds with C<-msse> and quadmath on 32-bit x86 systems would crash with a misaligned access early in the build. [GH #22609]


  • This set of changes requires a perldelta entry, and it is included.
  • This set of changes requires a perldelta entry, and I need help writing it.
  • This set of changes does not require a perldelta entry.

@jkeenan
Copy link
Contributor

jkeenan commented Sep 19, 2024

@tonycoz, you've checked the box "This set of changes requires a perldelta entry, and it is included." But there are no changes to pod/perldelta.pod in the p.r. Can you clarify?

@tonycoz
Copy link
Contributor Author

tonycoz commented Sep 22, 2024

@tonycoz, you've checked the box "This set of changes requires a perldelta entry, and it is included." But there are no changes to pod/perldelta.pod in the p.r. Can you clarify?

See the last paragraph of the OP.

Perl SV body structures include xmg_stash and xmg_u fields at the
front, which are only valid for type SVt_PVMG and higher.

This allows those fields to be at a constant offset from the start
of the body.

To save memory perl generally allocates the bodies where
type < SVt_PVMG without the space needed for these two fields,
offsetting the body pointer back by the size of the two fields.  At
least for the first body in an arena this is technically
undefined behaviour, but we've done it forever.

With -msse __float128 requires 16 byte alignment, but for XPVNV
bodies the hack used here means that the base of the XPVNV
body ends up mis-aligned on 32-bit systems.

On 64-bit systems the combined size of those fields is 16-bytes so
the modified pointer is still properly aligned.

To fix this allocate the full XPVNV structure when 16 byte alignment
is required for NV, NV is more than 8 bytes and pointers are small
enough that the NV would have been mis-aligned.

Fixes Perl#22577
@tonycoz
Copy link
Contributor Author

tonycoz commented Sep 24, 2024

Significantly expanded the commit message to better explain how this stuff works and why it didn't for the failing builds.

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.

perl 5.40.0: Crash while trying to compile perl on i686 with quadmath and SSE.
2 participants