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

GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014

Closed
vstinner opened this issue Sep 28, 2023 · 2 comments
Closed

GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014

vstinner opened this issue Sep 28, 2023 · 2 comments
Assignees

Comments

@vstinner
Copy link
Member

vstinner commented Sep 28, 2023

Building the Python main branch on RHEL8 with gcc -Og -flto (GCC 8.5.0 on ppc64le) emits the following compiler warnings:

./Include/internal/pycore_semaphore.h:54:1: warning: type of ‘_PySemaphore_Wakeup’ does not match original declaration [-Wlto-type-mismatch]
 _PySemaphore_Wakeup(_PySemaphore *sema);
 ^
Python/parking_lot.c:197:1: note: ‘_PySemaphore_Wakeup’ was previously declared here
 _PySemaphore_Wakeup(_PySemaphore *sema)
 ^
Python/parking_lot.c:197:1: note: code may be misoptimized unless -fno-strict-aliasing is used
./Include/internal/pycore_semaphore.h:57:18: warning: type of ‘_PySemaphore_Init’ does not match original declaration [-Wlto-type-mismatch]
 PyAPI_FUNC(void) _PySemaphore_Init(_PySemaphore *sema);
                  ^
Python/parking_lot.c:53:1: note: ‘_PySemaphore_Init’ was previously declared here
 _PySemaphore_Init(_PySemaphore *sema)
 ^
Python/parking_lot.c:53:1: note: code may be misoptimized unless -fno-strict-aliasing is used
./Include/internal/pycore_semaphore.h:49:1: warning: type of ‘_PySemaphore_Wait’ does not match original declaration [-Wlto-type-mismatch]
 _PySemaphore_Wait(_PySemaphore *sema, _PyTime_t timeout_ns, int detach);
 ^
Python/parking_lot.c:178:1: note: ‘_PySemaphore_Wait’ was previously declared here
 _PySemaphore_Wait(_PySemaphore *sema, _PyTime_t timeout, int detach)
 ^
Python/parking_lot.c:178:1: note: code may be misoptimized unless -fno-strict-aliasing is used
./Include/internal/pycore_semaphore.h:58:18: warning: type of ‘_PySemaphore_Destroy’ does not match original declaration [-Wlto-type-mismatch]
 PyAPI_FUNC(void) _PySemaphore_Destroy(_PySemaphore *sema);
                  ^
Python/parking_lot.c:81:1: note: ‘_PySemaphore_Destroy’ was previously declared here
 _PySemaphore_Destroy(_PySemaphore *sema)
 ^
Python/parking_lot.c:81:1: note: code may be misoptimized unless -fno-strict-aliasing is used
./python -E -S -m sysconfig --generate-posix-vars ;\
if test $? -ne 0 ; then \
	echo "generate-posix-vars failed" ; \
	rm -f ./pybuilddir.txt ; \
	exit 1 ; \
fi

cc @colesbury @ericsnowcurrently

Linked PRs

@colesbury
Copy link
Contributor

Thanks, I'll take a look at this today.

@colesbury colesbury self-assigned this Sep 28, 2023
@colesbury
Copy link
Contributor

colesbury added a commit to colesbury/cpython that referenced this issue Sep 28, 2023
…re.h

The pycore_semaphore.h header is included by Python/lock.c and
Python/parking_lot.c. The macro `_POSIX_SEMAPHORES` was not consistently
defined across the two files (due to a missing include of `<unistd.h>`)
leading to different struct definitions. The RHEL8 ppc64le LTO buildbot
correctly warned due to this issue.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* PY_TIMEOUT_MAX is no longer defined as a macro but as a constant
  since its value depends on _POSIX_THREADS.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS or
  _POSIX_SEMAPHORES macros are tested.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* PY_TIMEOUT_MAX is no longer defined as a macro but as a constant
  since its value depends on _POSIX_THREADS.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS or
  _POSIX_SEMAPHORES macros are tested.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS or
  _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS or
  _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Document the change and give hints how to fix affected code.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Prevent integer overflow in the preprocessor when computing
  PY_TIMEOUT_MAX_VALUE on Windows:
  replace "0xFFFFFFFELL * 1000 < LLONG_MAX"
  with "0xFFFFFFFELL < LLONG_MAX / 1000".
* Document the change and give hints how to fix affected code.
vstinner added a commit that referenced this issue Sep 30, 2023
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Prevent integer overflow in the preprocessor when computing
  PY_TIMEOUT_MAX_VALUE on Windows:
  replace "0xFFFFFFFELL * 1000 < LLONG_MAX"
  with "0xFFFFFFFELL < LLONG_MAX / 1000".
* Document the change and give hints how to fix affected code.
* Add an exception for PY_TIMEOUT_MAX  name to smelly.py
* Add PY_TIMEOUT_MAX to the stable ABI
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* Remove unused include: <locale.h>. Remove <fcntl.h> in traceback.h.
* Remove redundant include: <assert.h> and <stddef.h> are already
  included by "Python.h".
* Remove include <object.h> in faulthandler.c.
* Add missing <stdbool.h> in pycore_pythread.h if HAVE_PTHREAD_STUBS
  is defined.
* Fix also warnings in pthread_stubs.h: don't redefine macros if they
  are already defined, like the __NEED_pthread_t macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* Remove unused <locale.h> include.
* Remove <fcntl.h> include in traceback.h.
* Remove redundant include: <assert.h> and <stddef.h> are already
  included by "Python.h".
* Remove <object.h> include in faulthandler.c.
* Add missing <stdbool.h> in pycore_pythread.h if HAVE_PTHREAD_STUBS
  is defined.
* Fix also warnings in pthread_stubs.h: don't redefine macros if they
  are already defined, like the __NEED_pthread_t macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
* Remove unused <locale.h> include.
* Remove <fcntl.h> include in traceback.h.
* Remove redundant include: <assert.h> and <stddef.h> are already
  included by "Python.h".
* Remove <object.h> include in faulthandler.c.
* Add missing <stdbool.h> in pycore_pythread.h if HAVE_PTHREAD_STUBS
  is defined.
* Fix also warnings in pthread_stubs.h: don't redefine macros if they
  are already defined, like the __NEED_pthread_t macro.
vstinner added a commit that referenced this issue Sep 30, 2023
* Remove unused <locale.h> includes.
* Remove unused <fcntl.h> include in traceback.h.
* Remove redundant <assert.h> and <stddef.h> includes. They  are already
  included by "Python.h".
* Remove <object.h> include in faulthandler.c. Python.h already includes it.
* Add missing <stdbool.h> in pycore_pythread.h if HAVE_PTHREAD_STUBS
  is defined.
* Fix also warnings in pthread_stubs.h: don't redefine macros if they
  are already defined, like the __NEED_pthread_t macro.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 30, 2023
Oops, I commited debug code by mistake, sorry about that.
vstinner added a commit that referenced this issue Sep 30, 2023
Oops, I commited debug code by mistake, sorry about that.
vstinner added a commit to vstinner/cpython that referenced this issue Oct 2, 2023
If the timeout is greater than PY_TIMEOUT_MAX,
PyThread_acquire_lock_timed() uses a timeout of PY_TIMEOUT_MAX
microseconds, which is around 280.6 years. This case is unlikely and
limiting a timeout to 280.6 years sounds like a reasonable trade-off.

The constant PY_TIMEOUT_MAX is not used in PyPI top 5,000 projects.
vstinner added a commit to vstinner/cpython that referenced this issue Oct 2, 2023
If the timeout is greater than PY_TIMEOUT_MAX,
PyThread_acquire_lock_timed() uses a timeout of PY_TIMEOUT_MAX
microseconds, which is around 280.6 years. This case is unlikely and
limiting a timeout to 280.6 years sounds like a reasonable trade-off.

The constant PY_TIMEOUT_MAX is not used in PyPI top 5,000 projects.
vstinner added a commit that referenced this issue Oct 2, 2023
If the timeout is greater than PY_TIMEOUT_MAX,
PyThread_acquire_lock_timed() uses a timeout of PY_TIMEOUT_MAX
microseconds, which is around 280.6 years. This case is unlikely and
limiting a timeout to 280.6 years sounds like a reasonable trade-off.

The constant PY_TIMEOUT_MAX is not used in PyPI top 5,000 projects.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…on#110139)

* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Prevent integer overflow in the preprocessor when computing
  PY_TIMEOUT_MAX_VALUE on Windows:
  replace "0xFFFFFFFELL * 1000 < LLONG_MAX"
  with "0xFFFFFFFELL < LLONG_MAX / 1000".
* Document the change and give hints how to fix affected code.
* Add an exception for PY_TIMEOUT_MAX  name to smelly.py
* Add PY_TIMEOUT_MAX to the stable ABI
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
* Remove unused <locale.h> includes.
* Remove unused <fcntl.h> include in traceback.h.
* Remove redundant <assert.h> and <stddef.h> includes. They  are already
  included by "Python.h".
* Remove <object.h> include in faulthandler.c. Python.h already includes it.
* Add missing <stdbool.h> in pycore_pythread.h if HAVE_PTHREAD_STUBS
  is defined.
* Fix also warnings in pthread_stubs.h: don't redefine macros if they
  are already defined, like the __NEED_pthread_t macro.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Oops, I commited debug code by mistake, sorry about that.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
)

If the timeout is greater than PY_TIMEOUT_MAX,
PyThread_acquire_lock_timed() uses a timeout of PY_TIMEOUT_MAX
microseconds, which is around 280.6 years. This case is unlikely and
limiting a timeout to 280.6 years sounds like a reasonable trade-off.

The constant PY_TIMEOUT_MAX is not used in PyPI top 5,000 projects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants