bpo-34373: Fix time.mktime() on AIX #12726

vstinner · 2019-04-08T12:39:52Z

Fix time.mktime() on AIX: fix code checking for mktime() failure
for years before 1970.
mktime(): rename variable 'buf' to 'tm'.
_PyTime_localtime(): avoid abs() which is limited to int type,
use "localtime" rather than "ctime" in the error message.
_PyTime_localtime(): EINVAL constant is now always available.
_PyTime_localtime(): initialize errno to 0 just in case if
localtime_r() doesn't set errno on error.

https://bugs.python.org/issue34373

vstinner · 2019-04-08T12:40:14Z

Fix time.mktime() error handling on AIX for year before 1970. Other changes: * mktime(): rename variable 'buf' to 'tm'. * _PyTime_localtime(): * Use "localtime" rather than "ctime" in the error message (specific to AIX). * Always initialize errno to 0 just in case if localtime_r() doesn't set errno on error. * On AIX, avoid abs() which is limited to int type. * EINVAL constant is now always available.

native-api · 2019-04-08T13:40:59Z

Modules/timemodule.c

    {
        PyErr_SetString(PyExc_OverflowError,
                        "mktime argument out of range");
        return NULL;
    }
+
+#ifdef _AIX


The purpose of this chunk is unclear.
Probably need to comment how exactly AIX is faulty like in the others.

The issue AIX mktime() cannot handle on it's own - is that mktime() does not accept a tm structure when the value for tm_year is < 70.

The new version has an _AIX specific block before the call to mktime() (to add years to tm_year until it is >= 70 and remember this 'delta' in number of days per 4 years including leap-day.

Once adjusted (only when tm_year is < 70) the code is "normal". If mktime() fails, a failure is returned.

If mktime() was successful - and _AIX and delta_days != 0 then the structure adjusted by mktime() needs to be adjusted further (back in time):

the original year is restored

the tm_wday is adjusted according to delta_days

the number of seconds (tt) is adjusted according to delta_days.

Hope this helps.

vstinner · 2019-04-09T10:32:38Z

@native-api: I added more comments. Is it better now?

@aixtools: Would you mind to review again my change please? (review the comments)

vstinner · 2019-04-09T17:12:09Z

This change also fix this compiler warning:

./Modules/timemodule.c:999:13: warning: 'buf.tm_year' is used uninitialized in this function [-Wuninitialized]

https://buildbot.python.org/all/#/builders/10/builds/2358

native-api · 2019-04-09T20:37:39Z

Wow. So my word can actually influence something here.
Seeing the "awaiting core review" label, I thought it would be treated as a passing remark at best.
I'll put more thought into it from now on now that I see that others actually risk relying on it!

vstinner · 2019-04-10T23:35:52Z

Don't worry. If anything goes wrong, it will be all my fault and I can handle it :-)

EGuesnet · 2020-01-27T14:08:50Z

Hi,
I have an error during regression tests with Python3.8.1 on AIX 6.1, and it seems related to this PR.
It occurs only on 64 bit. Test passes on 32 bit.

======================================================================
ERROR: test_add_file_after_2107 (test.test_zipfile.StoredTestsWithSourceFile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/test/test_zipfile.py", line 606, in test_add_file_after_2107
    self.assertRaises(struct.error, zipfp.write, TESTFN)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 816, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 202, in handle
    callable_obj(*args, **kwargs)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 1739, in write
    zinfo = ZipInfo.from_file(filename, arcname,
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 523, in from_file
    mtime = time.localtime(st.st_mtime)
OverflowError: localtime argument out of range

Is the code 32 bit only, or maybe the test was not updated?

I can reproduce the behavior as follow:

$ python3.8_32
Python 3.8.1 (default, Jan 27 2020, 11:34:59) 
[GCC 8.3.0] on aix
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: timestamp out of range for platform time_t

$ python3.8_64
Python 3.8.1 (default, Jan 27 2020, 11:30:15) 
[GCC 8.3.0] on aix
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: localtime argument out of range

$ python3.7_32
Python 3.7.4 (default, Jan 15 2020, 15:50:53) 
[GCC 8.3.0] on aix6
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: timestamp out of range for platform time_t

$ python3.7_64
Python 3.7.4 (default, Jan 15 2020, 15:46:22) 
[GCC 8.3.0] on aix6
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
time.struct_time(tm_year=2107, tm_mon=1, tm_mday=27, tm_hour=10, tm_min=7, tm_sec=32, tm_wday=3, tm_yday=27, tm_isdst=0)

Before the PR, localtime after 2038 worked on 64 bit.

aixtools · 2020-01-30T11:49:31Z

Been awhile since I worked on this. From memory, the (initial) patch was only meant to affect the 32-bit build - as 64-bit did pass, iirc. Although, I do not recall ever encountering a failure with test_zipfile.

…

On 27/01/2020 15:08, EGuesnet wrote: Hi, I have an error during regression tests with Python3.8.1 on AIX 6.1, and it seems related to this PR. It occurs only on 64 bit. Test passes on 32 bit. ====================================================================== ERROR: test_add_file_after_2107 (test.test_zipfile.StoredTestsWithSourceFile) ---------------------------------------------------------------------- Traceback (most recent call last): File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/test/test_zipfile.py", line 606,in test_add_file_after_2107 self.assertRaises(struct.error, zipfp.write, TESTFN) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 816,in assertRaises return context.handle('assertRaises', args, kwargs) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 202,in handle callable_obj(*args,**kwargs) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 1739,in write zinfo = ZipInfo.from_file(filename, arcname, File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 523,in from_file mtime = time.localtime(st.st_mtime) OverflowError: localtime argument out of range Is the code 32 bit only, or maybe the test was not updated? ------------------------------------------------------------------------ I can reproduce the behavior as follow: $ python3.8_32 Python 3.8.1 (default, Jan 27 2020, 11:34:59) [GCC 8.3.0] on aix Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: timestamp out of range for platform time_t $ python3.8_64 Python 3.8.1 (default, Jan 27 2020, 11:30:15) [GCC 8.3.0] on aix Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: localtime argument out of range $ python3.7_32 Python 3.7.4 (default, Jan 15 2020, 15:50:53) [GCC 8.3.0] on aix6 Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: timestamp out of range for platform time_t $ python3.7_64 Python 3.7.4 (default, Jan 15 2020, 15:46:22) [GCC 8.3.0] on aix6 Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) time.struct_time(tm_year=2107, tm_mon=1, tm_mday=27, tm_hour=10, tm_min=7, tm_sec=32, tm_wday=3, tm_yday=27, tm_isdst=0) Before the PR, localtime after 2038 worked on 64 bit. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12726?email_source=notifications&email_token=ACSZR5MN2RWVMC3IL2ENNNDQ73THJA5CNFSM4HEIKDVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ7TQGI#issuecomment-578762777>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACSZR5N3LUXRAUMYDXLKRGLQ73THJANCNFSM4HEIKDVA>.

vstinner · 2020-01-30T12:01:22Z

I have an error during regression tests with Python3.8.1 on AIX 6.1

This PR is closed. Please open a new issue at https://bugs.python.org/

the-knights-who-say-ni added the CLA signed label Apr 8, 2019

bedevere-bot added the awaiting core review label Apr 8, 2019

This was referenced Apr 8, 2019

bpo-34373: fix test_mktime and test_pthread_getcpuclickid tests on AIX #8726

Merged

bpo-31904: Add time module support for VxWorks RTOS #12305

Merged

native-api reviewed Apr 8, 2019

View reviewed changes

Add more comments on AIX mktime()

aaef958

native-api approved these changes Apr 9, 2019

View reviewed changes

vstinner merged commit 8709490 into python:master Apr 9, 2019

bedevere-bot removed the awaiting core review label Apr 9, 2019

vstinner deleted the mktime_aix branch April 9, 2019 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

bpo-34373: Fix time.mktime() on AIX #12726

bpo-34373: Fix time.mktime() on AIX #12726

Uh oh!

vstinner commented Apr 8, 2019 •

edited by bedevere-bot

Loading

Uh oh!

vstinner commented Apr 8, 2019

Uh oh!

native-api Apr 8, 2019

Uh oh!

aixtools Apr 8, 2019 •

edited

Loading

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

native-api commented Apr 9, 2019

Uh oh!

vstinner commented Apr 10, 2019

Uh oh!

EGuesnet commented Jan 27, 2020

Uh oh!

aixtools commented Jan 30, 2020 via email

Uh oh!

vstinner commented Jan 30, 2020

Uh oh!

Uh oh!

Uh oh!

bpo-34373: Fix time.mktime() on AIX #12726

bpo-34373: Fix time.mktime() on AIX #12726

Uh oh!

Conversation

vstinner commented Apr 8, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Apr 8, 2019

Uh oh!

native-api Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

aixtools Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

native-api commented Apr 9, 2019

Uh oh!

vstinner commented Apr 10, 2019

Uh oh!

EGuesnet commented Jan 27, 2020

Uh oh!

aixtools commented Jan 30, 2020 via email

Uh oh!

vstinner commented Jan 30, 2020

Uh oh!

Uh oh!

vstinner commented Apr 8, 2019 •

edited by bedevere-bot

Loading

aixtools Apr 8, 2019 •

edited

Loading