Azure / azure-storage-cpp

Microsoft Azure Storage Client Library for C++
http://azure.github.io/azure-storage-cpp
Apache License 2.0
131 stars 147 forks source link

utility::datetime.to_string() is breaking with debug break (version 7.5.0) #379

Open surendernitj opened 3 years ago

surendernitj commented 3 years ago

I upgraded from 5.2.0 to 7.5.0, build is green but utility::datetime.to_string() is breaking. Here is the stack

Program: c:\ut\TableMetadataStoreTests\TE.exe File: minkernel\crts\ucrt\src\appcrt\stdio\output.cpp Line: 273

Expression: ("Buffer too small", 0)

For information on how your program can cause an assertion failure, see the Visual C++ documentation on asserts.

(3980.7ce0): Break instruction exception - code 80000003 (first chance)
KERNELBASE!DebugBreak [inlined in KERNELBASE!wil::details::DebugBreak+0x2]:
00007ff8`ee2f9ad2 cc              int     3
0:005> k
 # Child-SP          RetAddr               Call Site
00 (Inline Function) --------`--------     KERNELBASE!DebugBreak [minkernel\kernelbase\debug.c @ 143] 
01 00000015`353f2098 00007ff8`8282aacd     KERNELBASE!wil::details::DebugBreak+0x2 [onecore\internal\sdk\inc\wil\opensource\wil\result_macros.h @ 1737] 
02 00000015`353f20a0 00007ff8`82920138     FileSyncMsf+0xaacd
03 00000015`353f20d0 00007ff8`82920311     FileSyncMsf+0x100138
04 00000015`353f2120 00007ff8`f0aa7cad     FileSyncMsf+0x100311
05 00000015`353f2150 00007ff8`f0ad3ff1     ntdll!LdrpCallInitRoutine+0x61 [minkernel\ntdll\ldr.c @ 212] 
06 00000015`353f21c0 00007ff8`f0ad3e8d     ntdll!LdrShutdownProcess+0x141 [minkernel\ntdll\ldrinit.c @ 6213] 
07 00000015`353f22c0 00007ff8`f074e0ab     ntdll!RtlExitUserProcess+0xad [minkernel\ntdll\rtlstrt.c @ 1571] 
08 00000015`353f22f0 00007ff8`7d1d933a     KERNEL32!FatalExit+0xb [clientcore\base\win32\client\process.c @ 2421] 
09 00000015`353f2320 00007ff8`7d1d92f2     ucrtbased!exit_or_terminate_process+0x3a [d:\th\minkernel\crts\ucrt\src\appcrt\startup\exit.cpp @ 130] 
0a 00000015`353f2350 00007ff8`7d1d9549     ucrtbased!common_exit+0x102 [d:\th\minkernel\crts\ucrt\src\appcrt\startup\exit.cpp @ 271] 
0b 00000015`353f23a0 00007ff8`7d1d0ff5     ucrtbased!_exit+0x19 [d:\th\minkernel\crts\ucrt\src\appcrt\startup\exit.cpp @ 293] 
0c 00000015`353f23d0 00007ff8`7d1cd15f     ucrtbased!raise+0x2e5 [d:\th\minkernel\crts\ucrt\src\appcrt\misc\signal.cpp @ 468] 
0d 00000015`353f24a0 00007ff8`7d1cde93     ucrtbased!common_message_window<wchar_t>+0x6af [d:\th\minkernel\crts\ucrt\src\appcrt\misc\dbgrpt.cpp @ 414] 
0e 00000015`353f4830 00007ff8`7d1cf87d     ucrtbased!__acrt_MessageWindowW+0x43 [d:\th\minkernel\crts\ucrt\src\appcrt\misc\dbgrpt.cpp @ 452] 
0f 00000015`353f4870 00007ff8`7d1cdd75     ucrtbased!_VCrtDbgReportW+0xb5d [d:\th\minkernel\crts\ucrt\src\appcrt\misc\dbgrptt.cpp @ 642] 
10 00000015`353fc970 00007ff8`7d1f6cd9     ucrtbased!_CrtDbgReportW+0x65 [d:\th\minkernel\crts\ucrt\src\appcrt\misc\dbgrpt.cpp @ 273] 
11 00000015`353fc9d0 00007ff8`7d2163b5     ucrtbased!common_vsprintf_s<char>+0x2b9 [d:\th\minkernel\crts\ucrt\src\appcrt\stdio\output.cpp @ 273] 
12 00000015`353fca50 00007ff8`681d586e     ucrtbased!__stdio_common_vsprintf_s+0x45 [d:\th\minkernel\crts\ucrt\src\appcrt\stdio\output.cpp @ 293] 
13 00000015`353fca90 00007ff8`681d5923     cpprest_2_10d!_vsprintf_s_l+0x5e [c:\program files (x86)\windows kits\10\include\10.0.10240.0\ucrt\stdio.h @ 1494] 
14 00000015`353fcae0 00007ff8`683364d8     cpprest_2_10d!sprintf_s+0x63 [c:\program files (x86)\windows kits\10\include\10.0.10240.0\ucrt\stdio.h @ 1838] 
15 00000015`353fcb50 00007ff8`670dc009     cpprest_2_10d!utility::datetime::to_string+0x418 [c:\users\jamis\desktop\vcpkg\buildtrees\cpprestsdk\src\v2.10.16-e7322b2d28.clean\release\src\utilities\asyncrt_utils.cpp @ 822]
Jinming-Hu commented 3 years ago

Hi @surendernitj, how did you install storage sdk and cpprest sdk, via vcpkg or nuget?

surendernitj commented 3 years ago

Hi @surendernitj, how did you install storage sdk and cpprest sdk, via vcpkg or nuget?

I installled it via nuget package.

Jinming-Hu commented 3 years ago

Hi @surendernitj, how did you install storage sdk and cpprest sdk, via vcpkg or nuget?

I installled it via nuget package.

Well, can you try 7.5.0.2 and see if this issue persists? 7.5.0 has a severe issue.

If this issues still exists in 7.5.0.2, can you share some code snippet that can reproduce this issue so that we can investigate?

surendernitj commented 3 years ago

I dont see 7.5.0.2 version in the release notes. I will try few older versions to see if it works. Sample code that breaks is:

utility::datetime time; utility::string_t s = time.to_string(utility::datetime::ISO_8601);

surendernitj commented 3 years ago

Its the same issue with 7.5.0.2 version. Its most probably related to casablanca and it seems its an issue with the min datetime.

Jinming-Hu commented 3 years ago
utility::datetime time;
utility::string_t s = time.to_string(utility::datetime::ISO_8601);

The default constructor of utility::datetime initializes it into uninitialized state. Converting such a datetime to string would be undefined behavior.

surendernitj commented 3 years ago

Code here https://microsoft.github.io/cpprestsdk/asyncrt__utils_8h_source.html says that default constructor is:

datetime() : m_interval(0) { }

It this changed now? This code used to work fine but its breaking with the upgrade.

Jinming-Hu commented 3 years ago

m_internal(0) means an uninitialized state. You can search bool is_initialized() const in the page.

Jinming-Hu commented 3 years ago

let me ask you a question, what do you expect to get from that piece of code?

utility::datetime time;
utility::string_t s = time.to_string(utility::datetime::ISO_8601);
surendernitj commented 3 years ago

The product code it not just this piece of code. I shared it in very simplified form. With code similar to this we want to set the minimum filetime (1601-01-01T00:00:00Z). Actual code is something similar to:

       entity_property   Prop;
        utility::datetime LeaseTime;
        LeaseTime = LeaseTime + FILETIME_AS_ULONGLONG(ft);  // ft is FILETIME which could be 0 (min filetime)

        Prop.set_property_type(edm_type::datetime);
        Prop.set_value( LeaseTime );

Could you please help me understand how i can set the minimum file time value for (1601-01-01T00:00:00Z) utility:datetime object.

I tried setting is by utility::datetime::from_string("1601-01-01T00:00:00Z") but it still fails when later on datetime is converted into string.

Jinming-Hu commented 3 years ago

The issue here is 1601-01-01T00:00:00Z is invalid. The minimum timestamp that can be represented with utility::datetime is 100 nanoseconds after that.

Can you try utility::datetime::from_string("1601-01-01T00:00:01Z")?

or something like utility::datetime::from_string("1601-01-01T00:00:00.0000001Z")?

surendernitj commented 3 years ago

Nope, 1601-01-01T00:00:01Z does not work. I am using different base value 2000-01-01T00:00:00Z which works for me. Clearly its a breaking change.

surendernitj commented 3 years ago

It seems following change broke it https://github.com/microsoft/cpprestsdk/commit/f10d9f8e214516d2c19aa6ef831ee874a58c0479

and as per the test, bound conditions are // boundary cases: TestDateTimeRoundtrip(_XPLATSTR("1970-01-01T00:00:00Z")); // epoch TestDateTimeRoundtrip(_XPLATSTR("2038-01-19T03:14:06+00:00"), _XPLATSTR("2038-01-19T03:14:06Z")); // INT_MAX - 1