Re: [PATCH] libstdc++: Make chrono::parse accept out-of-range values that aren't needed [PR126364]

On Fri, 31 Jul 2026 at 10:21, Jonathan Wakely <[email protected]> wrote:
>
> 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.

Yes, but there's no reason to preserve the bug in the release branches
when it's a trivial fix.

> > >>
> > >>
> > >>  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.

And that's what the code did previously, because read_unsigned sets
failbit, and then we reached the 'break'.

But I think my patch is still OK because __read_unsigned(2) will set
failbit if it sees "@@" instead of a number. That means even if we
don't break immediately, we won't extract any more characters and will
break when the next read fails. I don't think we need to
micro-optimize for the case of garbage input, so if we break
immediately or break a few nanoseconds later, it's fine.

I'll update the test to include "2026-07-31 xx:xx", "%F %R" cases.

>
> > >>
> > >>                           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
> > >>