From 68b2e428811d31783f70c9e051064de24f7bcbac Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 23 Feb 2023 07:32:22 +0300 Subject: [PATCH 1/8] gh-102837: few coverage nitpicks for the math module - input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071), hypot(L2682), log(L2307), ldexp(L2168) and dist(L2587,L2588,L2628). - rewrite math_floor like math_ceil (cover L1239) - drop inaccessible "if" branch (L3518) in perm_comb_small() - improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497), ditto fmod(L2378) - rewrite modf to fix inaccessible case(L2229), ditto for pow(L2988) (all line numbers wrt the main branch at 5e6661b) --- Lib/test/test_math.py | 25 +++++++++++++++++++++++++ Modules/mathmodule.c | 31 ++++++++++--------------------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index 433161c2dd4145..60aa74abcc756f 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -324,6 +324,7 @@ def testAtan2(self): self.ftest('atan2(0, 1)', math.atan2(0, 1), 0) self.ftest('atan2(1, 1)', math.atan2(1, 1), math.pi/4) self.ftest('atan2(1, 0)', math.atan2(1, 0), math.pi/2) + self.ftest('atan2(1, -1)', math.atan2(1, -1), 3*math.pi/4) # math.atan2(0, x) self.ftest('atan2(0., -inf)', math.atan2(0., NINF), math.pi) @@ -598,6 +599,7 @@ def testFmod(self): self.assertEqual(math.fmod(-3.0, NINF), -3.0) self.assertEqual(math.fmod(0.0, 3.0), 0.0) self.assertEqual(math.fmod(0.0, NINF), 0.0) + self.assertRaises(ValueError, math.fmod, INF, INF) def testFrexp(self): self.assertRaises(TypeError, math.frexp) @@ -714,6 +716,11 @@ def msum(iterable): s = msum(vals) self.assertEqual(msum(vals), math.fsum(vals)) + self.assertEqual(math.fsum([1.0, math.inf]), math.inf) + self.assertRaises(OverflowError, math.fsum, [1e+308, 1e+308]) + self.assertRaises(ValueError, math.fsum, [math.inf, -math.inf]) + self.assertRaises(TypeError, math.fsum, ['spam']) + def testGcd(self): gcd = math.gcd self.assertEqual(gcd(0, 0), 0) @@ -831,6 +838,8 @@ def testHypot(self): scale = FLOAT_MIN / 2.0 ** exp self.assertEqual(math.hypot(4*scale, 3*scale), 5*scale) + self.assertRaises(TypeError, math.hypot, *([1.0]*18), 'spam') + @requires_IEEE_754 @unittest.skipIf(HAVE_DOUBLE_ROUNDING, "hypot() loses accuracy on machines with double rounding") @@ -966,6 +975,8 @@ class T(tuple): dist((1, 2, 3, 4), (5, 6, 7)) with self.assertRaises(ValueError): # Check dimension agree dist((1, 2, 3), (4, 5, 6, 7)) + with self.assertRaises(TypeError): + dist((1,)*17 + ("spam",), (1,)*18) with self.assertRaises(TypeError): # Rejects invalid types dist("abc", "xyz") int_too_big_for_float = 10 ** (sys.float_info.max_10_exp + 5) @@ -973,6 +984,10 @@ class T(tuple): dist((1, int_too_big_for_float), (2, 3)) with self.assertRaises((ValueError, OverflowError)): dist((2, 3), (1, int_too_big_for_float)) + with self.assertRaises(TypeError): + dist((1,), 2) + with self.assertRaises(TypeError): + dist([1], 2) # Verify that the one dimensional case is equivalent to abs() for i in range(20): @@ -1111,6 +1126,7 @@ def test_lcm(self): def testLdexp(self): self.assertRaises(TypeError, math.ldexp) + self.assertRaises(TypeError, math.ldexp, 2.0, 1.1) self.ftest('ldexp(0,1)', math.ldexp(0,1), 0) self.ftest('ldexp(1,1)', math.ldexp(1,1), 2) self.ftest('ldexp(1,-1)', math.ldexp(1,-1), 0.5) @@ -1153,6 +1169,7 @@ def testLog(self): 2302.5850929940457) self.assertRaises(ValueError, math.log, -1.5) self.assertRaises(ValueError, math.log, -10**1000) + self.assertRaises(ValueError, math.log, 10, -10) self.assertRaises(ValueError, math.log, NINF) self.assertEqual(math.log(INF), INF) self.assertTrue(math.isnan(math.log(NAN))) @@ -2364,6 +2381,14 @@ def __float__(self): # argument to a float. self.assertFalse(getattr(y, "converted", False)) + def test_input_exceptions(self): + self.assertRaises(TypeError, math.exp, "spam") + self.assertRaises(TypeError, math.erf, "spam") + self.assertRaises(TypeError, math.atan2, "spam", 1.0) + self.assertRaises(TypeError, math.atan2, 1.0, "spam") + self.assertRaises(TypeError, math.atan2, 1.0) + self.assertRaises(TypeError, math.atan2, 1.0, 2.0, 3.0) + # Custom assertions. def assertIsNaN(self, value): diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index c9a2be66863993..cf6b9ca587d15d 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -1219,13 +1219,7 @@ static PyObject * math_floor(PyObject *module, PyObject *number) /*[clinic end generated code: output=c6a65c4884884b8a input=63af6b5d7ebcc3d6]*/ { - double x; - - if (PyFloat_CheckExact(number)) { - x = PyFloat_AS_DOUBLE(number); - } - else - { + if (!PyFloat_CheckExact(number)) { math_module_state *state = get_math_module_state(module); PyObject *method = _PyObject_LookupSpecial(number, state->str___floor__); if (method != NULL) { @@ -1235,10 +1229,11 @@ math_floor(PyObject *module, PyObject *number) } if (PyErr_Occurred()) return NULL; - x = PyFloat_AsDouble(number); - if (x == -1.0 && PyErr_Occurred()) - return NULL; } + double x = PyFloat_AsDouble(number); + if (x == -1.0 && PyErr_Occurred()) + return NULL; + return PyLong_FromDouble(floor(x)); } @@ -2223,12 +2218,10 @@ math_modf_impl(PyObject *module, double x) double y; /* some platforms don't do the right thing for NaNs and infinities, so we take care of special cases directly. */ - if (!Py_IS_FINITE(x)) { - if (Py_IS_INFINITY(x)) - return Py_BuildValue("(dd)", copysign(0., x), x); - else if (Py_IS_NAN(x)) - return Py_BuildValue("(dd)", x, x); - } + if (Py_IS_INFINITY(x)) + return Py_BuildValue("(dd)", copysign(0., x), x); + else if (Py_IS_NAN(x)) + return Py_BuildValue("(dd)", x, x); errno = 0; x = modf(x, &y); @@ -2985,7 +2978,7 @@ math_pow_impl(PyObject *module, double x, double y) else /* y < 0. */ r = odd_y ? copysign(0., x) : 0.; } - else if (Py_IS_INFINITY(y)) { + else { /* Py_IS_INFINITY(y) */ if (fabs(x) == 1.0) r = 1.; else if (y > 0. && fabs(x) > 1.0) @@ -3515,10 +3508,6 @@ static const uint8_t factorial_trailing_zeros[] = { static PyObject * perm_comb_small(unsigned long long n, unsigned long long k, int iscomb) { - if (k == 0) { - return PyLong_FromLong(1); - } - /* For small enough n and k the result fits in the 64-bit range and can * be calculated without allocating intermediate PyLong objects. */ if (iscomb) { From 9c32ae022f07ea90892175a2d7f5c777621be0e0 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Fri, 7 Apr 2023 15:54:42 +0300 Subject: [PATCH 2/8] Simplify math_1, like other functions Now exception messages kept in is_error(). This improve coverage for L1007. --- Modules/mathmodule.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 9c28179523e506..6ae24e80d64185 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -990,24 +990,16 @@ math_1(PyObject *arg, double (*func) (double), int can_overflow) return NULL; errno = 0; r = (*func)(x); - if (Py_IS_NAN(r) && !Py_IS_NAN(x)) { - PyErr_SetString(PyExc_ValueError, - "math domain error"); /* invalid arg */ - return NULL; - } + if (Py_IS_NAN(r) && !Py_IS_NAN(x)) + errno = EDOM; if (Py_IS_INFINITY(r) && Py_IS_FINITE(x)) { if (can_overflow) - PyErr_SetString(PyExc_OverflowError, - "math range error"); /* overflow */ + errno = ERANGE; else - PyErr_SetString(PyExc_ValueError, - "math domain error"); /* singularity */ - return NULL; + errno = EDOM; } - if (Py_IS_FINITE(r) && errno && is_error(r)) - /* this branch unnecessary on most platforms */ + if (errno && is_error(r)) return NULL; - return PyFloat_FromDouble(r); } From a0c619b98c90cf5b7168d5b5a7df2d3c2e9d8c95 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 2 Sep 2023 05:22:34 +0300 Subject: [PATCH 3/8] Add asserts --- Modules/mathmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 92621aed859fff..971dda682aa1ec 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -2935,7 +2935,8 @@ math_pow_impl(PyObject *module, double x, double y) else /* y < 0. */ r = odd_y ? copysign(0., x) : 0.; } - else { /* Py_IS_INFINITY(y) */ + else { + assert(Py_IS_INFINITY(y)); if (fabs(x) == 1.0) r = 1.; else if (y > 0. && fabs(x) > 1.0) @@ -3465,6 +3466,8 @@ static const uint8_t factorial_trailing_zeros[] = { static PyObject * perm_comb_small(unsigned long long n, unsigned long long k, int iscomb) { + assert(k != 0); + /* For small enough n and k the result fits in the 64-bit range and can * be calculated without allocating intermediate PyLong objects. */ if (iscomb) { From b991b7c327af625d063449b62c34fdd289b194a4 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 2 Sep 2023 07:03:12 +0300 Subject: [PATCH 4/8] Amend 9c32ae022f (comment restored) --- Modules/mathmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 971dda682aa1ec..9b6771cb607836 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -971,6 +971,7 @@ math_1(PyObject *arg, double (*func) (double), int can_overflow) errno = EDOM; } if (errno && is_error(r)) + /* this branch unnecessary on most platforms */ return NULL; return PyFloat_FromDouble(r); } From 828ba755e23d3a6bf78a719ef1670cd8747dccc1 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 2 Sep 2023 11:39:36 +0300 Subject: [PATCH 5/8] Revert back math_floor(), add tests to cover L1239 (same for ceil()) --- Lib/test/test_math.py | 6 ++++++ Modules/mathmodule.c | 15 ++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index a60445280607d8..ed84d8c14a640f 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -428,6 +428,9 @@ class TestNoCeil: self.assertRaises(TypeError, math.ceil, t) self.assertRaises(TypeError, math.ceil, t, 0) + self.assertEqual(math.ceil(FloatLike(+1.0)), +1.0) + self.assertEqual(math.ceil(FloatLike(-1.0)), -1.0) + @requires_IEEE_754 def testCopysign(self): self.assertEqual(math.copysign(1, 42), 1.0) @@ -578,6 +581,9 @@ class TestNoFloor: self.assertRaises(TypeError, math.floor, t) self.assertRaises(TypeError, math.floor, t, 0) + self.assertEqual(math.floor(FloatLike(+1.0)), +1.0) + self.assertEqual(math.floor(FloatLike(-1.0)), -1.0) + def testFmod(self): self.assertRaises(TypeError, math.fmod) self.ftest('fmod(10, 1)', math.fmod(10, 1), 0.0) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 9b6771cb607836..77ef9a6dfb3126 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -1184,7 +1184,13 @@ static PyObject * math_floor(PyObject *module, PyObject *number) /*[clinic end generated code: output=c6a65c4884884b8a input=63af6b5d7ebcc3d6]*/ { - if (!PyFloat_CheckExact(number)) { + double x; + + if (PyFloat_CheckExact(number)) { + x = PyFloat_AS_DOUBLE(number); + } + else + { math_module_state *state = get_math_module_state(module); PyObject *method = _PyObject_LookupSpecial(number, state->str___floor__); if (method != NULL) { @@ -1194,11 +1200,10 @@ math_floor(PyObject *module, PyObject *number) } if (PyErr_Occurred()) return NULL; + x = PyFloat_AsDouble(number); + if (x == -1.0 && PyErr_Occurred()) + return NULL; } - double x = PyFloat_AsDouble(number); - if (x == -1.0 && PyErr_Occurred()) - return NULL; - return PyLong_FromDouble(floor(x)); } From 6498162bdf0944d3eb16e9d7e13845b31275d2e5 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 2 Sep 2023 12:10:15 +0300 Subject: [PATCH 6/8] Tests for bad __floor__/__ceil__ descriptors (cover L1165, L1236) --- Lib/test/test_math.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index ed84d8c14a640f..3cca501159ee65 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -418,10 +418,16 @@ def __ceil__(self): return 42 class TestNoCeil: pass + class TestBadCeil: + class BadCeil: + def __get__(self, obj, objtype=None): + raise ValueError + __ceil__ = BadCeil() self.assertEqual(math.ceil(TestCeil()), 42) self.assertEqual(math.ceil(FloatCeil()), 42) self.assertEqual(math.ceil(FloatLike(42.5)), 43) self.assertRaises(TypeError, math.ceil, TestNoCeil()) + self.assertRaises(ValueError, math.ceil, TestBadCeil()) t = TestNoCeil() t.__ceil__ = lambda *args: args @@ -571,10 +577,16 @@ def __floor__(self): return 42 class TestNoFloor: pass + class TestBadFloor: + class BadFloor: + def __get__(self, obj, objtype=None): + raise ValueError + __floor__ = BadFloor() self.assertEqual(math.floor(TestFloor()), 42) self.assertEqual(math.floor(FloatFloor()), 42) self.assertEqual(math.floor(FloatLike(41.9)), 41) self.assertRaises(TypeError, math.floor, TestNoFloor()) + self.assertRaises(ValueError, math.floor, TestBadFloor()) t = TestNoFloor() t.__floor__ = lambda *args: args From 8fcca165e0c25949618fdbe148284066ba93cbca Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 3 Sep 2023 06:42:14 +0300 Subject: [PATCH 7/8] +1 --- Lib/test/test_math.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index 3cca501159ee65..b71d08b1124497 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -235,6 +235,10 @@ def __init__(self, value): def __index__(self): return self.value +class BadDescr: + def __get__(self, obj, objtype=None): + raise ValueError + class MathTests(unittest.TestCase): def ftest(self, name, got, expected, ulp_tol=5, abs_tol=0.0): @@ -419,10 +423,7 @@ def __ceil__(self): class TestNoCeil: pass class TestBadCeil: - class BadCeil: - def __get__(self, obj, objtype=None): - raise ValueError - __ceil__ = BadCeil() + __ceil__ = BadDescr() self.assertEqual(math.ceil(TestCeil()), 42) self.assertEqual(math.ceil(FloatCeil()), 42) self.assertEqual(math.ceil(FloatLike(42.5)), 43) @@ -578,10 +579,7 @@ def __floor__(self): class TestNoFloor: pass class TestBadFloor: - class BadFloor: - def __get__(self, obj, objtype=None): - raise ValueError - __floor__ = BadFloor() + __floor__ = BadDescr() self.assertEqual(math.floor(TestFloor()), 42) self.assertEqual(math.floor(FloatFloor()), 42) self.assertEqual(math.floor(FloatLike(41.9)), 41) From 796bdca44c9662d27353e7281bd7ecf6a5f9e0e0 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 3 Sep 2023 06:56:20 +0300 Subject: [PATCH 8/8] Revert "Simplify math_1, like other functions" This reverts commit 9c32ae022f07ea90892175a2d7f5c777621be0e0. --- Modules/mathmodule.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 77ef9a6dfb3126..2d896e7fe333a4 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -962,17 +962,24 @@ math_1(PyObject *arg, double (*func) (double), int can_overflow) return NULL; errno = 0; r = (*func)(x); - if (Py_IS_NAN(r) && !Py_IS_NAN(x)) - errno = EDOM; + if (Py_IS_NAN(r) && !Py_IS_NAN(x)) { + PyErr_SetString(PyExc_ValueError, + "math domain error"); /* invalid arg */ + return NULL; + } if (Py_IS_INFINITY(r) && Py_IS_FINITE(x)) { if (can_overflow) - errno = ERANGE; + PyErr_SetString(PyExc_OverflowError, + "math range error"); /* overflow */ else - errno = EDOM; + PyErr_SetString(PyExc_ValueError, + "math domain error"); /* singularity */ + return NULL; } - if (errno && is_error(r)) + if (Py_IS_FINITE(r) && errno && is_error(r)) /* this branch unnecessary on most platforms */ return NULL; + return PyFloat_FromDouble(r); }