On Sun, 12 Nov 2023 at 01:34, Cassio Neri <cassio.n...@gmail.com> wrote:
>
> The following has undefined behaviour (signed overflow) [1]:
> weekday max{sys_days{days{numeric_limits<days::rep>::max()}}};
>
> The issue is in this line when __n is very large and __n + 4 overflows:
> return weekday(__n >= -4 ? (__n + 4) % 7 : (__n + 5) % 7 + 6);
>
> In addition to fixing this bug, the new implementation makes the compiler emit
> shorter and branchless code for x86-64 and ARM [2].
>
> [1] https://godbolt.org/z/1s5bv7KfT
> [2] https://godbolt.org/z/zKsabzrhs
>
> libstdc++-v3/ChangeLog:
>
> * include/std/chrono: Fix weekday::_S_from_days
> * testsuite/std/time/weekday/1.cc: Add test for overflow.
> ---
>
> Good for trunk?
Pushed to trunk, thanks. I think this one is worth backporting too.
>
> libstdc++-v3/include/std/chrono | 11 +++++++++--
> libstdc++-v3/testsuite/std/time/weekday/1.cc | 9 +++++++++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
> index 10e868e5a03..c00dd133173 100644
> --- a/libstdc++-v3/include/std/chrono
> +++ b/libstdc++-v3/include/std/chrono
> @@ -930,8 +930,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> static constexpr weekday
> _S_from_days(const days& __d)
> {
> - auto __n = __d.count();
> - return weekday(__n >= -4 ? (__n + 4) % 7 : (__n + 5) % 7 + 6);
> + using _Rep = days::rep;
> + using _URep = make_unsigned_t<_Rep>;
> + const auto __n = __d.count();
> + const auto __m = static_cast<_URep>(__n);
> +
> + // 1970-01-01 (__n = 0, __m = 0 ) -> Thursday (4)
> + // 1969-31-12 (__n = -1, __m = _URep(-1)) -> Wednesday (3)
> + const auto __offset = __n >= 0 ? _URep(4) : 3 - _URep(-1) % 7 - 7;
> + return weekday((__m + __offset) % 7);
> }
>
> public:
> diff --git a/libstdc++-v3/testsuite/std/time/weekday/1.cc
> b/libstdc++-v3/testsuite/std/time/weekday/1.cc
> index 00278c8b01c..e89fca47d4b 100644
> --- a/libstdc++-v3/testsuite/std/time/weekday/1.cc
> +++ b/libstdc++-v3/testsuite/std/time/weekday/1.cc
> @@ -20,6 +20,7 @@
> // Class template day [time.cal.weekday]
>
> #include <chrono>
> +#include <limits>
>
> constexpr void
> constexpr_weekday()
> @@ -37,6 +38,14 @@ constexpr_weekday()
> static_assert(weekday{3}[2].weekday() == weekday{3});
> static_assert(weekday{3}[last].weekday() == weekday{3});
>
> + // Test for UB (overflow).
> + {
> + using rep = days::rep;
> + using std::numeric_limits;
> + constexpr weekday max{sys_days{days{numeric_limits<rep>::max()}}};
> + constexpr weekday min{sys_days{days{numeric_limits<rep>::min()}}};
> + }
> +
> static_assert(weekday{sys_days{1900y/January/1}} == Monday);
> static_assert(weekday{sys_days{1970y/January/1}} == Thursday);
> static_assert(weekday{sys_days{2020y/August/21}} == Friday);
> --
> 2.41.0
>
