Skip to content

[3.7] bpo-27987: align PyGC_Head to alignof(long double) (GH-13335) #13581

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

Merged

Conversation

methane
Copy link
Member

@methane methane commented May 26, 2019

Reverts #13569

#13318 fixed the issue happened by merging #13335.

https://bugs.python.org/issue27987

@methane methane changed the title Revert "[3.7] bpo-27987: Revert "align PyGC_Head to alignof(long double)" (GH-13335)" bpo-27987: align PyGC_Head to alignof(long double) (GH-13335) May 26, 2019
@methane methane requested a review from gpshead May 26, 2019 06:22
@gpshead gpshead self-assigned this May 26, 2019
@gpshead gpshead changed the title bpo-27987: align PyGC_Head to alignof(long double) (GH-13335) [3.7] bpo-27987: align PyGC_Head to alignof(long double) (GH-13335) May 26, 2019
@bedevere-bot bedevere-bot added the type-bug An unexpected behavior, bug, or error label May 26, 2019
@gpshead
Copy link
Member

gpshead commented May 26, 2019

I'm going to test this out with PGO and ubsan builds first. I agree that the obmalloc alignment leading to the ubsan issues is probably fixed by #13318 at this point.

What I'm missing is why this is even necessary at this point given the obmalloc alignment change? What does this change actually cause to happen?

@methane
Copy link
Member Author

methane commented May 27, 2019

What I'm missing is why this is even necessary at this point given the obmalloc alignment change? What does this change actually cause to happen?

Alignment is all or nothing. If this is not aligned, no need to align pointer returned by obmalloc.
See this figure.

alignment

#13318 make pointer PyObject_Malloc returned (a) aligned to 16 byte. Now PyGC_Head *gc is aligned to 16 byte.

This pull request make sizeof(PyGC_Head) (b) aligned to 16 byte. Now PyObject *o = (PyObject*)(gc+1) is aligned to 16 byte.

C compiler align (c) part automatically. Now member of PyObject is aligned.

@gpshead
Copy link
Member

gpshead commented Jun 3, 2019

thanks, that was helpful :) also, testing this on an ubsan build it shows a notable improvement. the two changes together work.

@gpshead gpshead merged commit 8766cb7 into 3.7 Jun 3, 2019
@gpshead gpshead deleted the revert-13569-undo-ea2b76bdc5f97f49701213d105b8ec2387ea2fa5 branch June 3, 2019 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants