AC_C_CHAR_UNSIGNED from configure.ac confuses GCC 12+ by defining __CHAR_UNSIGNED__

[hroncok]

BPO	46513
Nosy	@vstinner, @tiran, @encukou, @hroncok
PRs	bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ #30851 [3.10] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30914) #30914 [3.9] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30915) #30915

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-26.11:21:50.243>
created_at = <Date 2022-01-25.11:28:33.802>
labels = ['build', '3.9', '3.10', '3.11']
title = 'AC_C_CHAR_UNSIGNED from configure.ac confuses GCC 12+ by defining __CHAR_UNSIGNED__'
updated_at = <Date 2022-01-26.14:30:01.087>
user = 'https://github.com/hroncok'

bugs.python.org fields:

activity = <Date 2022-01-26.14:30:01.087>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-26.11:21:50.243>
closer = 'christian.heimes'
components = ['Build']
creation = <Date 2022-01-25.11:28:33.802>
creator = 'hroncok'
dependencies = []
files = []
hgrepos = []
issue_num = 46513
keywords = ['patch']
message_count = 13.0
messages = ['411575', '411576', '411654', '411655', '411665', '411666', '411718', '411719', '411739', '411740', '411741', '411752', '411753']
nosy_count = 4.0
nosy_names = ['vstinner', 'christian.heimes', 'petr.viktorin', 'hroncok']
pr_nums = ['30851', '30914', '30915']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue46513'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[hroncok]

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 #30851 confirmed that all the buildbots are happy with such change. Hence, I open this issue to actually do it.

Thanks

[tiran]

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

[vstinner]

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.

[vstinner]

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.

[vstinner]

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).

[vstinner]

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;

[miss-islington]

New changeset 6e5a193 by Christian Heimes in branch 'main':
bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851)
6e5a193

[encukou]

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.

[tiran]

New changeset 4371fbd by Christian Heimes in branch '3.10':
[3.10] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30914)
4371fbd

[tiran]

New changeset 04772cd by Christian Heimes in branch '3.9':
[3.9] bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ (GH-30851) (GH-30915)
04772cd

[tiran]

The fix is in all stable branches. Thanks!

[vstinner]

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
---

[vstinner]

In short, I did my checks on my side, and the merged change now LGTM. Thanks Christian for fixing it ;-)