classification
Title: Add -fwrapv for new icc versions
Type: behavior Stage: patch review
Components: Build Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Laël Cellier, benjamin.peterson, furkanonder, skrah
Priority: normal Keywords: patch

Created on 2020-04-08 10:40 by skrah, last changed 2020-06-30 18:07 by skrah.

Pull Requests
URL Status Linked Edit
PR 19561 open furkanonder, 2020-04-17 00:05
Messages (12)
msg365976 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-04-08 10:40
Newer icc version require -fwrapv:

https://software.intel.com/en-us/forums/intel-c-compiler/topic/849064
msg366673 - (view) Author: Furkan Onder (furkanonder) * Date: 2020-04-17 18:10
Pr has been sent.
msg371110 - (view) Author: Laël Cellier (Laël Cellier) Date: 2020-06-09 14:40
If -fwrapv isn’t enabled (whereas with gcc this is the default with ‑O2), then it means -ftrapv is enabled.

And from the manual page :
This option generates traps for signed overflow on addition, subtraction, multiplication operations.

Did you checked if the binary compiles fines with all ‑O2/‑O3 options replaced with ‑O0 in configure/Makefiles along gcc ?
msg371114 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-06-09 14:56
I'm aware of what -fwrapv does, it is a long standing issue in Python.

I didn't try to find the exact location of overflow, since we also use -fwrapv for gcc.

It is also possible that giving -fwrapv to icc disables another optimization that is the actual culprit.

But I don't have time to figure out any of that.
msg371126 - (view) Author: Laël Cellier (Laël Cellier) Date: 2020-06-09 16:51
No I’m meaning disabling all optimization when using gcc and using ‑ftrapv.

This should cause the same problem : a crashing binary…
msg371127 - (view) Author: Laël Cellier (Laël Cellier) Date: 2020-06-09 16:52
since ‑fwrapv is the default with ɢᴄᴄ as ‑O2 is used in Cpython build scripts.
msg372577 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-06-29 14:16
icc does not like the label reordering from:

   ddd1949fea59f256e51191540a4446f75ed608fa


This is one step further, but not much. Possibilities are still:

   1) The reordering exposes an overflow.

   2) The new ordering is not supported by icc, it introduces UB
      that is masked by -fwrapv (again, does -fwrapv only disable
      a single optimization? My guess is that it disables several).


This was found by automated bisecting, I still have no time to really
investigate what is going on.
msg372578 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-06-29 14:21
cc Benjamin, in case he has any ideas:  icc does not like the label
reordering in ceval.c, but that can be anything from an icc issue
to an actual overflow in ceval.c.
msg372636 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2020-06-29 21:54
I don't readily have access to ICC, so I can't personally debug this. That commit was supposed to be a functional noop, though it may have exposed different optimization opportunities to the compiler. I would like to see Python not assumed signed overflow.
msg372637 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-06-29 22:17
Yeah, I already felt a bit guilty about adding you -- it could be a compiler bug or an actual overflow.  My bet is also that the reordering exposes an existing overflow.  The reordering itself certainly looks correct to me.

When I have time, I'll try to look into it.
msg372638 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2020-06-29 22:26
I agree that your fix it correct, though because even if the specific issue you saw is a compiler bug, we need to tell our compilers than Python assumes signed wraparound.
msg372712 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2020-06-30 18:07
It looks like a compiler bug (line numbers are after macro expansion):


#0  0x00000000006ea678 in _PyEval_EvalFrameDefault (f=0x886d20 <_PyRuntime+352>, throwflag=-8) at Python/ceval.c:35554
#1  0x000000000057167b in _PyEval_EvalCodeWithName (_co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff, args=0x1, argcount=9370836, kwnames=0x0, kwargs=0x90bd90, kwcount=0, kwstep=1, 
    defs=0x0, defcount=0, kwdefs=0x0, closure=0x0, name=0x8f05a0, qualname=0x8f05a0) at Python/ceval.c:33634
#2  0x000000000043a693 in _PyFunction_FastCallKeywords (func=0x886d20 <_PyRuntime+352>, stack=0xfffffffffffffff8, nargs=-1, kwnames=0x1) at Objects/call.c:433
#3  0x00000000006e65f5 in call_function (pp_stack=0x886d20 <_PyRuntime+352>, oparg=-8, kwnames=0xffffffffffffffff) at Python/ceval.c:37762
#4  0x00000000006eb7a8 in _PyEval_EvalFrameDefault (f=0x886d20 <_PyRuntime+352>, throwflag=-8) at Python/ceval.c:36385
#5  0x000000000057167b in _PyEval_EvalCodeWithName (_co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff, args=0x1, argcount=9370836, kwnames=0x0, kwargs=0x0, kwcount=0, kwstep=2, defs=0x0, 
    defcount=0, kwdefs=0x0, closure=0x0, name=0x0, qualname=0x0) at Python/ceval.c:33634
#6  0x0000000000571e41 in PyEval_EvalCodeEx (_co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff, args=0x1, argcount=9370836, kws=0x0, kwcount=2, defs=0x0, defcount=0, kwdefs=0x0, closure=0x0)
    at Python/ceval.c:37166
#7  0x00000000006bdb95 in builtin___build_class__ (self=0x886d20 <_PyRuntime+352>, args=0xfffffffffffffff8, nargs=-1, kwnames=0x1) at Python/bltinmodule.c:221
#8  0x000000000043a41e in _PyMethodDef_RawFastCallKeywords (method=0x886d20 <_PyRuntime+352>, self=0xfffffffffffffff8, args=0xffffffffffffffff, nargs=1, kwnames=0x8efcd4) at Objects/call.c:656
#9  0x000000000043ada6 in _PyCFunction_FastCallKeywords (func=0x886d20 <_PyRuntime+352>, args=0xfffffffffffffff8, nargs=-1, kwnames=0x1) at Objects/call.c:730
#10 0x00000000006e6a64 in call_function (pp_stack=0x886d20 <_PyRuntime+352>, oparg=-8, kwnames=0xffffffffffffffff) at Python/ceval.c:37714
#11 0x00000000006eb7a8 in _PyEval_EvalFrameDefault (f=0x886d20 <_PyRuntime+352>, throwflag=-8) at Python/ceval.c:36385
#12 0x000000000057167b in _PyEval_EvalCodeWithName (_co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff, args=0x1, argcount=9370836, kwnames=0x0, kwargs=0x0, kwcount=0, kwstep=2, defs=0x0, 
    defcount=0, kwdefs=0x0, closure=0x0, name=0x0, qualname=0x0) at Python/ceval.c:33634
#13 0x0000000000570c52 in PyEval_EvalCodeEx (_co=<optimized out>, globals=<optimized out>, locals=<optimized out>, args=<optimized out>, argcount=<optimized out>, kws=<optimized out>, kwcount=<optimized out>, 
    defs=<optimized out>, defcount=<optimized out>, kwdefs=<optimized out>, closure=<optimized out>) at Python/ceval.c:37166
#14 PyEval_EvalCode (co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff) at Python/ceval.c:33611
#15 0x00000000005b20ba in exec_code_in_module (name=<optimized out>, module_dict=<optimized out>, code_object=<optimized out>) at Python/import.c:952
#16 PyImport_ImportFrozenModuleObject (name=<optimized out>) at Python/import.c:1357
#17 PyImport_ImportFrozenModule (name=0x886d20 <_PyRuntime+352> "\020\023\216") at Python/import.c:1376
#18 0x00000000005ca1f7 in _Py_InitializeCore_impl (interp_p=0xfffffffffffffff8, core_config=0xffffffffffffffff) at Python/pylifecycle.c:197
#19 0x00000000005c9be5 in _Py_InitializeCore (interp_p=0xfffffffffffffff8, src_config=0xffffffffffffffff) at Python/pylifecycle.c:745
#20 0x0000000000429859 in pymain_init (pymain=0x886d20 <_PyRuntime+352>, interp_p=0xfffffffffffffff8) at Modules/main.c:1733
#21 0x0000000000428ddf in pymain_main (pymain=<optimized out>) at Modules/main.c:1753
#22 _Py_UnixMain (argc=8940832, argv=0xfffffffffffffff8) at Modules/main.c:1792
#23 0x00007ffff7c90f43 in __libc_start_main () from /lib64/libc.so.6
#24 0x000000000042652e in _start ()


In frame 13 argcount is still 0:

#13 0x0000000000570c52 in PyEval_EvalCodeEx (_co=<optimized out>, globals=<optimized out>, locals=<optimized out>, args=<optimized out>, argcount=<optimized out>, kws=<optimized out>, kwcount=<optimized out>, 
    defs=<optimized out>, defcount=<optimized out>, kwdefs=<optimized out>, closure=<optimized out>) at Python/ceval.c:37166
37166       return _PyEval_EvalCodeWithName(_co, globals, locals,
(gdb) l
37161                     PyObject *const *kws, int kwcount,
37162                     PyObject *const *defs, int defcount,
37163                     PyObject *kwdefs, PyObject *closure)
37164   {
37165       if (argcount != 0) abort();
37166       return _PyEval_EvalCodeWithName(_co, globals, locals,
37167                                       args, argcount,
37168                                       kws, kws != ((void*)0) ? kws + 1 : ((void*)0),
37169                                       kwcount, 2,
37170                                       defs, defcount,


In frame 12 it looks uninitialized:

(gdb) f 12
#12 0x000000000057167b in _PyEval_EvalCodeWithName (_co=0x886d20 <_PyRuntime+352>, globals=0xfffffffffffffff8, locals=0xffffffffffffffff, args=0x1, argcount=9370836, kwnames=0x0, kwargs=0x0, kwcount=0, kwstep=2, defs=0x0, 
    defcount=0, kwdefs=0x0, closure=0x0, name=0x0, qualname=0x0) at Python/ceval.c:33634
33634       return interp->eval_frame(f, throwflag);


So yes, if -fwrapv of all things prevents that, let's go for it.
History
Date User Action Args
2020-06-30 18:07:54skrahsetmessages: + msg372712
2020-06-29 22:26:14benjamin.petersonsetmessages: + msg372638
2020-06-29 22:17:57skrahsetmessages: + msg372637
2020-06-29 21:54:15benjamin.petersonsetmessages: + msg372636
2020-06-29 14:21:28skrahsetnosy: + benjamin.peterson
messages: + msg372578
2020-06-29 14:16:13skrahsetmessages: + msg372577
2020-06-09 16:52:17Laël Celliersetmessages: + msg371127
2020-06-09 16:51:08Laël Celliersetmessages: + msg371126
2020-06-09 14:56:14skrahsetmessages: + msg371114
2020-06-09 14:40:02Laël Celliersetnosy: + Laël Cellier
messages: + msg371110
2020-04-17 18:10:14furkanondersetmessages: + msg366673
2020-04-17 00:05:01furkanondersetkeywords: + patch
nosy: + furkanonder

pull_requests: + pull_request18905
stage: needs patch -> patch review
2020-04-08 10:40:29skrahcreate