Re: [PATCH] libstdc++: Make chrono::parse accept out-of-range values that aren't needed [PR126364]
Jonathan Wakely <[email protected]>
| Newsgroups | gmane.comp.gcc.patches,gmane.comp.gcc.libstdc++.devel |
|---|---|
| Message-ID | <CACb0b4mua28s74uPn3PW1tt289VhRarNPo5hrLNNu=mhkB7ZHA@mail.gmail.com> |
On Fri, 31 Jul 2026 at 06:48, Tomasz Kaminski <[email protected]> wrote: > > > > On Thu, Jul 30, 2026 at 1:41 PM Jonathan Wakely <[email protected]> wrote: >> >> When parsing a time with %R or %T we should ignore out of range hours >> and minutes if the type being parsed doesn't need them, e.g. when >> parsing a chrono::year_month_day from "2026-07-29 99:99:99" we do not >> set failbit, and should continue parsing after the invalid hours and >> minutes. >> >> Because we were short circuiting as soon as we saw "99" (in either >> field) we didn't parse to the end of the %R or %T field, and then could >> set failbit if there were any subsequent characters or flags to parse. >> >> The fix is to only short-circuit when setting failbit, and continue >> parsing otherwise. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/126364 >> * include/bits/chrono_io.h (_Parser::operator()) <R>: Only break >> early when setting failbit. >> * testsuite/std/time/parse/126364.cc: New test. >> --- >> >> Tested x86_64-linux. > > LGTM with only one small change. >> >> >> This should be backported to 14, 15 and 16 as well. > > Agreed; this is OK to backport as the patch is trivial. > However, this is C++20 feature, so we do not strictly need to if I understand > correctly. >> >> >> libstdc++-v3/include/bits/chrono_io.h | 12 ++++--- >> .../testsuite/std/time/parse/126364.cc | 34 +++++++++++++++++++ >> 2 files changed, 42 insertions(+), 4 deletions(-) >> create mode 100644 libstdc++-v3/testsuite/std/time/parse/126364.cc >> >> diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h >> index c5170368f82f..51e07ff55593 100644 >> --- a/libstdc++-v3/include/bits/chrono_io.h >> +++ b/libstdc++-v3/include/bits/chrono_io.h >> @@ -4737,8 +4737,10 @@ namespace __detail >> if (__val == -1 || __val > 23) [[unlikely]] >> { > > You do not need these braces anymore, and can put it directly under the above if > (We want to merge them into one, as the condition mentioned above is not unlikely) I assume you mean we do *not* want to merge them into one. I'll make that change, thanks. >> >> if ((_M_need & _ChronoParts::_TimeOfDay) != 0) >> - __err |= ios_base::failbit; >> - break; >> + { >> + __err |= ios_base::failbit; >> + break; >> + } >> } >> if (!__read_chr(':')) [[unlikely]] >> break; >> @@ -4748,8 +4750,10 @@ namespace __detail >> if (__val == -1 || __val > 60) [[unlikely]] >> { > > Same here. >> >> if ((_M_need & _ChronoParts::_TimeOfDay) != 0) >> - __err |= ios_base::failbit; >> - break; >> + { >> + __err |= ios_base::failbit; >> + break; >> + } >> } >> __min = minutes(__val); >> >> diff --git a/libstdc++-v3/testsuite/std/time/parse/126364.cc b/libstdc++-v3/testsuite/std/time/parse/126364.cc >> new file mode 100644 >> index 000000000000..74a94a6b84b8 >> --- /dev/null >> +++ b/libstdc++-v3/testsuite/std/time/parse/126364.cc >> @@ -0,0 +1,34 @@ >> +// { dg-do run { target c++20 } } >> + >> +// Bug 126364 - chrono::from_stream %T and %R short circuit on out of range >> +// values even when it doesn't fail the parse >> + >> +#include <chrono> >> +#include <sstream> >> +#include <testsuite_hooks.h> >> + >> +using namespace std::chrono; >> + >> +void >> +test_pr126364() >> +{ >> + auto check = [](const char* input, char const* fmt) { >> + std::istringstream is(input); >> + year_month_day ymd{}; >> + return from_stream(is, fmt, ymd).good() && ymd.ok(); >> + }; >> + >> + VERIFY( check("2019-09-05T25:36:57Z", "%FT%TZ") ); // hour 25 only >> + VERIFY( check("2019-09-05T20:99:57Z", "%FT%TZ") ); // minute 99 only >> + VERIFY( check("2019-09-05T20:36:99Z", "%FT%TZ") ); // second 99 only >> + VERIFY( check("2019-09-05T25:36:57Z", "%FT%T") ); // hour 25 only, no Z >> + VERIFY( check("2019-09-05T20:99:57Z", "%FT%T") ); // minute 99 only, no Z >> + VERIFY( check("2019-09-05T20:36:99Z", "%FT%T") ); // second 99 only, no Z >> + VERIFY( check("2019-09-05 25", "%F %H") ); // %H out of range alone >> + VERIFY( check("2019-09-05 99", "%F %M") ); // %M out of range alone >> +} >> + >> +int main() >> +{ >> + test_pr126364(); >> +} >> -- >> 2.55.0 >>