gh-151297: Fix undefined behavior in _PyObject_MiRealloc#151358
Conversation
The standard says that a call to `memcpy` must pass a valid source and
destination pointer even if the size is 0, so we must avoid calling
`memcpy` when our source pointer is NULL. If we don't, an optimizing
compiler can decide that the pointer must be non-NULL based on the
presence of UB, and optimize out checks for null pointers.
Specifically, note that the standard says:
Where an argument declared as size_t n specifies the length of the
array for a function, n can have the value zero on a call to that
function. Unless explicitly stated otherwise in the description of
a particular function in this subclause, pointer arguments on such
a call shall still have valid values, as described in 7.1.4.
And section 7.1.4 says:
If an argument to a function has an invalid value (such as a value
outside the domain of the function, or a pointer outside the address
space of the program, or a null pointer, or a pointer to
non-modifiable storage when the corresponding parameter is not
const-qualified) or a type (after default argument promotion) not
expected by a function with a variable number of arguments, the
behavior is undefined.
The specification for `memcpy` doesn't state that it's allowed to be
called with null pointers, and Linux's `/usr/include/string.h` declares
`memcpy` as `__nonnull ((1, 2))`.
Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
ZeroIntensity
left a comment
There was a problem hiding this comment.
Just some standard housekeeping nitpicks. The crux of the fix looks good.
I suspect we don't have a test case stressing this path. Would you mind adding one?
…NGPkUM.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
What would you like the test to look like? It's not really possible to test for a lack of UB... Or do you just mean a test for |
Yeah, that. |
|
K, I've added one |
|
Thanks @godlygeek for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15. |
|
Sorry, @godlygeek and @ZeroIntensity, I could not cleanly backport this to |
|
GH-151388 is a backport of this pull request to the 3.15 branch. |
|
Sorry, @godlygeek and @ZeroIntensity, I could not cleanly backport this to |
|
Ah, looks like this doesn't apply to 3.13 and 3.14; the UB was added in 98e55d7, which wasn't backported. |
…-151358) (GH-151388) The standard says that a call to `memcpy` must pass a valid source and destination pointer even if the size is 0, so we must avoid calling `memcpy` when our source pointer is NULL. If we don't, an optimizing compiler can decide that the pointer must be non-NULL based on the presence of UB, and optimize out checks for null pointers. Specifically, note that the standard says: Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. And section 7.1.4 says: If an argument to a function has an invalid value (such as a value outside the domain of the function, or a pointer outside the address space of the program, or a null pointer, or a pointer to non-modifiable storage when the corresponding parameter is not const-qualified) or a type (after default argument promotion) not expected by a function with a variable number of arguments, the behavior is undefined. The specification for `memcpy` doesn't state that it's allowed to be called with null pointers, and Linux's `/usr/include/string.h` declares `memcpy` as `__nonnull ((1, 2))`. (cherry picked from commit c375992) Co-authored-by: Matt Wozniski <mwozniski@bloomberg.net>
The standard says that a call to
memcpymust pass a valid source and destination pointer even if the size is 0, so we must avoid callingmemcpywhen our source pointer is NULL. If we don't, an optimizing compiler can decide that the pointer must be non-NULL based on the presence of UB, and optimize out checks for null pointers.Specifically, note that the standard says:
And section 7.1.4 says:
The specification for
memcpydoesn't state that it's allowed to be called with null pointers, and Linux's/usr/include/string.hdeclaresmemcpyas__nonnull ((1, 2)).