Skip to content

gh-120010: fix invalid (nan+nanj) results in _Py_c_prod() #120287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Dec 6, 2024
17 changes: 17 additions & 0 deletions Lib/test/test_complex.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,22 @@ def test_mul(self):
self.assertRaises(TypeError, operator.mul, 1j, None)
self.assertRaises(TypeError, operator.mul, None, 1j)

for z, w, r in [(1e300+1j, complex(INF, INF), complex(NAN, INF)),
(1e300+1j, complex(NAN, INF), complex(-INF, INF)),
(1e300+1j, complex(INF, NAN), complex(INF, INF)),
(complex(INF, 1), complex(NAN, INF), complex(NAN, INF)),
(complex(INF, 1), complex(INF, NAN), complex(INF, NAN)),
(complex(NAN, 1), complex(1, INF), complex(-INF, NAN)),
(complex(1, NAN), complex(1, INF), complex(NAN, INF)),
(complex(1e200, NAN), complex(1e200, NAN), complex(INF, NAN)),
(complex(1e200, NAN), complex(NAN, 1e200), complex(NAN, INF)),
(complex(NAN, 1e200), complex(1e200, NAN), complex(NAN, INF)),
(complex(NAN, 1e200), complex(NAN, 1e200), complex(-INF, NAN)),
(complex(NAN, NAN), complex(NAN, NAN), complex(NAN, NAN))]:
with self.subTest(z=z, w=w, r=r):
self.assertComplexesAreIdentical(z * w, r)
self.assertComplexesAreIdentical(w * z, r)

def test_mod(self):
# % is no longer supported on complex numbers
with self.assertRaises(TypeError):
Expand Down Expand Up @@ -340,6 +356,7 @@ def test_pow(self):
self.assertAlmostEqual(pow(1j, 200), 1)
self.assertRaises(ValueError, pow, 1+1j, 1+1j, 1+1j)
self.assertRaises(OverflowError, pow, 1e200+1j, 1e200+1j)
self.assertRaises(OverflowError, pow, 1e200+1j, 5)
self.assertRaises(TypeError, pow, 1j, None)
self.assertRaises(TypeError, pow, None, 1j)
self.assertAlmostEqual(pow(1j, 0.5), 0.7071067811865476+0.7071067811865475j)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Correct invalid corner cases which resulted in ``(nan+nanj)`` output in complex
multiplication, e.g., ``(1e300+1j)*(nan+infj)``. Patch by Sergey B Kirpichev.
60 changes: 56 additions & 4 deletions Objects/complexobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,63 @@ _Py_c_neg(Py_complex a)
}

Py_complex
_Py_c_prod(Py_complex a, Py_complex b)
_Py_c_prod(Py_complex z, Py_complex w)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ok now. This doesn't require a mixed-mode variant.

Yet, maybe I should change variable names back for consistency with the rest? Current naming here follows to the C standard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serhiy-storchaka what do you think about renaming of arguments?

I think mapping local variables "abcd" -> "tuvw" will hot hurt readability and then we can preserve original argument names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with both variants. Preserving names from the old code can make the diff smaller, but in this case the new code is too different from the old code. Using names from the C standard examples helps comparing our code with the original code. The latter looks more important here.

{
Py_complex r;
r.real = a.real*b.real - a.imag*b.imag;
r.imag = a.real*b.imag + a.imag*b.real;
double a = z.real, b = z.imag, c = w.real, d = w.imag;
double ac = a*c, bd = b*d, ad = a*d, bc = b*c;
Py_complex r = {ac - bd, ad + bc};

/* Recover infinities that computed as nan+nanj. See e.g. the C11,
Annex G.5.1, routine _Cmultd(). */
if (isnan(r.real) && isnan(r.imag)) {
int recalc = 0;

if (isinf(a) || isinf(b)) { /* z is infinite */
/* "Box" the infinity and change nans in the other factor to 0 */
a = copysign(isinf(a) ? 1.0 : 0.0, a);
b = copysign(isinf(b) ? 1.0 : 0.0, b);
if (isnan(c)) {
c = copysign(0.0, c);
}
if (isnan(d)) {
d = copysign(0.0, d);
}
recalc = 1;
}
if (isinf(c) || isinf(d)) { /* w is infinite */
/* "Box" the infinity and change nans in the other factor to 0 */
c = copysign(isinf(c) ? 1.0 : 0.0, c);
d = copysign(isinf(d) ? 1.0 : 0.0, d);
if (isnan(a)) {
a = copysign(0.0, a);
}
if (isnan(b)) {
b = copysign(0.0, b);
}
recalc = 1;
}
if (!recalc && (isinf(ac) || isinf(bd) || isinf(ad) || isinf(bc))) {
/* Recover infinities from overflow by changing nans to 0 */
if (isnan(a)) {
a = copysign(0.0, a);
}
if (isnan(b)) {
b = copysign(0.0, b);
}
if (isnan(c)) {
c = copysign(0.0, c);
}
if (isnan(d)) {
d = copysign(0.0, d);
}
recalc = 1;
}
if (recalc) {
r.real = Py_INFINITY*(a*c - b*d);
r.imag = Py_INFINITY*(a*d + b*c);
}
}

return r;
}

Expand Down
Loading