[PATCH] Configure should enable -fwrapv for clang

[donmez]

BPO	11149
Nosy	@brettcannon, @rhettinger, @jcea, @mdickinson, @pitrou, @vstinner, @skrah, @jmroot, @moubctez
Files	clang-fwrapv.diff: Enable -fwrapv for clang

^{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 = 'https://github.com/brettcannon'
closed_at = <Date 2011-09-15.09:18:26.510>
created_at = <Date 2011-02-08.10:46:46.700>
labels = ['type-bug', 'build']
title = '[PATCH] Configure should enable -fwrapv for clang'
updated_at = <Date 2011-12-08.21:31:16.096>
user = 'https://bugs.python.org/donmez'

bugs.python.org fields:

activity = <Date 2011-12-08.21:31:16.096>
actor = 'python-dev'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2011-09-15.09:18:26.510>
closer = 'skrah'
components = ['Build']
creation = <Date 2011-02-08.10:46:46.700>
creator = 'donmez'
dependencies = []
files = ['20717']
hgrepos = []
issue_num = 11149
keywords = ['patch', 'needs review']
message_count = 23.0
messages = ['128170', '128171', '128172', '128174', '128183', '128184', '128379', '128443', '128519', '131540', '131544', '135235', '136575', '143991', '143993', '144012', '144025', '144026', '144046', '144048', '144070', '148088', '149055']
nosy_count = 11.0
nosy_names = ['brett.cannon', 'rhettinger', 'jcea', 'mark.dickinson', 'pitrou', 'vstinner', 'donmez', 'skrah', 'jmr', 'python-dev', 'adam@NetBSD.org']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue11149'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

[donmez]

clang (as with gcc 4.x) assumes signed integer overflow is undefined. But Python depends on the fact that signed integer overflow wraps.

Currently configure script adds -fwrapv flag for GCC (added by me back in the day), same should be done with clang.

Patch attached.

[vstinner]

You should maybe add a test into _testcapi for this issue.

[donmez]

Currently test_overflow inside test_list fails without this patch. Is that good enough or should we create a specific test?

[vstinner]

Ah, there is already a test for that: ok, it's fine and enough.

[brettcannon]

I am using LLVM/Clang 2.8 and test_list passes fine for me as-is. Does it only fail under 32 or 64-bit conditions? Configure compiles under 32-bits for me even though I have -arch x86_64 set.

[donmez]

This is only reproducable with clang 2.9 trunk builds due to new optimizations. Also note that clang commits confirm that -fwrapv must be provided if you assume signed integer overflow wraps. This was also the case with a spec test.

I tested this on OSX 10.6.6 with x86-64 build.

[donmez]

I think the patch is good to go, any comments/questions ?

[pitrou]

This is a potential crasher.
It would also be nice if you listed those cases where Python assumes signed overflow behaviour. Can you open a separate issue for that?

[donmez]

@antoine Pitrou , that is tracked by http://bugs.python.org/issue1621

[donmez]

Whats holding this up?

[brettcannon]

Someone having the time to do a patch review.

[rhettinger]

Brett, can you test this?