BUG: Integer overflow in remainder for Windows #19260
Conversation
numpy/core/tests/test_umath.py
Outdated
| with pytest.raises(FloatingPointError): | ||
| np.remainder(a, -1) | ||
| # with pytest.raises(FloatingPointError): | ||
| # np.remainder(iinfo.min, -1) |
There was a problem hiding this comment.
This is not using the same function as
numpy/numpy/core/src/umath/loops.c.src
Line 847 in 75f852e
So only array cases are solved by adding the MIN&-1 check
ganesh-k13
force-pushed
the
BUG_19246_mod_crash
branch
from
bade866
to
35789c0
Compare
There was a problem hiding this comment.
Just stumbled on this. @ganesh-k13 might be nice to finish it?
| @@ -849,7 +849,7 @@ NPY_NO_EXPORT void | |||
| BINARY_LOOP { | |||
| const @type@ in1 = *(@type@ *)ip1; | |||
| const @type@ in2 = *(@type@ *)ip2; | |||
| if (in2 == 0) { | |||
| if (in2 == 0 || (in1 == NPY_MIN_@TYPE@ && in2 == -1)) { | |||
| npy_set_floatstatus_divbyzero(); | |||
There was a problem hiding this comment.
Do we have to set the error flag for the second branch?
|
Yeah, let me rebase and try it now. |
|
So the problem, on the whole, has been transferred to a different location via #21124, able to reproduce it though, but the issue is still there. |
|
I'm stuck in this weird issue where: Now getting gdb working on windows seems harder than SIMD :). Will try to fix it soon. |
|
Hmm, but its not that the first ends up in the array and the second the scalar path? I can't see why, but both should be 32bit on windows, so fine. There is always the possibility that the compiler optimizes away an |
|
Thanks Sebastian! Yeah I did |
I have done this in the past by
|
|
Thanks for the detailed steps @mattip ! I'll give it a go. |
|
Thanks @mattip , I'm able to hit the code, etc. Just that the I'll dig a bit deeper, but your steps were super useful! We should document this somewhere, will help CPython and not just NumPy. Looking at build logs, it seems |
Try modifying the actual code with the flags in |
|
Ah ok, I was editing in the wrong place, I edited this and got the numpy/numpy/distutils/ccompiler_opt.py Line 2314 in 9d32cf7 Only problem however, is that the pdb does not belong to any of the modules, so how/where do we load it to? As a workaround, My call stacks frame0: So on loading So my doubt is how do I use the generated the |
|
It's too bad we never captured this information for the debugging docs. Do you see |
|
Yep the flags seem right. I ended up using WSL to step through the code and verify changes on powershell :). We'll get back to windows debug someday and document it. Somehow the problem is now in numpy/numpy/core/src/umath/scalarmath.c.src Lines 191 to 194 in 369a677 Which is weird cause the problem should have always been here right? Anyways it does fix the issue(take this with a pinch of salt, as I've said this 3 times with the issue transferring to a different file). Now interesting thing is, >>> np.iinfo(np.int64).min // np.int64(-1)
<stdin>:1: RuntimeWarning: overflow encountered in longlong_scalars
-9223372036854775808WSL: >>> np.iinfo(np.int64).min // np.int64(-1)
fish: 'python3' terminated by signal SIGFPE (Floating point exception)Linux: I need to reboot, will update here when I boot into it. Now there are more than 2 places where we do unchecked |
|
This is super interesting. TIL moment if I'm being honest: This is in while below is in I'm basically able to repro the issues consistently on Linux, albeit only on 64 bits. Will fix them soon and add UT. |
|
I think this was covered by gh-21507, so closing, thanks. |
Integer overflow in remainder for Windows
%operator is used between MIN value of int and -1TODO:
Note: I have not committed the actual fix yet, I want to confirm if the crash is reproducible in Azure Pipelines
Local Reproduction
resolves #19246