Skip to content

<chrono>: Fine grained bounds checking #1871

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

Merged
Stephan T. Lavavej (StephanTLavavej) merged 4 commits into from
Apr 21, 2021
Merged

<chrono>: Fine grained bounds checking #1871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a26bfeb
<chrono>: Fine grained bounds checking
eldakesh-ms Apr 21, 2021
ff2c5c5
Break
eldakesh-ms Apr 21, 2021
73f26ed
Code review.
StephanTLavavej Apr 21, 2021
20560e3
Adjust formatting.
StephanTLavavej Apr 21, 2021
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
187 changes: 135 additions & 52 deletions stl/inc/chrono
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5965,13 +5965,8 @@ namespace chrono {
if (_Custom_write(_Stream, _Spec, _Time, _Val)) {
continue;
}
// Otherwise, we should throw on out-of-bounds to avoid triggering asserts within put_time
// machinery.
if constexpr (_Has_ok<_Ty>) {
if (!_Val.ok()) {
_THROW(format_error("Cannot localize out-of-bounds time point."));
}
}

_Validate_specifiers(_Spec, _Val);

_Stream << _STD put_time<_CharT>(&_Time, _Fmt_string(_Spec).data());
}
Expand All @@ -5998,29 +5993,17 @@ namespace chrono {
const auto _Month = _Time.tm_mon + 1;
const bool _Has_modifier = _Spec._Modifier != '\0';
switch (_Spec._Type) {
case 'a':
case 'A':
case 'u':
case 'w':
if constexpr (_Is_any_of_v<_Ty, year_month_weekday, year_month_weekday_last>) {
if (!_Val.weekday().ok()) {
_THROW(format_error("Cannot print invalid weekday"));
}
_Os << _STD put_time(&_Time, _Fmt_string(_Spec).data());
return true;
} else if constexpr (_Has_ok<_Ty>) {
if (!_Val.ok()) {
_THROW(format_error("Cannot print invalid weekday"));
}
}
return false;
case 'd':
case 'd': // Print days as a decimal, even if invalid.
case 'e':
// Most months have a proper last day, but February depends on the year.
if constexpr (is_same_v<_Ty, month_day_last>) {
if (_Val.month() == February) {
_THROW(format_error("Cannot print the last day of February without a year"));
}

if (!_Val.ok()) {
return false;
}
}

if (_Has_modifier) {
Expand Down Expand Up @@ -6063,20 +6046,6 @@ namespace chrono {
if constexpr (_Is_specialization_v<_Ty, duration>) {
_Os << _STD abs(_CHRONO duration_cast<days>(_Val).count());
return true;
} else if constexpr (is_same_v<_Ty, month_day>) {
if (_Val.month() > February) {
_THROW(format_error("The day of year for a month_day past February is ambiguous."));
}
} else if constexpr (is_same_v<_Ty, month_day_last>) {
if (_Val.month() >= February) {
_THROW(format_error("The day of year for a month_day_last other than January is ambiguous"));
}
}

if constexpr (_Has_ok<_Ty>) {
if (!_Val.ok()) {
_THROW(format_error("Cannot print invalid day of year"));
}
}
return false;
case 'q':
Expand All @@ -6089,7 +6058,7 @@ namespace chrono {
_Os << _STD abs(_Val.count());
}
return true;
case 'm':
case 'm': // Print months as a decimal, even if invalid.
if (_Has_modifier) {
return false;
}
Expand All @@ -6099,7 +6068,7 @@ namespace chrono {
}
_Os << _Month;
return true;
case 'Y':
case 'Y': // Print years as a decimal, even if invalid.
if (_Has_modifier) {
return false;
}
Expand All @@ -6109,13 +6078,13 @@ namespace chrono {
}
_Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:04}"), _STD abs(_Year));
return true;
case 'y':
case 'y': // Print the two-digit year as a decimal, even if invalid.
if (_Has_modifier) {
return false;
}
_Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Year < 0 ? 100 + (_Year % 100) : _Year % 100);
return true;
case 'C':
case 'C': // Print the century as a decimal, even if invalid.
if (_Has_modifier) {
return false;
}
Expand All @@ -6126,30 +6095,23 @@ namespace chrono {
_Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"),
_STD abs(_Time_parse_fields::_Decompose_year(_Year).first) / 100);
return true;
case 'F':
case 'F': // Print YMD even if invalid.
_Custom_write(_Os, {._Type = 'Y'}, _Time, _Val);
_Os << _CharT{'-'};
_Custom_write(_Os, {._Type = 'm'}, _Time, _Val);
_Os << _CharT{'-'};
_Custom_write(_Os, {._Type = 'd'}, _Time, _Val);
return true;
case 'D':
case 'D': // Print YMD even if invalid.
_Custom_write(_Os, {._Type = 'm'}, _Time, _Val);
_Os << _CharT{'/'};
_Custom_write(_Os, {._Type = 'd'}, _Time, _Val);
_Os << _CharT{'/'};
_Custom_write(_Os, {._Type = 'y'}, _Time, _Val);
return true;
case 'H':
if constexpr (_Is_specialization_v<_Ty, hh_mm_ss>) {
if (_Val.hours() >= hours{24}) {
_THROW(format_error("Cannot localize hh_mm_ss with an absolute value of 24 hours or more."));
}
}
return false;
case 'T':
// Alias for %H:%M:%S but we need to rewrite %S to display fractions of a second.
_Custom_write(_Os, {._Type = 'H'}, _Time, _Val);
_Validate_specifiers({._Type = 'H'}, _Val);
_Os << _STD put_time(&_Time, _STATICALLY_WIDEN(_CharT, "%H:%M:"));
[[fallthrough]];
case 'S':
Expand Down Expand Up @@ -6202,6 +6164,127 @@ namespace chrono {
}
}

template <class _Ty>
static void _Validate_specifiers(const _Chrono_spec<_CharT>& _Spec, const _Ty& _Val) {
// clang-format off
if constexpr (_Is_specialization_v<_Ty, duration> || is_same_v<_Ty, sys_info>
|| _Is_specialization_v<_Ty, time_point> || _Is_specialization_v<_Ty, _Local_time_format_t>) {
return;
}
// clang-format on

if constexpr (_Is_specialization_v<_Ty, hh_mm_ss>) {
if (_Spec._Type == 'H' && _Val.hours() >= hours{24}) {
_THROW(format_error("Cannot localize hh_mm_ss with an absolute value of 24 hours or more."));
}
return;
}
Comment on lines +6176 to +6181

@eldakesh-ms Elnar Dakeshov (eldakesh-ms) Apr 21, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apart from H, my understanding is that hh_mm_ss is always valid (for it's specifiers).


constexpr bool _Is_ymd =
_Is_any_of_v<_Ty, year_month_day, year_month_day_last, year_month_weekday, year_month_weekday_last>;

const auto _Validate = [&] {
switch (_Spec._Type) {
case 'a':
case 'A':
case 'u':
case 'w':
if constexpr (_Is_any_of_v<_Ty, weekday, weekday_last>) {
return _Val.ok();
} else if constexpr (_Is_any_of_v<_Ty, weekday_indexed, year_month_weekday,
year_month_weekday_last>) {
return _Val.weekday().ok();
} else if constexpr (is_same_v<_Ty, month_weekday>) {
return _Val.weekday_indexed().weekday().ok();
} else if constexpr (is_same_v<_Ty, month_weekday_last>) {
return _Val.weekday_last().ok();
} else if constexpr (_Is_any_of_v<_Ty, year_month_day, year_month_day_last>) {
return _Val.ok();
}
Comment thread
mnatsuhara marked this conversation as resolved.
break;
Comment on lines +6188 to +6204

@eldakesh-ms Elnar Dakeshov (eldakesh-ms) Apr 21, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By far the finickiest specifier, because almost everything can go wrong and you can still get a weekday.


case 'b':
case 'B':
case 'h':
case 'm':
if constexpr (is_same_v<_Ty, month>) {
return _Val.ok();
} else if constexpr (_Is_any_of_v<_Ty, month_day, month_day_last, month_weekday, month_weekday_last,
year_month> || _Is_ymd) {
return _Val.month().ok();
}
break;

case 'C':
case 'y':
case 'Y':
if constexpr (is_same_v<_Ty, year>) {
return _Val.ok();
} else if constexpr (_Is_any_of_v<_Ty, year_month> || _Is_ymd) {
return _Val.year().ok();
}
break;

case 'd':
case 'e':
if constexpr (_Is_any_of_v<_Ty, day, month_day_last>) {
return _Val.ok();
} else if constexpr (is_same_v<_Ty, month_day>) {
return _Val.day().ok();
} else if constexpr (_Is_ymd) {
const year_month_day& _Ymd{_Val};
return _Ymd.day().ok();
}
break;

case 'D':
case 'F':
if constexpr (_Has_ok<_Ty>) {
return _Val.ok();
}
break;

case 'j':
if constexpr (is_same_v<_Ty, month_day>) {
if (_Val.month() > February) {
_THROW(format_error("The day of year for a month_day past February is ambiguous."));
}
return true;
} else if constexpr (is_same_v<_Ty, month_day_last>) {
if (_Val.month() >= February) {
_THROW(
format_error("The day of year for a month_day_last other than January is ambiguous"));
}
return true;
} else if constexpr (_Is_ymd) {
return _Val.ok();
}
break;

case 'g':
case 'G':
case 'U':
case 'V':
case 'W':
if constexpr (_Is_ymd) {
return _Val.ok();
}
Comment thread
eldakesh-ms marked this conversation as resolved.
break;

default:
if constexpr (_Has_ok<_Ty>) {
return _Val.ok();
}
return true;
}
_STL_INTERNAL_CHECK(false);

@eldakesh-ms Elnar Dakeshov (eldakesh-ms) Apr 21, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idea is that if a specifier is being checked manually, we should check it for every type that is valid for that specifier (apart from the always correct types at the top). This way, if we miss specific handling for a type-specifier combo, we can catch it internally.

return false;
};
if (!_Validate()) {
_THROW(format_error("Cannot localize out-of-bounds time point."));
}
}

_NODISCARD static array<_CharT, 4> _Fmt_string(const _Chrono_spec<_CharT>& _Spec) {
array<_CharT, 4> _Fmt_str;
size_t _Next_idx = 0;
Expand Down
10 changes: 10 additions & 0 deletions tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ void test_weekday_indexed_formatter() {
assert(format(STR("{:%a %A}"), weekday_indexed{Monday, 2}) == STR("Mon Monday"));
assert(format(STR("{:%u %w}"), weekday_indexed{Tuesday, 3}) == STR("2 2"));
assert(format(STR("{:%u %w}"), weekday_indexed{Sunday, 4}) == STR("7 0"));
assert(format(STR("{:%u %w}"), weekday_indexed{Sunday, 10}) == STR("7 0"));
}

template <typename CharT>
Expand All @@ -464,6 +465,8 @@ void test_month_day_formatter() {

assert(format(STR("{:%B %d}"), June / 17) == STR("June 17"));
throw_helper(STR("{:%Y}"), June / 17);
assert(format(STR("{:%B}"), June / day{40}) == STR("June"));
throw_helper(STR("{:%B}"), month{13} / 17);

assert(format(STR("{:%j}"), January / 5) == STR("005"));
assert(format(STR("{:%j}"), February / 5) == STR("036"));
Expand All @@ -479,6 +482,7 @@ void test_month_day_last_formatter() {
assert(format(STR("{:%B}"), June / last) == STR("June"));
assert(format(STR("{:%d}"), June / last) == STR("30"));
throw_helper(STR("{:%d}"), February / last);
throw_helper(STR("{:%B}"), month{13} / last);

assert(format(STR("{:%j}"), January / last) == STR("031"));
throw_helper(STR("{:%j}"), February / last);
Expand All @@ -505,6 +509,9 @@ void test_month_weekday_formatter() {

assert(format(STR("{:%b %B %h %m %a %A %u %w}"), mwd1) == STR("Aug August Aug 08 Tue Tuesday 2 2"));
assert(format(STR("{:%b %B %h %m %a %A %u %w}"), mwd2) == STR("Dec December Dec 12 Sun Sunday 7 0"));

assert(format(STR("{:%u}"), invalid1) == STR("5"));
throw_helper(STR("{:%u}"), invalid2);
}

template <typename CharT>
Expand All @@ -525,6 +532,9 @@ void test_month_weekday_last_formatter() {

assert(format(STR("{:%b %B %h %m %a %A %u %w}"), mwdl1) == STR("Aug August Aug 08 Tue Tuesday 2 2"));
assert(format(STR("{:%b %B %h %m %a %A %u %w}"), mwdl2) == STR("Dec December Dec 12 Sun Sunday 7 0"));

assert(format(STR("{:%u}"), invalid2) == STR("5"));
throw_helper(STR("{:%u}"), invalid1);
}

template <typename CharT>
Expand Down