From 733bf32fd65aee9278c65a50f5e05071b15a02bd Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Fri, 10 Aug 2018 16:21:14 +0000 Subject: [PATCH 1/7] fix test_mktime and test_pthread_getcpuclickid tests add range checking for _PyTime_localtime (for AIX) --- Lib/test/test_time.py | 9 ++++- .../2018-08-10-16-17-51.bpo-34373.SKdb1k.rst | 2 + Modules/timemodule.c | 39 ++++++++++++++----- Python/pytime.c | 16 ++++++++ 4 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index 7354b969907b16..7660194d11ffe9 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -124,9 +124,16 @@ def test_clock_monotonic(self): @unittest.skipUnless(hasattr(time, 'clock_gettime'), 'need time.clock_gettime()') def test_pthread_getcpuclockid(self): + from ctypes import c_void_p, sizeof clk_id = time.pthread_getcpuclockid(threading.get_ident()) self.assertTrue(type(clk_id) is int) - self.assertNotEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) + # when in 32-bit mode AIX only returns the predefined constant + if not sys.platform.startswith("aix"): + self.assertNotEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) + elif ((sizeof(c_void_p) * 8) == 64): + self.assertNotEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) + else: + self.assertEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) t1 = time.clock_gettime(clk_id) t2 = time.clock_gettime(clk_id) self.assertLessEqual(t1, t2) diff --git a/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst b/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst new file mode 100644 index 00000000000000..11e4ab97c64936 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst @@ -0,0 +1,2 @@ +fix test_mktime and test_pthread_getcpuclickid tests +add range checking for _PyTime_localtime (for AIX) diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 998216cc8fdbdf..d5717b4d27c39b 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -174,10 +174,15 @@ static PyObject * time_clock_gettime(PyObject *self, PyObject *args) { int ret; - int clk_id; struct timespec tp; +#if defined(_AIX) && (SIZEOF_LONG == 8) + long clk_id; + if (!PyArg_ParseTuple(args, "l:clock_gettime", &clk_id)) { +#else + int clk_id; if (!PyArg_ParseTuple(args, "i:clock_gettime", &clk_id)) { +#endif return NULL; } @@ -947,28 +952,44 @@ time_mktime(PyObject *self, PyObject *tup) } #ifdef _AIX /* year < 1902 or year > 2037 */ - if (buf.tm_year < 2 || buf.tm_year > 137) { + if ((buf.tm_year < 2) || (buf.tm_year > 137)) { /* Issue #19748: On AIX, mktime() doesn't report overflow error for * timestamp < -2^31 or timestamp > 2**31-1. */ PyErr_SetString(PyExc_OverflowError, "mktime argument out of range"); return NULL; } + { + time_t clk; + int year = buf.tm_year; + int days = 0; + + /* year < 1970 */ + if (year < 70) do + { + buf.tm_year += 4; + days -= (366 + (365 * 3)); + } while (buf.tm_year < 70); + + buf.tm_wday = -1; + clk = mktime(&buf); + buf.tm_year = year; + + if (buf.tm_wday != -1 && days) + buf.tm_wday = (buf.tm_wday + days) % 7; + + tt = clk + (days * (24 * 3600)); + } #else buf.tm_wday = -1; /* sentinel; original value ignored */ -#endif tt = mktime(&buf); +#endif /* Return value of -1 does not necessarily mean an error, but tm_wday * cannot remain set to -1 if mktime succeeded. */ if (tt == (time_t)(-1) -#ifndef _AIX /* Return value of -1 does not necessarily mean an error, but * tm_wday cannot remain set to -1 if mktime succeeded. */ - && buf.tm_wday == -1 -#else - /* on AIX, tm_wday is always sets, even on error */ -#endif - ) + && buf.tm_wday == -1) { PyErr_SetString(PyExc_OverflowError, "mktime argument out of range"); diff --git a/Python/pytime.c b/Python/pytime.c index 0e9413174195d9..724806021e5059 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1062,6 +1062,22 @@ _PyTime_localtime(time_t t, struct tm *tm) } return 0; #else /* !MS_WINDOWS */ +#ifdef _AIX + /* + * AIX does not return NULL on an error + * so test ranges - asif! + * (1902-01-01, -2145916800.0) + * (2038-01-01, 2145916800.0) + */ + if (abs(t) > (time_t) 2145916800) { +#ifdef EINVAL + errno = EINVAL; +#endif + PyErr_SetString(PyExc_OverflowError, + "ctime argument out of range"); + return -1; + } +#endif if (localtime_r(&t, tm) == NULL) { #ifdef EINVAL if (errno == 0) { From 98c69ae60ca756df9f030bc2d583b37824079832 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Fri, 10 Aug 2018 18:01:15 +0000 Subject: [PATCH 2/7] patchcheck --- Modules/timemodule.c | 8 ++++---- Python/pytime.c | 18 ++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Modules/timemodule.c b/Modules/timemodule.c index d5717b4d27c39b..1d45f63d804caa 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -967,8 +967,8 @@ time_mktime(PyObject *self, PyObject *tup) /* year < 1970 */ if (year < 70) do { - buf.tm_year += 4; - days -= (366 + (365 * 3)); + buf.tm_year += 4; + days -= (366 + (365 * 3)); } while (buf.tm_year < 70); buf.tm_wday = -1; @@ -976,9 +976,9 @@ time_mktime(PyObject *self, PyObject *tup) buf.tm_year = year; if (buf.tm_wday != -1 && days) - buf.tm_wday = (buf.tm_wday + days) % 7; + buf.tm_wday = (buf.tm_wday + days) % 7; - tt = clk + (days * (24 * 3600)); + tt = clk + (days * (24 * 3600)); } #else buf.tm_wday = -1; /* sentinel; original value ignored */ diff --git a/Python/pytime.c b/Python/pytime.c index 724806021e5059..68c49a86da25b6 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1063,19 +1063,17 @@ _PyTime_localtime(time_t t, struct tm *tm) return 0; #else /* !MS_WINDOWS */ #ifdef _AIX - /* - * AIX does not return NULL on an error - * so test ranges - asif! - * (1902-01-01, -2145916800.0) - * (2038-01-01, 2145916800.0) - */ + /* AIX does not return NULL on an error + so test ranges - asif! + (1902-01-01, -2145916800.0) + (2038-01-01, 2145916800.0) */ if (abs(t) > (time_t) 2145916800) { #ifdef EINVAL - errno = EINVAL; + errno = EINVAL; #endif - PyErr_SetString(PyExc_OverflowError, - "ctime argument out of range"); - return -1; + PyErr_SetString(PyExc_OverflowError, + "ctime argument out of range"); + return -1; } #endif if (localtime_r(&t, tm) == NULL) { From ec59363ca1d78c3ef8fe046b9e72db4d7d122d0b Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Tue, 14 Aug 2018 21:19:51 +0000 Subject: [PATCH 3/7] format according to PEP7 --- .../2018-08-10-16-17-51.bpo-34373.SKdb1k.rst | 5 ++- Modules/timemodule.c | 43 +++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst b/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst index 11e4ab97c64936..1df5449eced63d 100644 --- a/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst +++ b/Misc/NEWS.d/next/Tests/2018-08-10-16-17-51.bpo-34373.SKdb1k.rst @@ -1,2 +1,3 @@ -fix test_mktime and test_pthread_getcpuclickid tests -add range checking for _PyTime_localtime (for AIX) +Fix ``test_mktime`` and ``test_pthread_getcpuclickid`` tests for AIX +Add range checking for ``_PyTime_localtime`` for AIX +Patch by Michael Felt diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 1d45f63d804caa..fb4680e377c901 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -945,12 +945,21 @@ time_mktime(PyObject *self, PyObject *tup) { struct tm buf; time_t tt; +#ifdef _AIX + time_t clk; + int year = buf.tm_year; + int days = 0; +#endif + if (!gettmarg(tup, &buf, "iiiiiiiii;mktime(): illegal time tuple argument")) { return NULL; } -#ifdef _AIX +#ifndef _AIX + buf.tm_wday = -1; /* sentinel; original value ignored */ + tt = mktime(&buf); +#else /* year < 1902 or year > 2037 */ if ((buf.tm_year < 2) || (buf.tm_year > 137)) { /* Issue #19748: On AIX, mktime() doesn't report overflow error for @@ -959,30 +968,20 @@ time_mktime(PyObject *self, PyObject *tup) "mktime argument out of range"); return NULL; } - { - time_t clk; - int year = buf.tm_year; - int days = 0; - - /* year < 1970 */ - if (year < 70) do - { - buf.tm_year += 4; - days -= (366 + (365 * 3)); - } while (buf.tm_year < 70); + /* year < 1970 */ + if (year < 70) do { + buf.tm_year += 4; + days -= (366 + (365 * 3)); + } while (buf.tm_year < 70); - buf.tm_wday = -1; - clk = mktime(&buf); - buf.tm_year = year; + buf.tm_wday = -1; + clk = mktime(&buf); + buf.tm_year = year; - if (buf.tm_wday != -1 && days) - buf.tm_wday = (buf.tm_wday + days) % 7; + if (buf.tm_wday != -1 && days) + buf.tm_wday = (buf.tm_wday + days) % 7; - tt = clk + (days * (24 * 3600)); - } -#else - buf.tm_wday = -1; /* sentinel; original value ignored */ - tt = mktime(&buf); + tt = clk + (days * (24 * 3600)); #endif /* Return value of -1 does not necessarily mean an error, but tm_wday * cannot remain set to -1 if mktime succeeded. */ From dd033c47204980c157458863872d90d20576b2cd Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Sun, 16 Sep 2018 19:46:07 +0000 Subject: [PATCH 4/7] fix pre-mature initialization of variable --- Modules/timemodule.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Modules/timemodule.c b/Modules/timemodule.c index fb4680e377c901..eeae3ca3884d90 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -948,7 +948,7 @@ time_mktime(PyObject *self, PyObject *tup) #ifdef _AIX time_t clk; int year = buf.tm_year; - int days = 0; + int delta_days = 0; #endif if (!gettmarg(tup, &buf, @@ -968,20 +968,21 @@ time_mktime(PyObject *self, PyObject *tup) "mktime argument out of range"); return NULL; } - /* year < 1970 */ - if (year < 70) do { + year = buf.tm_year; + /* year < 1970 - adjust buf.tm_year into legal range */ + while (buf.tm_year < 70) { buf.tm_year += 4; - days -= (366 + (365 * 3)); - } while (buf.tm_year < 70); + delta_days -= (366 + (365 * 3)); + } buf.tm_wday = -1; clk = mktime(&buf); buf.tm_year = year; - if (buf.tm_wday != -1 && days) - buf.tm_wday = (buf.tm_wday + days) % 7; + if (buf.tm_wday != -1 && delta_days) + buf.tm_wday = (buf.tm_wday + delta_days) % 7; - tt = clk + (days * (24 * 3600)); + tt = clk + (delta_days * (24 * 3600)); #endif /* Return value of -1 does not necessarily mean an error, but tm_wday * cannot remain set to -1 if mktime succeeded. */ From d57588362558a234330b768cd6f0bc22c51545a6 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Sun, 16 Sep 2018 19:46:38 +0000 Subject: [PATCH 5/7] Remove special condition for AIX --- Lib/test/test_time.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index 7660194d11ffe9..ee2906214cd0ef 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -438,13 +438,6 @@ def test_localtime_without_arg(self): def test_mktime(self): # Issue #1726687 for t in (-2, -1, 0, 1): - if sys.platform.startswith('aix') and t == -1: - # Issue #11188, #19748: mktime() returns -1 on error. On Linux, - # the tm_wday field is used as a sentinel () to detect if -1 is - # really an error or a valid timestamp. On AIX, tm_wday is - # unchanged even on success and so cannot be used as a - # sentinel. - continue try: tt = time.localtime(t) except (OverflowError, OSError): From 61d681050730d77630828467be14591f8ea7c725 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Sun, 16 Sep 2018 19:54:12 +0000 Subject: [PATCH 6/7] Add additional parentheses for test condition --- Modules/timemodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/timemodule.c b/Modules/timemodule.c index eeae3ca3884d90..31049ce4ef76bd 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -979,7 +979,7 @@ time_mktime(PyObject *self, PyObject *tup) clk = mktime(&buf); buf.tm_year = year; - if (buf.tm_wday != -1 && delta_days) + if ((buf.tm_wday != -1) && delta_days) buf.tm_wday = (buf.tm_wday + delta_days) % 7; tt = clk + (delta_days * (24 * 3600)); From 7a48ff327d2d7e18252dc8ea085f1183bba99d78 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Fri, 28 Dec 2018 10:49:28 +0000 Subject: [PATCH 7/7] remove need for ctypes in test --- Lib/test/test_time.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index ee2906214cd0ef..dfbe91e4e2f14d 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -124,13 +124,12 @@ def test_clock_monotonic(self): @unittest.skipUnless(hasattr(time, 'clock_gettime'), 'need time.clock_gettime()') def test_pthread_getcpuclockid(self): - from ctypes import c_void_p, sizeof clk_id = time.pthread_getcpuclockid(threading.get_ident()) self.assertTrue(type(clk_id) is int) # when in 32-bit mode AIX only returns the predefined constant - if not sys.platform.startswith("aix"): + if not platform.system() == "AIX": self.assertNotEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) - elif ((sizeof(c_void_p) * 8) == 64): + elif (sys.maxsize.bit_length() > 32): self.assertNotEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID) else: self.assertEqual(clk_id, time.CLOCK_THREAD_CPUTIME_ID)