From 2582e81d0f38fc1c71fd1df9603f464137a7bcab Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 29 Nov 2024 10:12:12 +0800 Subject: [PATCH 1/2] Avoid arithmetic overflow in the constructors of `weekday` Co-authored-by: Cassio Neri --- stl/inc/chrono | 5 +++-- .../tests/P0355R7_calendars_and_time_zones_dates/test.cpp | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index a895f59a2c2..07e0589a41a 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -473,10 +473,11 @@ namespace chrono { private: unsigned char _Weekday; - // courtesy of Howard Hinnant + // courtesy of Howard Hinnant (modified to avoid overflow) // https://howardhinnant.github.io/date_algorithms.html#weekday_from_days _NODISCARD static constexpr unsigned int _Weekday_from_days(int _Tp) noexcept { - return static_cast(_Tp >= -4 ? (_Tp + 4) % 7 : (_Tp + 5) % 7 + 6); + const auto _Before_modulo = static_cast(_Tp) + (_Tp >= 0 ? 4u : 0u); + return _Before_modulo % 7u; // for codegen by MSVC on x86/ARM64, see GH-5153 } }; diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp index 6a59c863a07..6558238e24b 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp @@ -281,6 +281,10 @@ constexpr void weekday_test() { assert(Sunday - Monday == days{6}); assert(Sunday - Tuesday == days{5}); assert(Wednesday - Thursday == days{6}); + + // GH-5153 ": Undefined behaviour in weekday::weekday(const sys_days&)." + assert(weekday{sys_days::max()} == weekday{sys_days::max() - days{7}}); + assert(weekday{local_days::max()} == weekday{local_days::max() - days{7}}); } constexpr void weekday_indexed_test() { From f535837b0217e6d5b8e3b340220de31bd909c018 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Fri, 29 Nov 2024 19:09:35 -0800 Subject: [PATCH 2/2] Casey's code review suggestions Clarify some comments, and add a `static_assert` to catch if the code is ported somewhere for which `unsigned int` isn't 32 bits. --- stl/inc/chrono | 3 ++- .../std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 07e0589a41a..e8e701c39c5 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -476,8 +476,9 @@ namespace chrono { // courtesy of Howard Hinnant (modified to avoid overflow) // https://howardhinnant.github.io/date_algorithms.html#weekday_from_days _NODISCARD static constexpr unsigned int _Weekday_from_days(int _Tp) noexcept { + _STL_INTERNAL_STATIC_ASSERT(~0u % 7u == 3u); // offset for `_Tp < 0` needs to change const auto _Before_modulo = static_cast(_Tp) + (_Tp >= 0 ? 4u : 0u); - return _Before_modulo % 7u; // for codegen by MSVC on x86/ARM64, see GH-5153 + return _Before_modulo % 7u; // separate expression for MSVC codegen, see GH-5153 } }; diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp index 6558238e24b..745eb7b3315 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_dates/test.cpp @@ -282,7 +282,7 @@ constexpr void weekday_test() { assert(Sunday - Tuesday == days{5}); assert(Wednesday - Thursday == days{6}); - // GH-5153 ": Undefined behaviour in weekday::weekday(const sys_days&)." + // GH-5153 ": integer overflow in weekday::weekday(sys_days::max())" assert(weekday{sys_days::max()} == weekday{sys_days::max() - days{7}}); assert(weekday{local_days::max()} == weekday{local_days::max() - days{7}}); }