8000 Remove strict-aliasing rules errors on gcc 4.8.5 by corona10 · Pull Request #16714 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

Remove strict-aliasing rules errors on gcc 4.8.5 #16714

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
merged 1 commit into from
Oct 11, 2019

Conversation

corona10
Copy link
Member
@corona10 corona10 commented Oct 11, 2019

Remove below error on
CentOS Linux release 7.6.1810
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36)

Objects/dictobject.c:1136:5: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
     PyDictKeyEntry *ep = &DK_ENTRIES(mp->ma_keys)[0];

@methane
Copy link
Member
methane commented Oct 11, 2019

I don't have gcc 4.8.5 environment but your patch looks good to me.

@methane methane merged commit c39d1dd into python:master Oct 11, 2019
@corona10 corona10 deleted the gcc_4_warning branch October 11, 2019 08:57
@corona10
Copy link
Member Author

@methane
Thanks for the review!

@corona10
Copy link
Member Author

@methane Can we open the backport patch for 3.8 3.7?

@methane
Copy link
Member
methane commented Oct 11, 2019

@corona10 I think this is safe to backport. But I want to wait for buildbots before backporting.

@corona10 corona10 restored the gcc_4_warning branch October 11, 2019 10:23
@corona10
Copy link
Member Author

@methane Got it!

@vstinner
Copy link
Member

"But I want to wait for buildbots before backporting."

Thank you!

@miss-islington
Copy link
Contributor

Thanks @corona10 for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @corona10 for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-16756 is a backport of this pull request to the 3.8 branch.

@miss-islington
Copy link
Contributor

Sorry, @corona10 and @methane, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker c39d1ddc012987e2159a997e27665d2d579c0ce0 3.7

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 14, 2019
(cherry picked from commit c39d1dd)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
miss-islington added a commit that referenced this pull request Oct 14, 2019
(cherry picked from commit c39d1dd)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@vstinner
Copy link
Member
Objects/dictobject.c:1136:5: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
     PyDictKeyEntry *ep = &DK_ENTRIES(mp->ma_keys)[0];

Hum, there are other places with similar code:

delitem_common():

    PyDictKeyEntry *ep;
    ...
    ep = &DK_ENTRIES(mp->ma_keys)[ix];

_PyDict_Pop_KnownHash:

    PyDictKeyEntry *ep;
    ...
    ep = &DK_ENTRIES(mp->ma_keys)[ix];

dict_equal:

        PyDictKeyEntry *ep = &DK_ENTRIES(a->ma_keys)[i];

Why only insert_to_emptydict() required to be modified?

I'm not sure that the magic DK_ENTRIES() macro respects strict aliasing.

#define DK_ENTRIES(dk) \
    ((PyDictKeyEntry*)(&((int8_t*)((dk)->dk_indices))[DK_SIZE(dk) * DK_IXSIZE(dk)]))

_dictkeysobject structure ends with:

    char dk_indices[];  /* char is required to avoid strict aliasing. */

_dictkeysobject ends with the following field, but the field is not declared in the structure:

PyDictKeyEntry dk_entries[dk_usable];

Since it's not declared, I'm not sure that the compiler is able to align it properly.

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
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.

6 participants
0