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 | <CACb0b4nG_pF606SDevKgMp9Ey=YMm8YSUVVzpyFQN6uBF2s1xg@mail.gmail.com> |
On Fri, 31 Jul 2026 at 10:13, Jonathan Wakely <[email protected]> wrote: > > 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. Actually, thinking about this further, I think if __val == -1 is true, we do want to break. That means we failed to parse any number at all. It makes sense to ignore 99:99 but for garbage like @@:?? the parse should fail. > >> > >> 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 > >>