test_time fails: strftime('%Y', y) for negative year

[florentx]

BPO	13312
Nosy	@gpshead, @abalkin, @vstinner, @jwilk, @florentx, @vadmium, @miss-islington
PRs	bpo-13312: Avoid int underflow in time year. #8912 [3.7] bpo-13312: Avoid int underflow in time year. (GH-8912) #8913 [3.6] bpo-13312: Avoid int underflow in time year. (GH-8912) #8914
Files	issue13312.patch issue13312.v2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2018-08-25.05:54:42.870>
created_at = <Date 2011-11-01.16:52:38.444>
labels = ['3.7', '3.8', 'type-bug', 'tests']
title = "test_time fails: strftime('%Y', y) for negative year"
updated_at = <Date 2018-08-27.13:38:03.330>
user = 'https://github.com/florentx'

bugs.python.org fields:

activity = <Date 2018-08-27.13:38:03.330>
actor = 'vstinner'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2018-08-25.05:54:42.870>
closer = 'gregory.p.smith'
components = ['Tests']
creation = <Date 2011-11-01.16:52:38.444>
creator = 'flox'
dependencies = []
files = ['38290', '43709']
hgrepos = []
issue_num = 13312
keywords = ['patch', 'buildbot']
message_count = 19.0
messages = ['146793', '146818', '146822', '146828', '146829', '146830', '220505', '236981', '236985', '236988', '236989', '236993', '236994', '270371', '324026', '324038', '324039', '324040', '324177']
nosy_count = 8.0
nosy_names = ['gregory.p.smith', 'belopolsky', 'vstinner', 'jwilk', 'flox', 'python-dev', 'martin.panter', 'miss-islington']
pr_nums = ['8912', '8913', '8914']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13312'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[florentx]

On builder "AMD64 FreeBSD 8.2 3.x" for the TIME_MINYEAR:

======================================================================
FAIL: test_negative (test.test_time.TestStrftime4dyear)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/test_time.py", line 397, in test_negative
    return super().test_negative()
  File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/test_time.py", line 425, in test_negative
    self.assertEqual(self.yearstr(TIME_MINYEAR), str(TIME_MINYEAR))
AssertionError: '2147483648' != '-2147483648'
- 2147483648
+ -2147483648
? +

[python-dev]

New changeset 9cb1b85237a9 by Florent Xicluna in branch 'default':
Issue bpo-13312: skip the single failing value for now.
http://hg.python.org/cpython/rev/9cb1b85237a9

[python-dev]

New changeset d877d7f3b679 by Florent Xicluna in branch 'default':
Actually, there's more than one failing value. (changeset 9cb1b85237a9, issue bpo-13312).
http://hg.python.org/cpython/rev/d877d7f3b679

[florentx]

It fails for very low negative years:
-2147483648 <= year < -2147481748
(-1<<31) <= year < (-1<<31) + 1900

Every other value behaves correctly on this FreeBSD buildbot:

• (-1<<31) + 1900 <= year < (+1<<31) : correctly formatted with '%Z'
• year < (-1<<31) : raises OverfowError
• year >= (+1<<31) : raises OverfowError

[florentx]

I mean formatted with '%Y', of course.

[python-dev]

New changeset 1a0bfc26af57 by Florent Xicluna in branch 'default':
Issue bpo-13312: skip the failing negative years for now.
http://hg.python.org/cpython/rev/1a0bfc26af57

[BreamoreBoy]

The failing negative years test is still being skipped. I'm assuming this was not originally intended.

[BreamoreBoy]

I believe that this can be closed as the test code was changed completely in bpo-17960.

[abalkin]

Mark,

Issue bpo-17960 ("Clarify the required behaviour of locals()") does not seem to be relevant here. I think you meant to refer to a changeset, not issue number. If so please use hash number such as d877d7f3b679.

[abalkin]

As far as I can tell, the original report was about a test failing due to a system-dependent behavior of time.asctime(). However, since changeset 1e62a0cee092 (see issue bpo-8013), we no longer call system asctime. I believe the test disabled in 1a0bfc26af57 can now be restored.

[BreamoreBoy]

Sorry should have been bpo-17690.

[abalkin]

We still have the following in Lib/test/test_time.py:

        # Issue python/cpython#57521: it may return wrong value for year < TIME_MINYEAR + 1900
        # Skip the value test, but check that no error is raised
        self.yearstr(TIME_MINYEAR)

I reviewed the current time.asctime() code and it does look like it invokes undefined behavior for extremely large negative years.

The problem is that in C struct tm, year is stored as year - 1900 and for year < -2147481748 (= -2**31 + 1900) we trigger an overflow of a signed integer.

Can someone confirm that on AMD64 FreeBSD subtracting 1900 from -2147483648 and then adding it back does not give -2147483648?

[abalkin]

Attached patch eliminates undefined behavior, but I am not sure fixing this is worth the trouble.

[vadmium]

If you enable GCC’s -ftrapv option, the subtraction overflow triggers an abort. Alexander’s patch works around the problem for asctime(), but the problem still exists in other cases, such as:

>>> time.mktime((-2**31 + 1899, *(0,) * 8))
Aborted (core dumped)
[Exit 134]

Attaching a version of the patch without the conflicting whitespace changes.

Why does Python even need to support such extreme time values? It would seem much simpler to raise an exception.

[gpshead]

New changeset 76be0ff by Gregory P. Smith in branch 'master':
bpo-13312: Avoid int underflow in time year. (GH-8912)
76be0ff

[miss-islington]

New changeset d5f017b by Miss Islington (bot) in branch '3.7':
bpo-13312: Avoid int underflow in time year. (GH-8912)
d5f017b

[miss-islington]

New changeset c47acc2 by Miss Islington (bot) in branch '3.6':
bpo-13312: Avoid int underflow in time year. (GH-8912)
c47acc2

[gpshead]

OverflowError is now raised for negative values that would trigger a problem and the unittest has been updated to test this.

[vstinner]

Note for myself: Python 2.7 is not affected by this bug because it doesn't accept year < 1900.