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
bpo-29683 - Fixes to _PyCode_SetExtra when co_extra->ce->extras is allocated #376
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
@brianfcoleman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @brettcannon, @avassalotti, @gvanrossum, @1st1 and @vadmium to be potential reviewers. |
@@ -885,14 +887,17 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) | |||
co_extra->ce_extras, tstate->co_extra_user_count * sizeof(void*)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously allocated co_extra->ce_extras
is leaked if PyMem_Realloc()
returns NULL
. It would be better to save the result of PyMem_Realloc()
to the variable and check it for NULL
first than assign it to co_extra->ce_extras
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made this change
Objects/codeobject.c
Outdated
@@ -868,6 +868,8 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) | |||
} | |||
co_extra = (_PyCodeObjectExtra *) o->co_extra; | |||
|
|||
co_extra->ce_size = 0; | |||
|
|||
co_extra->ce_extras = PyMem_Malloc( | |||
tstate->co_extra_user_count * sizeof(void*)); | |||
if (co_extra->ce_extras == NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would not be better to deallocate o->co_extra
and set it to NULL
rather of setting co_extra->ce_size
to 0
?
return -1; | ||
} | ||
|
||
co_extra->ce_size = tstate->co_extra_user_count; | ||
|
||
for (Py_ssize_t i = co_extra->ce_size; i < co_extra->ce_size; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
@brianfcoleman, please sign the CLA and add an entry in Misc/NEWS. |
4aed1c0
to
45c3243
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except minor details of Misc/NEWS entry.
Misc/NEWS
Outdated
@@ -32,6 +32,9 @@ Core and Builtins | |||
anymore. It affects ``co_firstlineno`` and ``co_lnotab`` of code object | |||
for module and class. | |||
|
|||
- bpo-29683: Fixes to memory allocation in _PyCode_SetExtra. Patch by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New entries usually added are added at the start of corresponding section, so that entries are list in reversed chronological order.
It is recommended to use two spaces after sentence-ending period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made this change
allocated. On PyMem_Realloc failure, _PyCode_SetExtra should free co_extra if co_extra->ce_extras could not be allocated. On PyMem_Realloc success, _PyCode_SetExtra should set all unused slots in co_extra->ce_extras to NULL.
Thanks @brianfcoleman for your contribution! Would you mind to backport the change to Python 3.6? |
Bumps [codecov](https://github.com/codecov/codecov-python) from 2.1.7 to 2.1.9. - [Release notes](https://github.com/codecov/codecov-python/releases) - [Changelog](https://github.com/codecov/codecov-python/blob/master/CHANGELOG.md) - [Commits](codecov/codecov-python@v2.1.7...v2.1.9) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Mariatta <Mariatta@users.noreply.github.com>
On PyMem_Malloc failure, _PyCode_SetExtra should set co_extra->ce_size = 0.
On PyMem_Realloc failure, _PyCode_SetExtra should set co_extra->ce_size = 0.
On PyMem_Realloc success, _PyCode_SetExtra should set all unused slots in co_extra->ce_extras to NULL.