`<chrono>`/`<format>`: Using `milliseconds::max()` triggers icx' UBSan

[bersbersbers]

Describe the bug

Formatting std::chrono::milliseconds::max() using std::format triggers icx' UBSan. Output below.

There is an integer overflow in converting std::chrono::milliseconds::max() to std::chrono::hh_mm_ss.

Note: I did not expect std::format("{:%Q}", ...) to even trigger conversion to std::chrono::hh_mm_ss.

Command-line test case

C:\Temp>type repro.cpp
#include <chrono>
#include <format>

int main() {
    auto duration = std::chrono::milliseconds::max();
    auto unit = std::format("{:%Q}", duration);
    std::chrono::hh_mm_ss time(duration);
}

C:\Temp>"C:\Program Files (x86)\Intel\oneAPI\compiler\2024.2\bin\icx" repro.cpp -o repro.exe -std:c++20 -D_MSVC_STL_DOOM_FUNCTION(x)=__debugbreak() -fsanitize=undefined /link /NODEFAULTLIB:libmmt.lib /LIBPATH:"C:\Program Files (x86)\Intel\oneAPI\compiler\2024.2\lib"
Intel(R) oneAPI DPC++/C++ Compiler for applications running on Intel(R) 64, Version 2024.2.0 Build 20240602
Copyright (C) 1985-2024 Intel Corporation. All rights reserved.

   Creating library repro.lib and object repro.exp

C:\Temp>.\repro.exe
C:\Program Files\Microsoft Visual Studio\18\Professional\VC\Tools\MSVC\14.50.35717\include\__msvc_chrono.hpp:300:39: runtime error: signed integer overflow: 9223372036854775807 - -7371675709200000 cannot be represented in type 'long long'
    #0 0x7ff7839212bd  (C:\Temp\repro.exe+0x1400012bd)
    #1 0x7ff7839420e3  (C:\Temp\repro.exe+0x1400220e3)
    #2 0x7ff78392301a  (C:\Temp\repro.exe+0x14000301a)
    #3 0x7ff783922753  (C:\Temp\repro.exe+0x140002753)
    #4 0x7ff783921b80  (C:\Temp\repro.exe+0x140001b80)
    #5 0x7ff78392149d  (C:\Temp\repro.exe+0x14000149d)
    #6 0x7ff783921092  (C:\Temp\repro.exe+0x140001092)
    #7 0x7ff7839795eb in __ubsan_handle_cfi_bad_type (C:\Temp\repro.exe+0x1400595eb)
    #8 0x7ffc9ac8e8d6  (C:\WINDOWS\System32\KERNEL32.DLL+0x18002e8d6)
    #9 0x7ffc9ce8c3fb  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18008c3fb)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior C:\Program Files\Microsoft Visual Studio\18\Professional\VC\Tools\MSVC\14.50.35717\include\__msvc_chrono.hpp:300:39

Expected behavior

UBSan not triggering

STL version

202508 (14.50.35717)

[frederick-vs-ja]

IIUC we correctly follow the specification in [time.hms.members]/3 for hh_mm_ss<milliseconds>. The "divergence", caused by _Duration_underflows_with is inactive in this case.

So such UB can be considered specified by the standard. Perhaps we should consider submitting an LWG issue for this.

[MattStephanson]

The reason that the conversion to hh_mm_ss is done is that a duration can be formatted with time-of-day specifiers like %H, %M, and %S. So it's useful to convert it, and the several other chrono types, to a tm object and defer most of the formatting work to put_time. The result is that we eagerly try to convert to a tm, which happens to make a layover at hh_mm_ss on the way. libcxx uses a similar design (https://github.com/llvm/llvm-project/blob/0ad1bc96429863fe9fa65706df9a86cec649bf60/libcxx/include/__chrono/formatter.h#L255).

I suppose you could peek at the specifier and make a special case for %Q, but that wouldn't completely solve the issue because other specifiers are available. For durations longer than hours::max(), maybe you'd have to resort to 128-bit arithmetic to get the HMS parts modulo 86,400 s.

[bersbersbers]

Thanks for the answers. I understand that hh_mm_ss is likely implemented correctly, and that the fact that std::format converts to hh_mm_ss is unfortunate when not strictly needed but generally necessary.

I can add on of my own findings, since I didn't really understand where the negative hours were coming from in the first place. I incorrectly assumed that milliseconds::max() would always be a shorter time than hours::max(). That is not true in the Microsoft STL. Different from other major implementations, it uses 64-bit milliseconds but only 32-bit hours: https://godbolt.org/z/6hzePoYjW

I think that's the major ingredient that leads to this issue. If hours::max() is longer than milliseconds::max(), there is no problem.

Update: my personal workaround will be to define duration_cast<milliseconds>(nanoseconds::max()) in the code and use that as a safe, all-convertible maximum value.