https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109772
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Jonathan Wakely <r...@gcc.gnu.org>: https://gcc.gnu.org/g:aa39ed4467db0cf18f300fcf475e09c4496527cf commit r14-740-gaa39ed4467db0cf18f300fcf475e09c4496527cf Author: Jonathan Wakely <jwak...@redhat.com> Date: Wed May 10 16:15:03 2023 +0100 libstdc++: Fix chrono::hh_mm_ss::subseconds() [PR109772] I borked the logic in r13-4526-g5329e1a8e1480d so that the selected partial specialization of hh_mm_ss::__subseconds might not be able to represent the correct number of subseconds. This can result in a truncated value being stored for the subseconds, e.g., 4755859375 gets truncated to 460892079 because the correct value doesn't fit in uint_least32_t. Instead of checking whether the maximum value of the incoming duration type can be represented, we would need to check whether that maximum value can be represented after being converted to the correct precision type: template<typename _Tp> static constexpr bool __fits = duration_cast<precision>(_Duration::max()).count() <= duration_values<_Tp>::max(); However, this can fail to compile, due to integer overflow in the constexpr multiplications. Instead, we could limit the check to the case where the incoming duration has the same period as the precision, where no conversion is needed and so no overflow can happen. But that seems of very limited value, as it would only benefit specializations like hh_mm_ss<duration<int, std::pico>>, which can only represent a time-of-day between -00:00:00.0215 and +00:00:00.0215 measured in picoseconds! Additionally, the hh_mm_ss::__subseconds partial specializations do not have disjoint constraints, so that some hh_mm_ss specializations result in ambiguities tying to match a __subseconds partial specialization. The most practical fix is to just stop using the __fits variable template in the constraints of the partial specializations. This fixes the truncated values by not selecting an inappropriate partial specialization, and fixes the ambiguous match by ensuring the constraints are disjoint. Fixing this changes the layout of some specializations, so is an ABI change. It only affects specializations that have a small (less than 64-bit) representation type and either a very small period (e.g. like the picosecond specialization above) or a non-power-of-ten period like ratio<1, 1024>. For example both hh_mm_ss<duration<int, std::pico>> and hh_mm_ss<duration<int, ratio<1, 1024>> are affected (increasing from 16 bytes to 24 on x86_64), but hh_mm_ss<duration<int, ratio<1, 1000>> and hh_mm_ss<duration<long, ratio<1, 1024>> are not affected. libstdc++-v3/ChangeLog: PR libstdc++/109772 * include/std/chrono (hh_mm_ss::__fits): Remove variable template. (hh_mm_ss::__subseconds): Remove __fits from constraints. * testsuite/std/time/hh_mm_ss/109772.cc: New test. * testsuite/std/time/hh_mm_ss/1.cc: Adjust expected size for hh_mm_ss<duration<int, std::pico>>.
