[BUG] Access violation under Windows in test_chrono_system_clock_roundtrip_time

[ghost]

Issue description

Test case test_chrono_system_clock_roundtrip_time from test_chrono.py fails with access violation (null pointer dereference) when passing instances of datetime.time to m.test_chrono2()

pybind11/tests/test_chrono.py

Line 104 in 2e31e46

datetime2 = m.test_chrono2(time1)

Steps to reproduce

Build the project and run unit tests.

> mkdir build
> cd build
> cmake -G 'Visual Studio 15 2017' -A x64 ..
> cmake --build . --config Debug -- /m
> cmake --build . --config Debug --target check
...
  ..\..\tests\test_async.py ..                                             [  0%]
  ..\..\tests\test_builtin_casters.py ....s........s..                     [  5%]
  ..\..\tests\test_call_policies.py ........                               [  7%]
  ..\..\tests\test_callbacks.py .........                                  [ 10%]
  Windows fatal exception: access violation

  Current thread 0x00004fa8 (most recent call first):
    File "C:\Users\dorozhkin\Scratch\pybind11\tests\test_chrono.py", line 104 in test_chrono_system_clock_roundtrip_time

Behaviour is observed with 2.6.0.rc2 under Windows (VS2017/VS2019 compilers and Python 3.7 and 3.8 have been tested).

Removing all variants of datetime.time(..) parameters https://github.com/pybind/pybind11/blob/2e31e466dc7591612b4b17300797d352e1f3f801/tests/test_chrono.py#L85..L91 makes all tests pass.

[ghost]

Sample debugging session with the issue triggered.

[YannickJadoul]

MSVC specifies that it returns a nullptr

if the date passed to the function is:

• Before midnight, January 1, 1970.
• After 03:14:07, January 19, 2038, UTC (using _time32 and time32_t).
• After 23:59:59, December 31, 3000, UTC (using _time64 and __time64_t).

https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/localtime-localtime32-localtime64?view=vs-2019

So this seems very similar to #2417? @dorozhkin-stc, two silly things:

1. You're in GMT+3?
2. Does this only happen between 12 and 3, in your timezone? Can you reproduce it during the day?

@bstaletic, this might be that same bug from before, but in a slightly different form. Any suggestions?
@henryiii, should we still push a solution for 2.6.0, or is this a fix for 2.6.1?

[henryiii]

If we solve it before the release, and it looks safe to add, I think we can put in in 2.6.0. Otherwise, I don't think there's a problem making it target 2.6.1, I don't think it's a release blocker? (though we do claim to have fixed a rare time bug, and sadly now we know about another one...)

[bstaletic]

According to the C standard, localtime() returns:

a pointer to the broken-down time, or a null pointer if the specified
time cannot be converted to local time.

So unfortunately for us, we can't blame MSVC. Instead, we should check for NULL and throw.

The Linux man page says this:

ERRORS
       EOVERFLOW
              The result cannot be represented.

But MSVC says that it might set errno to EINVAL. So... yes, just check for NULL and throw.

[YannickJadoul]

So... yes, just check for NULL and throw.

Well, no, because: remember those time zone conversions, and pybind11 picking the 1st of January 1790 to convert a datetime.time to C++? Then we'd fail to convert some times to C++, depending on the time zone?

[bstaletic]

We can try hacking around it, by converting datetime.time as if it were 2000-01-01 instead of 1970-01-01, then somehow subtract 30 years, but... we're just reinventing C++20 chrono at this point. In a horrible way, no less.

EDIT: Or 1971 - 1, to avoid leap years and leap seconds. And only if the year we get is 1970. And this still wouldn't work for 2040... We definitely should be checking for NULL.

[ghost]

@YannickJadoul

You're in GMT+3?

Indeed I am in GMT+3.

Does this only happen between 12 and 3, in your timezone? Can you reproduce it during the day?

The issue had been originally triggered (and reported) when I tried to build the project around 21-22 PM in local time.

[YannickJadoul]

@dorozhkin-stc Thanks!

The issue had been originally triggered (and reported) when I tried to build the project around 21-22 PM in local time.

I forgot: we added some tests in #2417 that don't just use the current time, so it shouldn't matter when you run it, indeed :-)

[Andrea-Manghi]

Hello, I'm trying to compile pybind11 tests on visual studio and I encountered the same problem.
The thing is that in my case I'm in GTM+2 timezone, and the problem seems to happen independently from the actual time.
If you need it here's the project build log:
build_log.txt