| msg276195 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2016-09-13 07:23 |
| Since last week I'm getting a compiler warning in _ssl.c. The compiler warning is related to the type of _ssl_locks_count. It's an unsigned. When I cast it to an int like PyMem_New(PyThread_type_lock, (int)_ssl_locks_count), the warning goes away. gcc -pthread -fPIC -Wno-unused-result -Wsign-compare -g -Og -Wall -Wstrict-prototypes -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -I./Include -I. -IInclude -I/usr/local/include -I/home/heimes/dev/python/3.6/Include -I/home/heimes/dev/python/3.6 -c /home/heimes/dev/python/3.6/Modules/_ssl.c -o build/temp.linux-x86_64-3.6-pydebug/home/heimes/dev/python/3.6/Modules/_ssl.o In file included from ./Include/Python.h:66:0, from /home/heimes/dev/python/3.6/Modules/_ssl.c:19: /home/heimes/dev/python/3.6/Modules/_ssl.c: In function ‘_setup_ssl_threads’: ./Include/pymem.h:136:18: warning: comparison is always false due to limited range of data type [-Wtype-limits] ( ((size_t)(n) > PY_SSIZE_T_MAX / sizeof(type)) ? NULL : \ ^ /home/heimes/dev/python/3.6/Modules/_ssl.c:5076:22: note: in expansion of macro ‘PyMem_New’ _ssl_locks = PyMem_New(PyThread_type_lock, _ssl_locks_count); ^~~~~~~~~ Also see #17884 |
|
|
| msg276196 - (view) |
Author: SilentGhost (SilentGhost) *  |
Date: 2016-09-13 07:28 |
| It seems to be also related to #23545 (Martin saw a similar warning about a month ago). |
|
|
| msg276197 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-13 07:28 |
| It's related to new enabled GCC warnings: see issue #23545 where the warning was already reported. The code is fine. It's just hard to compute the limits of a data type in a C macro :-/ |
|
|
| msg276199 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-13 07:35 |
| I'm going to add a workaround. _ssl_locks_count is an unsigned int because CRYPTO_num_locks() returns an unsigned int. We can safely use a signed int here. OpenSSL needs about 20, 30 locks or so. |
|
|
| msg276212 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-13 08:40 |
| As Serhiy suggested in the other bug, one workaround is just to disable the warning with -Wno-type-limits. It would depend if the benefits of the warning outweigh the annoyance of coming up with a more complicated workaround for this specific case. |
|
|
| msg276213 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-13 08:47 |
| The code is going to go away with OpenSSL 1.1.0. |
|
|
| msg276220 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-13 09:08 |
| Perhaps another way to defeat the warning is to make PyMem_New() an inline function? I haven’t tried, but this way would make all the data types involved more explicit. |
|
|
| msg276221 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-09-13 09:09 |
| Martin: "Perhaps another way to defeat the warning is to make PyMem_New() an inline function?" See the issue #28092 for compilation issues of inline functions. We should decide if inline functions are ok or not. |
|
|
| msg276247 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-13 11:11 |
| I was thinking of “static inline” for PyMem_New(). I understand the Centos and OS X Tiger problem is only related “extern inline” vs plain “inline”, and “static inline” should not be affected. |
|
|
| msg276321 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-13 18:46 |
| I have an even better solution that gets rid of the warning and the extra memset() call: diff -r bedce61ae0a0 Modules/_ssl.c --- a/Modules/_ssl.c Tue Sep 13 20:22:02 2016 +0200 +++ b/Modules/_ssl.c Tue Sep 13 20:45:46 2016 +0200 @@ -5073,13 +5073,12 @@ if (_ssl_locks == NULL) { _ssl_locks_count = CRYPTO_num_locks(); - _ssl_locks = PyMem_New(PyThread_type_lock, _ssl_locks_count); + _ssl_locks = PyMem_Calloc(_ssl_locks_count, + sizeof(PyThread_type_lock)); if (_ssl_locks == NULL) { PyErr_NoMemory(); return 0; } - memset(_ssl_locks, 0, - sizeof(PyThread_type_lock) * _ssl_locks_count); for (i = 0; i < _ssl_locks_count; i++) { _ssl_locks[i] = PyThread_allocate_lock(); if (_ssl_locks[i] == NULL) { |
|
|
| msg276322 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2016-09-13 18:50 |
| Fixed in 9e8e15993aae |
|
|
| msg276368 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-14 02:05 |
| Thanks that works well Christian |
|
|