classification
Title: __builtin_bswap16 is used without checking it is supported
Type: compile error Stage: resolved
Components: Build Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jmr, vstinner
Priority: normal Keywords: patch

Created on 2020-08-23 01:02 by jmr, last changed 2020-12-08 13:31 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 21942 closed jmr, 2020-08-23 01:40
PR 21943 closed jmr, 2020-08-23 02:03
PR 21949 closed vstinner, 2020-08-24 16:33
PR 22042 merged vstinner, 2020-09-01 15:49
PR 22044 merged vstinner, 2020-09-01 16:29
PR 23260 merged vstinner, 2020-11-13 14:12
PR 23262 merged vstinner, 2020-11-13 15:08
Messages (13)
msg375806 - (view) Author: Joshua Root (jmr) * Date: 2020-08-23 01:02
Older clang versions don't have __builtin_bswap16, but it's always used when compiling with clang, which means the build fails with those older versions. The code should use __has_builtin to check.
msg375856 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-08-24 16:35
> Older clang versions don't have __builtin_bswap16,

According to https://github.com/nodejs/node/pull/7644 it's available in clang 3.2 but not in clang 3.0.

I wrote PR 21949 to fix the issue: it only uses __has_builtin() if __clang__ is defined, it's different than PR 21942.
msg376193 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-01 15:30
Joshua Root: I'm curious. Can I ask you on which platform do you use clang older than 3.2? It has been released 8 years ago, December 2012. The LLVM project is known to evolve very quickly!
msg376199 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-01 16:25
New changeset e6905e4c82cc05897dc1bf5ab2b5b94b2b043a7f by Victor Stinner in branch 'master':
bpo-41617: Fix pycore_bitutils.h to support clang 3.0 (GH-22042)
https://github.com/python/cpython/commit/e6905e4c82cc05897dc1bf5ab2b5b94b2b043a7f
msg376204 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-01 18:54
New changeset 4217b3c12809b070928413f75949a7ddc4f2221c by Victor Stinner in branch '3.9':
bpo-41617: Fix pycore_byteswap.h to support clang 3.0 (GH-22042) (GH-22044)
https://github.com/python/cpython/commit/4217b3c12809b070928413f75949a7ddc4f2221c
msg376205 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-01 18:56
Thank you Joshua Root for your bug report! Your PR 21942 was good, but I chose to avoid __has_builtin() since it is not supported by all compilers and only available since GCC 10 for GCC.
msg377149 - (view) Author: Joshua Root (jmr) * Date: 2020-09-19 04:31
> I'm curious. Can I ask you on which platform do you use clang older than 3.2?

Mac OS X 10.7 / Xcode 4.6.3. I'm not using it personally, but we have automated builds on that platform.

Unfortunately the patch ultimately committed did not fix the build there. Clang reports its version as "Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)". __clang_major__ is 4 and __clang_minor__ is 2.

Apple's versioning scheme is different to that of LLVM upstream, which is one reason why I preferred detecting features directly rather than inserting externally-derived knowledge about which versions provide which features.

Apologies for not getting back to you about this sooner; the notifications appear to have gotten lost.
msg380889 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-13 14:13
> Unfortunately the patch ultimately committed did not fix the build there. Clang reports its version as "Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)". __clang_major__ is 4 and __clang_minor__ is 2.

Oh... I wrote PR 23260 which is based on your original PR 21942. I added a new _Py__has_builtin() macro rather than defining a __has_builtin() which always returns 0.
msg380894 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-13 14:38
New changeset b3b98082c5431e77c64cab2c85525a804436b505 by Victor Stinner in branch 'master':
bpo-41617: Add _Py__has_builtin() macro (GH-23260)
https://github.com/python/cpython/commit/b3b98082c5431e77c64cab2c85525a804436b505
msg380900 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-13 15:38
New changeset ec306a2fd91d8b961b2a80c080dd2262bb17d862 by Victor Stinner in branch '3.9':
bpo-41617: Add _Py__has_builtin() macro (GH-23260) (GH-23262)
https://github.com/python/cpython/commit/ec306a2fd91d8b961b2a80c080dd2262bb17d862
msg380902 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-13 15:38
Ok, I hope that this issue it's really fixed ;-) Thanks again Joshua Root.
msg382731 - (view) Author: Joshua Root (jmr) * Date: 2020-12-08 12:28
Confirmed fixed in 3.9.1 and 3.10.0a3. Thanks Victor!
msg382733 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-12-08 13:31
> Confirmed fixed in 3.9.1 and 3.10.0a3. Thanks Victor!

Woooot!
History
Date User Action Args
2020-12-08 13:31:40vstinnersetmessages: + msg382733
2020-12-08 12:28:45jmrsetmessages: + msg382731
2020-11-13 15:38:39vstinnersetstatus: open -> closed

messages: + msg380902
stage: patch review -> resolved
2020-11-13 15:38:09vstinnersetmessages: + msg380900
2020-11-13 15:08:55vstinnersetpull_requests: + pull_request22158
2020-11-13 14:38:25vstinnersetmessages: + msg380894
2020-11-13 14:13:31vstinnersetmessages: + msg380889
2020-11-13 14:12:06vstinnersetstage: resolved -> patch review
pull_requests: + pull_request22157
2020-09-19 04:31:43jmrsetstatus: closed -> open

messages: + msg377149
2020-09-01 18:56:47vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-09-01 18:56:30vstinnersetmessages: + msg376205
2020-09-01 18:54:46vstinnersetmessages: + msg376204
2020-09-01 16:29:29vstinnersetpull_requests: + pull_request21141
2020-09-01 16:25:28vstinnersetmessages: + msg376199
2020-09-01 15:49:34vstinnersetpull_requests: + pull_request21139
2020-09-01 15:30:47vstinnersetmessages: + msg376193
2020-08-24 16:35:36vstinnersetmessages: + msg375856
2020-08-24 16:33:09vstinnersetpull_requests: + pull_request21059
2020-08-24 03:50:03ned.deilysetnosy: + vstinner
2020-08-23 02:03:55jmrsetpull_requests: + pull_request21053
2020-08-23 01:43:24jmrsetversions: + Python 3.10
2020-08-23 01:40:43jmrsetkeywords: + patch
stage: patch review
pull_requests: + pull_request21052
2020-08-23 01:02:46jmrcreate