[3.6] bpo-33312: Fix clang ubsan out of bounds warnings in dict. (GH-6537) #6544
Conversation
…6537) Fix clang ubsan (undefined behavior sanitizer) warnings in dictobject.c by adjusting how the internal struct _dictkeysobject shared keys structure is declared. This remains ABI compatible. We get rid of the union at the end of the struct being used for conveinence to avoid typecasting in favor of char[] variable length array at the end of a struct. This is known to clang to be used for variable sized objects and will not cause an undefined behavior problem. Similarly, char arrays do not have strict aliasing undefined behavior when cast. PEP-007 does not currently list variable length arrays (VLAs) as allowed in our subset of C99. If this turns out to be a problem, the fix to this is to change the char `dk_indices[]` into `dk_indices[1]` and restore the three size computation subtractions this change removes: `- Py_MEMBER_SIZE(PyDictKeysObject, dk_indices)` If this works as is I'll make a separate PR to update PEP-007. (cherry picked from commit 397f1b2) Co-authored-by: Gregory P. Smith <[email protected]>
|
@gpshead: Backport status check is done, and it's a success ✅ . |
gpshead
left a comment
gpshead
left a comment
There was a problem hiding this comment.
I need to merge https://github.com/python/cpython/pull/6549/files into this PR before it goes in.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Backport python@11659d0 into this change instead of leaving it a separate followup change.
gpshead
left a comment
gpshead
left a comment
There was a problem hiding this comment.
even after tests pass, i'm going to let this sit for a while watching for any unexpected fallout on the master and 3.7 sides.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be poked with soft cushions! |
|
@gpshead: What's the status of this backport? |
|
Still desired but buried on my todo list. |
|
@gpshead: ping again, 2 months later. |
zware
changed the title
zware
changed the title
5 participants
Fix clang ubsan (undefined behavior sanitizer) warnings in dictobject.c by
adjusting how the internal struct _dictkeysobject shared keys structure is
declared.
This remains ABI compatible. We get rid of the union at the end of the
struct being used for conveinence to avoid typecasting in favor of char[]
variable length array at the end of a struct. This is known to clang to be
used for variable sized objects and will not cause an undefined behavior
problem. Similarly, char arrays do not have strict aliasing undefined
behavior when cast.
PEP-007 does not currently list variable length arrays (VLAs) as allowed
in our subset of C99. If this turns out to be a problem, the fix to this is
to change the char
dk_indices[]intodk_indices[1]and restore thethree size computation subtractions this change removes:
- Py_MEMBER_SIZE(PyDictKeysObject, dk_indices)If this works as is I'll make a separate PR to update PEP-007.
(cherry picked from commit 397f1b2)
Co-authored-by: Gregory P. Smith [email protected]
https://bugs.python.org/issue33312