Skip to content
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.

Sign up for GitHub

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

Jump to bottom

gh-89653: PEP 670: Fix PyUnicode_READ() cast #92872

Merged
merged 2 commits into from
May 17, 2022
Merged

gh-89653: PEP 670: Fix PyUnicode_READ() cast #92872

merged 2 commits into from
May 17, 2022

Conversation

vstinner
Copy link
Member

_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.

@vstinner vstinner mentioned this pull request May 17, 2022
Closed
@vstinner
Copy link
Member Author

See bug report: #92800

@vstinner
Copy link
Member Author

cc @erlend-aasland @da-woods

@da-woods
Copy link
Contributor

I'll have to wait until later to test and confirm that it's fixed the Cython bug.

Is it worth copying the test-case from #92818? (The actual fix here is probably more sensible though)

@vstinner
Copy link
Member Author

Is it worth copying the test-case from #92818? (The actual fix here is probably more sensible though)

Oh right, I added tests.

@vstinner
gh-89653: PEP 670: Fix PyUnicode_READ() cast
75e5da7 
_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.
@vstinner
Copy link
Member Author

I rebased the PR and I added more tests about the PyUnicode_READ() cast.

kumaraditya303
kumaraditya303 reviewed May 17, 2022
View reviewed changes
Lib/test/_testcppext.cpp Outdated
{
PyObject *str = PyUnicode_FromString("abc");
if (str == NULL) {
return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return NULL;
return nullptr;

@tacaswell
Copy link
Contributor

I can confirm it fixes the cython bug.

@tacaswell
Copy link
Contributor

However on this branch greenlets is still broken:

$ gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding/include/python3.12 -c src/greenlet/gre
enlet.cpp -o build/temp.linux-x86_64-3.12/src/greenlet/greenlet.o
In file included from /home/tcaswell/.pybuild/bleeding/include/python3.12/Python.h:38,
                 from src/greenlet/greenlet.cpp:14:
src/greenlet/greenlet_refs.hpp: In member function ‘void greenlet::refs::PyErrPieces::normalize()’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:781:41: note: in expansion of macro ‘_PyObject_CAST’
  781 | #  define PyType_Check(op) PyType_Check(_PyObject_CAST(op))
      |                                         ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyerrors.h:57:6: note: in expansion of macro ‘PyType_Check’
   57 |     (PyType_Check((x)) &&                                               \
      |      ^~~~~~~~~~~~
src/greenlet/greenlet_refs.hpp:993:17: note: in expansion of macro ‘PyExceptionClass_Check’
  993 |             if (PyExceptionClass_Check(type)) {
      |                 ^~~~~~~~~~~~~~~~~~~~~~
In file included from /home/tcaswell/.pybuild/bleeding/include/python3.12/Python.h:44:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:775:59: note: in definition of macro ‘PyType_FastSubclass’
  775 | #define PyType_FastSubclass(type, flag) PyType_HasFeature(type, flag)
      |                                                           ^~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’
  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
      |                               ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyerrors.h:61:25: note: in expansion of macro ‘Py_TYPE’
   61 |     PyType_FastSubclass(Py_TYPE(x), Py_TPFLAGS_BASE_EXC_SUBCLASS)
      |                         ^~~~~~~
src/greenlet/greenlet_refs.hpp:1003:22: note: in expansion of macro ‘PyExceptionInstance_Check’
 1003 |             else if (PyExceptionInstance_Check(type)) {
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp: In destructor ‘greenlet::ThreadState::~ThreadState()’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:266:59: note: in expansion of macro ‘_PyObject_CAST’
  266 | #  define PyObject_TypeCheck(ob, type) PyObject_TypeCheck(_PyObject_CAST(ob), type)
      |                                                           ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/methodobject.h:17:31: note: in expansion of macro ‘PyObject_TypeCheck’
   17 | #define PyCFunction_Check(op) PyObject_TypeCheck(op, &PyCFunction_Type)
      |                               ^~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:400:33: note: in expansion of macro ‘PyCFunction_Check’
  400 |                              && PyCFunction_Check(refs.at(0))
      |                                 ^~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:127:35: note: in expansion of macro ‘_PyObject_CAST’
  127 | #  define Py_REFCNT(ob) Py_REFCNT(_PyObject_CAST(ob))
      |                                   ^~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:401:33: note: in expansion of macro ‘Py_REFCNT’
  401 |                              && Py_REFCNT(refs.at(0)) == 2) {
      |                                 ^~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:581:29: note: in expansion of macro ‘_PyObject_CAST’
  581 |         PyObject *_py_tmp = _PyObject_CAST(op); \
      |                             ^~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:422:33: note: in expansion of macro ‘Py_CLEAR’
  422 |                                 Py_CLEAR(function_w);
      |                                 ^~~~~~~~
src/greenlet/greenlet.cpp: In function ‘PyObject* green_repr(greenlet::refs::BorrowedGreenlet)’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedGreenlet’ {aka ‘greenlet::refs::_BorrowedGreenlet<_greenlet, greenlet::refs::GreenletChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’
  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
      |                               ^~~~~~~~~~~~~~
src/greenlet/greenlet.cpp:2395:33: note: in expansion of macro ‘Py_TYPE’
 2395 |     const char* const tp_name = Py_TYPE(self)->tp_name;
      |                                 ^~~~~~~

@vstinner
Copy link
Member Author

However on this branch greenlets is still broken:

If you consider that it's a regression, please open a separated issue. It seems to be a different warning/error.

@vstinner vstinner merged commit e6fd799 into python:main May 17, 2022
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@vstinner vstinner deleted the unicode_read_cast branch May 17, 2022 17:20
@bedevere-bot
Copy link

GH-92884 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 17, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 17, 2022
@vstinner @miss-islington
pythongh-89653: PEP 670: Fix PyUnicode_READ() cast (pythonGH-92872)
60e987f 
_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.
(cherry picked from commit e6fd799)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request May 17, 2022
@miss-islington @vstinner
gh-89653: PEP 670: Fix PyUnicode_READ() cast (GH-92872)
1df4550 
_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.
(cherry picked from commit e6fd799)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vstinner @da-woods @tacaswell @miss-islington @bedevere-bot @kumaraditya303