Skip to content

bpo-9566: Fix compiler warnings in peephole.c #11011

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jkloth
Copy link
Contributor

@jkloth jkloth commented Dec 7, 2018

Fixes compiler warnings in peephole.c when compiled for 64-bit.

https://bugs.python.org/issue9566

@serhiy-storchaka
Copy link
Member

This is a duplicate of #10652.

@vstinner
Copy link
Member

vstinner commented Dec 7, 2018

@jkloth: Would you be ok to close this PR in favor in mine, PR #10652?

@@ -135,6 +135,13 @@ fold_tuple_on_constants(_Py_CODEUNIT *codestr, Py_ssize_t codelen,
/* Pre-conditions */
assert(PyList_CheckExact(consts));

#if SIZEOF_SIZE_T > 4
if (PyList_GET_SIZE(consts) >= UINT_MAX-1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My PR uses "if ((size_t)index >= UINT_MAX - 1) {" which shouldn't emit a compiler warning on 32-bit size_t.

#if SIZEOF_SIZE_T > 4
/* only replace if it is not too far away */
if (oparg > UINT_MAX) {
h = -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot overflow in practice. My PR uses an assertion instead, but also makes sure that the code is smaller than INT_MAX at PyCode_Optimize() entry point (sanity check, it should never occur in pratice, see my comment in my PR ;-)).

eval loop's oparg (unsigned int), no rewritting
can be done. */
#if SIZEOF_SIZE_T > 4
if (oparg > UINT_MAX)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It cannot overflow in practice, again, see my PR.

@@ -429,7 +457,7 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names,
for (i = 0, nops = 0; i < codelen; i++) {
assert(i - nops <= INT_MAX);
/* original code offset => new code offset */
blocks[i] = i - nops;
blocks[i] = (int)(i - nops);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocks uses unsigned int, not signed int.

@bedevere-bot
Copy link

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@vstinner
Copy link
Member

vstinner commented Dec 7, 2018

This PR has multiple issues that I already fixed in my PR. So I close this one, sorry @jkloth. I prefer to focus on a single PR :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants