This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: AC_C_CHAR_UNSIGNED from configure.ac confuses GCC 12+ by defining __CHAR_UNSIGNED__
Type: compile error Stage: resolved
Components: Build Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, hroncok, petr.viktorin, vstinner
Priority: normal Keywords: patch

Created on 2022-01-25 11:28 by hroncok, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30851 merged christian.heimes, 2022-01-25 11:34
PR 30914 merged christian.heimes, 2022-01-26 09:06
PR 30915 merged christian.heimes, 2022-01-26 09:07
Messages (13)
msg411575 - (view) Author: Miro Hrončok (hroncok) * Date: 2022-01-25 11:28
As described at https://mail.python.org/archives/list/python-dev@python.org/thread/MPHZ3TGSHMSF7C4P7JEP2ZCYLRA3ERC5/ the AC_C_CHAR_UNSIGNED macro from configure.ac confuses GCC 12+ as it exports a reserved symbol __CHAR_UNSIGNED__ through pyconfig.h.

My assumption was that AC_C_CHAR_UNSIGNED can be dropped entirely and the PR in https://github.com/python/cpython/pull/30851 confirmed that all the buildbots are happy with such change. Hence, I open this issue to actually do it.

Thanks
msg411576 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-25 11:35
Modules/audioop.c uses __CHAR_UNSIGNED__, but the absence or presence of the symbol does not affect behavior:

#if defined(__CHAR_UNSIGNED__)
#if defined(signed)
/* This module currently does not work on systems where only unsigned
   characters are available.  Take it out of Setup.  Sorry. */
#endif
#endif
msg411654 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-25 18:13
Fedora downstream issue: https://bugzilla.redhat.com/show_bug.cgi?id=2043555

We should make sure that Python can be built with -fsigned-char *and* with -funsigned-char: build Python, but also run its test suite.
msg411655 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-25 18:14
> Modules/audioop.c uses __CHAR_UNSIGNED__, but the absence or presence of the symbol does not affect behavior

This comment is really weird since audioop explicitly uses "signed char" and "unsigned char". Maybe to avoid any risk of confusion, the C code should use int8_t and uint8_t.
msg411665 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-25 19:13
To test if a C type is signed or not, I wrote this macro:

// Test if a C type is signed.
//
// Usage: assert(_Py_CTYPE_IS_SIGNED(char)); // fail if 'char' type is unsigned
#define _Py_CTYPE_IS_SIGNED(T) (((T)-1) < 0)

I planned to use it to raise an error on "import audioop" if the C "char" type is unsigned, but it seems like it's not needed, since the C extensions seems to work if char is signed or unsigned (I only read the C code, I didn't run test_audioop to actually test it).
msg411666 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-25 19:16
My audioop.c change which looks to be wrong (useless):

diff --git a/Modules/audioop.c b/Modules/audioop.c
index 3aeb6f04f13..4c04b17ce7f 100644
--- a/Modules/audioop.c
+++ b/Modules/audioop.c
@@ -1948,6 +1941,13 @@ audioop_exec(PyObject* module)
 {
     audioop_state *state = get_audioop_state(module);
 
+    if (!_Py_CTYPE_IS_SIGNED(char)) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "the audioop module does not support systems "
+                        "where the char type is unsigned");
+        return -1;
+    }
+
     state->AudioopError = PyErr_NewException("audioop.error", NULL, NULL);
     if (state->AudioopError == NULL) {
         return -1;
msg411718 - (view) Author: miss-islington (miss-islington) Date: 2022-01-26 09:03
New changeset 6e5a193816e1bdf11f5fb78d620995fd6987ccf8 by Christian Heimes in branch 'main':
bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851)
https://github.com/python/cpython/commit/6e5a193816e1bdf11f5fb78d620995fd6987ccf8
msg411719 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2022-01-26 09:04
Yeah, that looks like it's for some long-forgotten compiler that didn't implement `signed char` at all. 1994 was a fun time, apparently.
msg411739 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-26 11:21
New changeset 4371fbd4328781496f5f2c6938c4d9a84049b187 by Christian Heimes in branch '3.10':
[3.10] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30914)
https://github.com/python/cpython/commit/4371fbd4328781496f5f2c6938c4d9a84049b187
msg411740 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-26 11:21
New changeset 04772cd6f164c1219c8c74d55626ba114f01aa96 by Christian Heimes in branch '3.9':
[3.9] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30915)
https://github.com/python/cpython/commit/04772cd6f164c1219c8c74d55626ba114f01aa96
msg411741 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-26 11:21
The fix is in all stable branches. Thanks!
msg411752 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-26 14:29
IMO making the assumption that "char" is signed or not in C code is bad. If Python has code like that, it must be signed to explicitly use one of these types: unsigned char or uint8_t, signed char or int8_t. Hopefully, Python can now use C99 <stdint.h> since Python 3.6.

On my x86-64 Fedora 35 (GCC 11.2.1), the "char" type is signed. I built Python with -funsigned-char and I ran the test suite: the whole test suite pass! Commands:

---
make distclean
./configure --with-pydebug CFLAGS="-O0 -funsigned-char" --with-system-expat --with-system-ffi
make
./python -m test -j0 -r
---

Using ./configure CFLAGS, -funsigned-char is also used to build C extensions. Example: 

   gcc (...) -O0 -funsigned-char (...) Modules/_elementtree.c (...)


For completeness, I also built Python with -fsigned-char. Again, the full test suite passed ;-)

---
make distclean
./configure --with-pydebug CFLAGS="-O0 -fsigned-char" --with-system-expat --with-system-ffi
make
./python -m test -r -j0
---
msg411753 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-26 14:30
In short, I did my checks on my side, and the merged change now LGTM. Thanks Christian for fixing it ;-)
History
Date User Action Args
2022-04-11 14:59:55adminsetgithub: 90671
2022-01-26 14:30:01vstinnersetmessages: + msg411753
2022-01-26 14:29:28vstinnersetmessages: + msg411752
2022-01-26 11:21:50christian.heimessetstatus: open -> closed
resolution: fixed
messages: + msg411741

stage: patch review -> resolved
2022-01-26 11:21:08christian.heimessetmessages: + msg411739
2022-01-26 11:21:08christian.heimessetmessages: + msg411740
2022-01-26 09:07:01christian.heimessetpull_requests: + pull_request29094
2022-01-26 09:06:09christian.heimessetpull_requests: + pull_request29093
2022-01-26 09:04:44petr.viktorinsetnosy: - miss-islington
messages: + msg411719
2022-01-26 09:03:58miss-islingtonsetnosy: + miss-islington
messages: + msg411718
2022-01-25 19:16:28vstinnersetmessages: + msg411666
2022-01-25 19:13:02vstinnersetmessages: + msg411665
2022-01-25 18:14:01vstinnersetmessages: + msg411655
2022-01-25 18:13:07vstinnersetmessages: + msg411654
2022-01-25 11:35:42christian.heimessetmessages: + msg411576
2022-01-25 11:34:07christian.heimessetkeywords: + patch
stage: patch review
pull_requests: + pull_request29062
2022-01-25 11:28:33hroncokcreate