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.

Sign up for GitHub

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

Fix overflow in time_point formatting with large dates #3727

Merged
merged 3 commits into from
Nov 25, 2023
Merged

Fix overflow in time_point formatting with large dates #3727

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbfeea8
Fix #3725 and rename fmt_safe_duration_cast to fmt_duration_cast
cschreib Nov 24, 2023
0b52401
Refactor fmt_duration_cast to put #ifdef inside the function
cschreib Nov 25, 2023
eedce0a
Fix compilation error with FMT_USE_LOCAL_TIME
cschreib Nov 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 61 additions & 46 deletions include/fmt/chrono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,45 @@ auto write(OutputIt out, const std::tm& time, const std::locale& loc,
return write_encoded_tm_str(out, string_view(buf.data(), buf.size()), loc);
}

#if FMT_SAFE_DURATION_CAST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved into fmt_duration_cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, if I understood what you meant correctly!

// throwing version of safe_duration_cast
// only available for integer<->integer or float<->float casts
template <typename To, typename FromRep, typename FromPeriod,
FMT_ENABLE_IF((std::is_integral<FromRep>::value &&
std::is_integral<typename To::rep>::value) ||
(std::is_floating_point<FromRep>::value &&
std::is_floating_point<typename To::rep>::value))>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a trait for this and reuse here and below (negated).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

To fmt_duration_cast(std::chrono::duration<FromRep, FromPeriod> from) {
int ec;
To to = safe_duration_cast::safe_duration_cast<To>(from, ec);
if (ec) FMT_THROW(format_error("cannot format duration"));
return to;
}
// mixed integer<->float cast is not supported with safe_duration_cast
// fallback to standard duration cast in this case
template <typename To, typename FromRep, typename FromPeriod,
FMT_ENABLE_IF((std::is_integral<FromRep>::value !=
std::is_integral<typename To::rep>::value) &&
(std::is_floating_point<FromRep>::value !=
std::is_floating_point<typename To::rep>::value))>
To fmt_duration_cast(std::chrono::duration<FromRep, FromPeriod> from) {
return std::chrono::duration_cast<To>(from);
}
#else
// standard duration cast, may overflow and invoke undefined behavior
template <typename To, typename FromRep, typename FromPeriod>
To fmt_duration_cast(std::chrono::duration<FromRep, FromPeriod> from) {
return std::chrono::duration_cast<To>(from);
}
#endif

template <typename Duration>
std::time_t to_time_t(
std::chrono::time_point<std::chrono::system_clock, Duration> time_point) {
return fmt_duration_cast<std::chrono::duration<std::time_t>>(
time_point.time_since_epoch())
.count();
}
} // namespace detail

FMT_BEGIN_EXPORT
Expand Down Expand Up @@ -478,8 +517,8 @@ inline std::tm localtime(std::time_t time) {
#if FMT_USE_LOCAL_TIME
template <typename Duration>
inline auto localtime(std::chrono::local_time<Duration> time) -> std::tm {
return localtime(std::chrono::system_clock::to_time_t(
std::chrono::current_zone()->to_sys(time)));
return localtime(
detail::to_time_t(std::chrono::current_zone()->to_sys(time)));
}
#endif

Expand Down Expand Up @@ -523,9 +562,10 @@ inline std::tm gmtime(std::time_t time) {
return gt.tm_;
}

template <typename Duration>
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));
std::chrono::time_point<std::chrono::system_clock, Duration> time_point) {
return gmtime(detail::to_time_t(time_point));
}

namespace detail {
Expand Down Expand Up @@ -1051,13 +1091,12 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) {
std::chrono::seconds::rep>::type,
std::ratio<1, detail::pow10(num_fractional_digits)>>;

const auto fractional =
d - std::chrono::duration_cast<std::chrono::seconds>(d);
const auto fractional = d - fmt_duration_cast<std::chrono::seconds>(d);
const auto subseconds =
std::chrono::treat_as_floating_point<
typename subsecond_precision::rep>::value
? fractional.count()
: std::chrono::duration_cast<subsecond_precision>(fractional).count();
: fmt_duration_cast<subsecond_precision>(fractional).count();
auto n = static_cast<uint32_or_64_or_128_t<long long>>(subseconds);
const int num_digits = detail::count_digits(n);

Expand Down Expand Up @@ -1620,17 +1659,6 @@ template <typename T> struct make_unsigned_or_unchanged<T, true> {
using type = typename std::make_unsigned<T>::type;
};

#if FMT_SAFE_DURATION_CAST
// throwing version of safe_duration_cast
template <typename To, typename FromRep, typename FromPeriod>
To fmt_safe_duration_cast(std::chrono::duration<FromRep, FromPeriod> from) {
int ec;
To to = safe_duration_cast::safe_duration_cast<To>(from, ec);
if (ec) FMT_THROW(format_error("cannot format duration"));
return to;
}
#endif

template <typename Rep, typename Period,
FMT_ENABLE_IF(std::is_integral<Rep>::value)>
inline std::chrono::duration<Rep, std::milli> get_milliseconds(
Expand All @@ -1640,17 +1668,17 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(
#if FMT_SAFE_DURATION_CAST
using CommonSecondsType =
typename std::common_type<decltype(d), std::chrono::seconds>::type;
const auto d_as_common = fmt_safe_duration_cast<CommonSecondsType>(d);
const auto d_as_common = fmt_duration_cast<CommonSecondsType>(d);
const auto d_as_whole_seconds =
fmt_safe_duration_cast<std::chrono::seconds>(d_as_common);
fmt_duration_cast<std::chrono::seconds>(d_as_common);
// this conversion should be nonproblematic
const auto diff = d_as_common - d_as_whole_seconds;
const auto ms =
fmt_safe_duration_cast<std::chrono::duration<Rep, std::milli>>(diff);
fmt_duration_cast<std::chrono::duration<Rep, std::milli>>(diff);
return ms;
#else
auto s = std::chrono::duration_cast<std::chrono::seconds>(d);
return std::chrono::duration_cast<std::chrono::milliseconds>(d - s);
auto s = fmt_duration_cast<std::chrono::seconds>(d);
return fmt_duration_cast<std::chrono::milliseconds>(d - s);
#endif
}

Expand Down Expand Up @@ -1751,14 +1779,8 @@ struct chrono_formatter {

// this may overflow and/or the result may not fit in the
// target type.
#if FMT_SAFE_DURATION_CAST
// might need checked conversion (rep!=Rep)
auto tmpval = std::chrono::duration<rep, Period>(val);
s = fmt_safe_duration_cast<seconds>(tmpval);
#else
s = std::chrono::duration_cast<seconds>(
std::chrono::duration<rep, Period>(val));
#endif
s = fmt_duration_cast<seconds>(std::chrono::duration<rep, Period>(val));
}

// returns true if nan or inf, writes to out.
Expand Down Expand Up @@ -2082,25 +2104,22 @@ struct formatter<std::chrono::time_point<std::chrono::system_clock, Duration>,
period::num != 1 || period::den != 1 ||
std::is_floating_point<typename Duration::rep>::value)) {
const auto epoch = val.time_since_epoch();
auto subsecs = std::chrono::duration_cast<Duration>(
epoch - std::chrono::duration_cast<std::chrono::seconds>(epoch));
auto subsecs = detail::fmt_duration_cast<Duration>(
epoch - detail::fmt_duration_cast<std::chrono::seconds>(epoch));

if (subsecs.count() < 0) {
auto second =
std::chrono::duration_cast<Duration>(std::chrono::seconds(1));
detail::fmt_duration_cast<Duration>(std::chrono::seconds(1));
if (epoch.count() < ((Duration::min)() + second).count())
FMT_THROW(format_error("duration is too small"));
subsecs += second;
val -= second;
}

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

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

Expand All @@ -2119,17 +2138,13 @@ struct formatter<std::chrono::local_time<Duration>, Char>
if (period::num != 1 || period::den != 1 ||
std::is_floating_point<typename Duration::rep>::value) {
const auto epoch = val.time_since_epoch();
const auto subsecs = std::chrono::duration_cast<Duration>(
epoch - std::chrono::duration_cast<std::chrono::seconds>(epoch));
const auto subsecs = fmt_duration_cast<Duration>(
epoch - fmt_duration_cast<std::chrono::seconds>(epoch));

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

return formatter<std::tm, Char>::format(
localtime(std::chrono::time_point_cast<std::chrono::seconds>(val)),
ctx);
return formatter<std::tm, Char>::format(localtime(val), ctx);
}
};
#endif
Expand Down
14 changes: 14 additions & 0 deletions test/chrono-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,20 @@ TEST(chrono_test, timestamps_ratios) {
t4(std::chrono::duration<int, std::ratio<63>>(1));

EXPECT_EQ(fmt::format("{:%M:%S}", t4), "01:03");

std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>
t5(std::chrono::seconds(32503680000));

EXPECT_EQ(fmt::format("{:%Y-%m-%d}", t5), "3000-01-01");

#if FMT_SAFE_DURATION_CAST
using years = std::chrono::duration<std::int64_t, std::ratio<31556952>>;
std::chrono::time_point<std::chrono::system_clock, years> t6(
(years(std::numeric_limits<std::int64_t>::max())));

EXPECT_THROW_MSG((void)fmt::format("{:%Y-%m-%d}", t6), fmt::format_error,
"cannot format duration");
#endif
}

TEST(chrono_test, timestamps_sub_seconds) {
Expand Down
Loading