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.

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

pyatomic_gcc.h with GCC discards const qualifier #120593

Closed
ndparker opened this issue Jun 16, 2024 · 6 comments
Closed

pyatomic_gcc.h with GCC discards const qualifier #120593

ndparker opened this issue Jun 16, 2024 · 6 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@ndparker
Copy link
Contributor

ndparker commented Jun 16, 2024

Bug report

Bug description:

Hi,

including Python.h with gcc and -Wcast-qual raises the following issue (Regression from 3.12):

    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:300:34: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
      300 | { return (void *)__atomic_load_n((void **)obj, __ATOMIC_SEQ_CST); }
          |                                  ^
    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr_relaxed':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:359:34: error: cast discards 'const' qualifier from pointer target xt (line 8))
$ python3.13 -V
Python 3.13.0b1

This is a follow-up to #120293

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

@ndparker ndparker added the type-bug An unexpected behavior, bug, or error label Jun 16, 2024
@picnixz
Copy link
Member

picnixz commented Jun 17, 2024

For the record, this is not the only place where const qualifiers are lost. If you compile cpython with make CFLAGS=-Wcast-qual -j12 you end up with a lot of warnings. For the line 359, the fix would be:

// pyatomic_gcc.h
static inline void *
_Py_atomic_load_ptr_relaxed(const void *obj)
-{ return (void *)__atomic_load_n((void **)obj, __ATOMIC_RELAXED); }
+{ return (void *)__atomic_load_n((void * const *)obj, __ATOMIC_RELAXED); }

As you can see you lose constness twice, namely when calling __atomic_load_n and when returning the type. Note that we have

// pyatomic_std.h
static inline void *
_Py_atomic_load_ptr_relaxed(const void *obj)
{
    _Py_USING_STD;
    return atomic_load_explicit((const _Atomic(void*)*)obj,
                                memory_order_relaxed);
}

As you can see, the cast here is essentially using a const void **, but still the return type should be a const as well (in the case of __atomic_load_n, the signature is type __atomic_load_n(type *ptr, int memorder);).

EDIT: I edited my comment in order to reflect the real signature of _Py_atomic_load_ptr_relaxed in cpython. The return type is a void * and not a const void * but the inner cast fix remains correct.

@ndparker
Copy link
Contributor Author

Yes, I'd like to clarify that this is specifically not about compiling CPython, it's about including Python.h only.

@picnixz
Copy link
Member

picnixz commented Jun 18, 2024

Yes, I understood that from the other issue but since you only mentioned 2 lines, I wanted to 1) give a way to fix it 2) say that it's not just 2 lines that are affected but a lot of calls.

@vstinner vstinner changed the title Python.h with GCC discards const qualifier pyatomic_gcc.h with GCC discards const qualifier Jun 26, 2024
vstinner added a commit to vstinner/cpython that referenced this issue Jun 26, 2024
Remove the 'const' qualifier of the argument of functions:

* _Py_atomic_load_ptr()
* _Py_atomic_load_ptr_relaxed()
* _Py_atomic_load_ptr_acquire()
vstinner added a commit to vstinner/cpython that referenced this issue Jun 26, 2024
Remove the const qualifier of the argument of functions:

* _PyLong_IsCompact()
* _PyLong_CompactValue()

Py_TYPE() argument is not const.

Fix the compiler warning:

  Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
  Include/pyport.h:19:31: error: cast discards ‘const’ qualifier from
  pointer target type [-Werror=cast-qual]
    (...)
  Include/cpython/longintrepr.h:133:30: note: in expansion of macro
  ‘Py_TYPE’
    assert(PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_LONG_SUBCLASS));
@vstinner vstinner added needs backport to 3.13 bugs and security fixes and removed needs backport to 3.13 bugs and security fixes labels Jun 26, 2024
vstinner added a commit to vstinner/cpython that referenced this issue Jun 26, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 26, 2024
(cherry picked from commit 9cd2dcb)

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner added a commit that referenced this issue Jun 26, 2024
Remove the const qualifier of the argument of functions:

* _PyLong_IsCompact()
* _PyLong_CompactValue()

Py_TYPE() argument is not const.

Fix the compiler warning:

  Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
  Include/pyport.h:19:31: error: cast discards ‘const’ qualifier from
  pointer target type [-Werror=cast-qual]
    (...)
  Include/cpython/longintrepr.h:133:30: note: in expansion of macro
  ‘Py_TYPE’
    assert(PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_LONG_SUBCLASS));
vstinner added a commit that referenced this issue Jun 26, 2024
)

gh-120593: Fix const qualifier in pyatomic.h (GH-121055)
(cherry picked from commit 9cd2dcb)

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner added a commit to vstinner/cpython that referenced this issue Jun 27, 2024
vstinner added a commit to vstinner/cpython that referenced this issue Jun 27, 2024
Check the usage of the 'const' qualifier in the Python C API in
test_cext.
vstinner added a commit that referenced this issue Jun 27, 2024
Check the usage of the 'const' qualifier in the Python C API in
test_cext.
@vstinner
Copy link
Member

The warnings were fixed by 9cd2dcb and e51e880.

I also added a check in test_cext for check for non-regression: b7a95df.

Thanks @ndparker for your bug report. It's now fixed, I close the issue.

mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…n#121053)

Remove the const qualifier of the argument of functions:

* _PyLong_IsCompact()
* _PyLong_CompactValue()

Py_TYPE() argument is not const.

Fix the compiler warning:

  Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
  Include/pyport.h:19:31: error: cast discards ‘const’ qualifier from
  pointer target type [-Werror=cast-qual]
    (...)
  Include/cpython/longintrepr.h:133:30: note: in expansion of macro
  ‘Py_TYPE’
    assert(PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_LONG_SUBCLASS));
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
Check the usage of the 'const' qualifier in the Python C API in
test_cext.
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…n#121053)

Remove the const qualifier of the argument of functions:

* _PyLong_IsCompact()
* _PyLong_CompactValue()

Py_TYPE() argument is not const.

Fix the compiler warning:

  Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
  Include/pyport.h:19:31: error: cast discards ‘const’ qualifier from
  pointer target type [-Werror=cast-qual]
    (...)
  Include/cpython/longintrepr.h:133:30: note: in expansion of macro
  ‘Py_TYPE’
    assert(PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_LONG_SUBCLASS));
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
Check the usage of the 'const' qualifier in the Python C API in
test_cext.
@markshannon
Copy link
Member

I don't think weakening the type of calls that should be const is the correct thing to do here.
Where there is a conflict in types we should be strengthening the type, not weakening it.
Why not strengthen the types in the callees, rather than weakening them in the callers?

estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…n#121053)

Remove the const qualifier of the argument of functions:

* _PyLong_IsCompact()
* _PyLong_CompactValue()

Py_TYPE() argument is not const.

Fix the compiler warning:

  Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
  Include/pyport.h:19:31: error: cast discards ‘const’ qualifier from
  pointer target type [-Werror=cast-qual]
    (...)
  Include/cpython/longintrepr.h:133:30: note: in expansion of macro
  ‘Py_TYPE’
    assert(PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_LONG_SUBCLASS));
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
Check the usage of the 'const' qualifier in the Python C API in
test_cext.
vstinner added a commit to vstinner/cpython that referenced this issue Jul 27, 2024
Change _PyLong_IsCompact() and _PyLong_CompactValue() parameter type
from 'PyObject*' to 'const PyObject*'. Avoid the Py_TYPE() macro
which does not support const parameter.
@vstinner
Copy link
Member

@markshannon:

I don't think weakening the type of calls that should be const is the correct thing to do here.

If you're talking about _PyLong_CompactValue(): I wrote PR gh-122367 to restore the const modifier.

If you're talking about something else, please elaborate which function/code you are referring to.

vstinner added a commit that referenced this issue Jul 28, 2024
Change _PyLong_IsCompact() and _PyLong_CompactValue() parameter type
from 'PyObject*' to 'const PyObject*'. Avoid the Py_TYPE() macro
which does not support const parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants