`<chrono>`: `format("%c", time_point)` can lead to crash

[achabense]

This is a follow-up of #924. The same crash can happen even with format, as it will result in a call to put_time(which will make call to functions like _Strftime):

#include<format>
#include<iostream>
#include<chrono>
int main() {
	using namespace std::chrono;
	try {
		sys_time<seconds> t{seconds(335303598060)};
		std::cout << std::format("{:%c}", t);
	}
	catch (std::exception& e) {
		std::cout << e.what();// unreacheable.
	}
	return 0;
}

STL/stl/inc/chrono

Line 5470 in c8d1efb

_Stream << _STD put_time<_CharT>(&_Time, _Fmt_string(_Spec, _Fmt_str));

Instead of an exception, the program will give a crashing error:

Unhandled exception at 0x00007FF80ACA1208 (ucrtbase.dll) in test.exe:
An invalid parameter was passed to a function that considers invalid parameters fatal.

Please consider doing some investigations for strftime series. The crash-on-failed-check behavior interacts really poorly with C++.

[achabense]

Here is a rough test for strftime. The lines that are not commented-out are exceptions that will not crash the program.

Code to test

int test() {

tm t{ -1,-1,-1,-1,-1,-1,-1,-1,-1 };
char buffer[100];

// Year
strftime(buffer, 100, "%Y", &t);
strftime(buffer, 100, "%EY", &t);
strftime(buffer, 100, "%y", &t);
//strftime(buffer, 100, "%0y", &t);//Shows "Expression: false" in debug mode.
strftime(buffer, 100, "%Ey", &t);
strftime(buffer, 100, "%C", &t);
strftime(buffer, 100, "%EC", &t);
strftime(buffer, 100, "%G", &t);
strftime(buffer, 100, "%g", &t);

// Month
//strftime(buffer, 100, "%b", &t);
//strftime(buffer, 100, "%h", &t);
//strftime(buffer, 100, "%B", &t);
//strftime(buffer, 100, "%m", &t);
//strftime(buffer, 100, "%0m", &t);

// Week
//strftime(buffer, 100, "%U", &t);
//strftime(buffer, 100, "%0U", &t);
//strftime(buffer, 100, "%W", &t);
//strftime(buffer, 100, "%0W", &t);
strftime(buffer, 100, "%V", &t);
//strftime(buffer, 100, "%0V", &t);

// Day of the year/month
//strftime(buffer, 100, "%j", &t);
//strftime(buffer, 100, "%d", &t);
//strftime(buffer, 100, "%0d", &t);
//strftime(buffer, 100, "%e", &t);
//strftime(buffer, 100, "%0e", &t);

// Day of the week
//strftime(buffer, 100, "%a", &t);
//strftime(buffer, 100, "%A", &t);
//strftime(buffer, 100, "%w", &t);
//strftime(buffer, 100, "%0w", &t);
//strftime(buffer, 100, "%u", &t);
//strftime(buffer, 100, "%0u", &t);

// Hour, minute, second
//strftime(buffer, 100, "%H", &t);
//strftime(buffer, 100, "%0H", &t);
//strftime(buffer, 100, "%I", &t);
//strftime(buffer, 100, "%0I", &t);
//strftime(buffer, 100, "%M", &t);
//strftime(buffer, 100, "%0M", &t);
//strftime(buffer, 100, "%S", &t);
//strftime(buffer, 100, "%0S", &t);

// Other
//strftime(buffer, 100, "%c", &t);
//strftime(buffer, 100, "%Ec", &t);
//strftime(buffer, 100, "%x", &t);
//strftime(buffer, 100, "%Ex", &t);
//strftime(buffer, 100, "%X", &t);
//strftime(buffer, 100, "%EX", &t);
//strftime(buffer, 100, "%D", &t);
//strftime(buffer, 100, "%F", &t);
//strftime(buffer, 100, "%r", &t);
//strftime(buffer, 100, "%R", &t);
//strftime(buffer, 100, "%T", &t);
//strftime(buffer, 100, "%p", &t);
strftime(buffer, 100, "%z", &t);
strftime(buffer, 100, "%Z", &t);

return 0;

}

[StephanTLavavej]

Thanks for the test cases. The first thing we need to understand is whether this is a bug in the UCRT strftime implementation according to its documented behavior, in which case we need to report it to the UCRT maintainers.

[MattStephanson]

sys_time<seconds> t{seconds(335303598060)};
std::cout << std::format("{:%c}", t);

This is a time in the year 12595. The UCRT only considers the years [0, 9999] valid, see Windows Kits\10\Source\10.0.22000.0\ucrt\time\wcsftime.cpp, line 963. POSIX.1-2017 adds "The %Y conversion specification to strftime() was frequently assumed to be a four-digit year, but the ISO C standard does not specify that %Y is restricted to any subset of allowed values from the tm_year field". So, this part appears to be a UCRT bug.

tm t{ -1,-1,-1,-1,-1,-1,-1,-1,-1 };
...

All of these tm fields are invalid except for tm_year, so I'm not sure what you're demonstrating here. "%0y", however, doesn't work because the UCRT only claims to conform to C99 and a subset of POSIX.1-1996, neither of which recognize the 0 flag.
Edit: I see your point is that you think these should throw instead of crashing, although I don't see anything in locale.time.put that specifies what the error handling should be.

[achabense]

The main problem is that in format, there has been a lot of effort to get rid of the crash behavior posed by put_time/strftime. I think we hope to completely avoid a direct crash for format.
As to the test for strftime, it's a copy of the list in ref. I admit that it is of little value except for the strftime(buffer, 100, "%V", &t); case(I wasn't aware that year can be -1 and "%0*"s are not supported here).
Yes, I think for strftime, a crash is too much for a failed time assumption, especially as there are a lot of other functions relying on it.
@MattStephanson Thanks for pointing out where I can find the source file 👀.

[achabense]

Unlike "%U/W" cases there is no similar check for "%V" case.

[achabense]

Below is a more exhaustive list that shows all specifiers that crash on sys_time<seconds>{seconds(335303598060)} due to usage of put_time. Aside from those commented-out cases, a lot of successful cases are actually tackled by _Custom_write instead of put_time.
I'm temporarily closing this issue, as though I think it's highly undesirable behavior for std::format to uncatchably crash on these cases, it seems that this is still conformant to the standard. (I will reopen it if I find something that indicates this violates the standard)

#include<format>
#include<iostream>
#include<chrono>
#include<ranges>

//lists from https://en.cppreference.com/w/cpp/chrono/system_clock/formatter

// invalid by the standard
const auto specs_invalid = {
    "Q","q"
};

const auto specs = {
    "C","y","Y","b","h","B","m","d","e","a","A","u","w",
    //"g", // crash due to put_time
    //"G", // crash due to put_time
    "V","j","U","W","D","F",
    //"x", // crash due to put_time
    "H","I","M","S","p","R","T","r","X","z","Z",
    //"c", // crash due to put_time
};

// O
const auto specs_O = {
    //"Oy", // crash due to put_time
    "Om","Od","Oe","Oe","Ow","OV","OU","OW","OH","OI","OM","OS","Oz"
};

// E
const auto specs_E = {
    //"EC", // crash due to put_time
    //"Ey", // crash due to put_time
    //"EY", // crash due to put_time
    //"Ex", // crash due to put_time
    "EX","Ez",
    // "Ec", // crash due to put_time
};

int main() {
    using namespace std::chrono;
    try {
        sys_time<seconds> t{ seconds(335303598060) };
        auto fmt_arg = make_format_args(t);
        auto all_specs = { specs,specs_O,specs_E };
        for (auto str : std::views::join(all_specs)) {
            char fmt[10];
            sprintf_s(fmt, "{:%%%s}", str);
            printf("trying %s", fmt);
            auto str = std::vformat(fmt, fmt_arg);
            puts(str.c_str());
        }
    }
    catch (std::exception& e) {
        std::cout << e.what();
    }
}

[TheStormN]

@achabense Although you've closed the issue it seems I've addressed it in #4840 and #4883 (the second PR is still in development). Tried the code from your last comment locally and it does not crash anymore.

Update: Fixed!