Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date overflow with std::chrono::time_point #3725

Closed
cschreib-ibex opened this issue Nov 24, 2023 · 2 comments · Fixed by #3727
Closed

Date overflow with std::chrono::time_point #3725

cschreib-ibex opened this issue Nov 24, 2023 · 2 comments · Fixed by #3727

Comments

@cschreib-ibex
Copy link

std::chrono::system_clock::time_point on linux platforms uses nanosecond resolution and a 64 bit signed counter, hence is unable to represent dates beyond year 2262. This can be circumvented by using a custom time_point with a lower time resolution when doing time calculations, such as

using my_time_point = std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>;

Unfortunately, it seems that fmtlib is unable to correctly format such time points despite the lower time resolution. Here is an example that reproduces the problem:

#include <chrono>
#include <iostream>
#include <fmt/chrono.h>

#if defined(__cpp_lib_format) && __cpp_lib_format >= 202106L
#define HAS_STD_FORMAT
#endif

#if defined(HAS_STD_FORMAT)
#include <format>
#endif

int main() {
    std::cout << "fmt version: " << FMT_VERSION << "\n";

    using TimePointBad = std::chrono::system_clock::time_point;
    auto timeBad = TimePointBad{} + std::chrono::years{1030};
    std::cout << "bad time_point years:  " <<
        1970 + std::chrono::duration_cast<std::chrono::years>(timeBad.time_since_epoch()).count() << "\n";
    std::cout << "fmt::format:           " << fmt::format("{:%F %T}", timeBad) << "\n";
    #if defined(HAS_STD_FORMAT)
    std::cout << "std::format:           " << std::format("{:%F %T}", timeBad) << "\n";
    #else
    std::cout << "std::format:           not available\n";
    #endif

    using TimePointGood = std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>;
    auto timeGood = TimePointGood{} + std::chrono::years{1030};
    std::cout << "good time_point years: " <<
        1970 + std::chrono::duration_cast<std::chrono::years>(timeGood.time_since_epoch()).count() << "\n";
    std::cout << "fmt::format:           " << fmt::format("{:%F %T}", timeGood) << "\n";
    #if defined(HAS_STD_FORMAT)
    std::cout << "std::format:           " << std::format("{:%F %T}", timeGood) << "\n";
    #else
    std::cout << "std::format:           not available\n";
    #endif

    return 0;
}

Output from latest master commit on my system (Ubuntu GCC 11.4; doesn't have <format>):

fmt version: 100101
bad time_point years:  1831
fmt::format:           1830-11-22 19:26:52.580896768
std::format:           not available
good time_point years: 3000
fmt::format:           1830-11-22 19:26:53.000
std::format:           not available

Output lines 2 and 3 show the problem of the standard std::chrono::system_clock::time_point; this is the output I expect.

Output line 5 shows that, using TimePointGood with millisecond resolution, the time point itself is able to represent dates far in the future.

Output line 6 shows that fmtlib still overflows.

Godbolt link; this is running an older version of fmtlib but it also affected. Using gcc trunk with std::format shows that the standard format doesn't have the issue and outputs what one would expect:

fmt version: 90100
bad time_point years:  1831
fmt::format:           1830-11-22 19:26:53
std::format:           1830-11-22 19:26:52.580896768
good time_point years: 3000
fmt::format:           1830-11-22 19:26:53
std::format:           2999-12-31 18:36:00.000
@cschreib-ibex
Copy link
Author

cschreib-ibex commented Nov 24, 2023

I think the issue is here:

fmt/include/fmt/chrono.h

Lines 2097 to 2103 in dd6f657

return formatter<std::tm, Char>::do_format(
gmtime(std::chrono::time_point_cast<std::chrono::seconds>(val)), ctx,
&subsecs);
}
return formatter<std::tm, Char>::format(
gmtime(std::chrono::time_point_cast<std::chrono::seconds>(val)), ctx);

and here:

fmt/include/fmt/chrono.h

Lines 526 to 529 in dd6f657

inline std::tm gmtime(
std::chrono::time_point<std::chrono::system_clock> time_point) {
return gmtime(std::chrono::system_clock::to_time_t(time_point));
}

  1. The time point is converted to seconds; this could be dangerous since this is only required to hold at least 292 years worth of time, and we're asking for more in this case. In practice this works OK since seconds is typically implemented with a 64 bit signed integer, which gives plenty of space (a 32 bit signed integer would only provide 68 years worth of time, hence isn't allowed). But it's technically beyond what the standard guarantees. To be safe, we could use std::chrono::duration<std::int64_t> instead, which can hold up to 300 billions years worth of seconds.
  2. This is where the real issue is. gmtime implicitly converts the time_point back to the system_clock::time_point, which has nanosecond precision on linux, hence overflows. We can't use system_clock::from_time_t, since it always requires a system_clock::time_point. Thankfully, system_clock is defined to have the same epoch as time_t (only in C++20; but even though this wasn't defined pre-C++20, all implementations did this anyway). This means you can get a time_t by simply counting the number of seconds in the time_point since the epoch, which is what we have already.

Hence, simply using this alternative gmtime overload should solve the problem:

template<typename Duration>
inline std::tm gmtime(
    std::chrono::time_point<std::chrono::system_clock, Duration> time_point) {
  using large_seconds = std::chrono::duration<std::int64_t>;
  return gmtime(std::chrono::time_point_cast<large_seconds>(time_point).time_since_epoch().count());
}

(and we can remove the time_point_cast in the formatter)

NB: this is assuming that time_t is 64 bit; if it's not, we're in deeper trouble :)

@cschreib
Copy link
Contributor

I opened a PR with a suggested fix #3727.

cschreib added a commit to cschreib/fmt that referenced this issue Nov 25, 2023
The function is now more generic and will handle all casts. It also
takes care of toggling safe vs unsafe casts using
FMT_SAFE_DURATION_CAST.
vitaut pushed a commit that referenced this issue Nov 25, 2023
* Fix #3725 and rename fmt_safe_duration_cast to fmt_duration_cast
The function is now more generic and will handle all casts. It also
takes care of toggling safe vs unsafe casts using
FMT_SAFE_DURATION_CAST.

* Refactor fmt_duration_cast to put #ifdef inside the function

* Fix compilation error with FMT_USE_LOCAL_TIME
happymonkey1 pushed a commit to happymonkey1/fmt that referenced this issue Apr 7, 2024
* Fix fmtlib#3725 and rename fmt_safe_duration_cast to fmt_duration_cast
The function is now more generic and will handle all casts. It also
takes care of toggling safe vs unsafe casts using
FMT_SAFE_DURATION_CAST.

* Refactor fmt_duration_cast to put #ifdef inside the function

* Fix compilation error with FMT_USE_LOCAL_TIME
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants