nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
43.15k stars 6.73k forks source link

The MSVC team recently test JSON project failed to run test on release configuration on windows_x64. #3542

Open Vincent9802 opened 2 years ago

Vincent9802 commented 2 years ago

Description

Hi All,

JSON failed to run test 'test-diagnostics_cpp11','test-json_pointer_cpp11' and 'test-unicode1_cpp11'on release configuration with MSVC on windows x64. Could you please help look at this issue or provide some workarounds? Thanks in advance.

Reproduction steps

  1. git clone https://github.com/nlohmann/json.git F:\gitP\nlohmann
  2. set VSCMD_SKIP_SENDTELEMETRY=1 & "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -host_arch=amd64 -arch=amd64 & set CL= /fsanitize=address /GS- /wd5072 & set LINK= /InferASanLibs /incremental:no /debug
  3. mkdir&cd F:\gitP\nlohmann\json\build_amd64
  4. cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_SYSTEM_VERSION=10.0.18362.0 -DJSON_BuildTests=On .. 2>&1
  5. msbuild /m /p:Platform=x64 /p:Configuration=Release nlohmann_json.sln /t:Rebuild 2>&1
  6. ctest -C Release --output-on-failure 2>&1

Expected vs. actual results

Expected test pass Actual results : The following tests FAILED: 8 - test-bson_cpp11 (Failed) 25 - test-diagnostics_cpp11 (Failed) 38 - test-json_pointer_cpp11 (Failed) 61 - test-unicode1_cpp11 (Failed) Errors while running CTest

Minimal code example

1.C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\link.exe /ERRORREPORT:QUEUE /OUT:"F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe" /INCREMENTAL:NO /NOLOGO /LIBPATH:"F:\gitP\bitcoin\tools\vcpkg\scripts\buildsystems\msbuild\..\..\..\installed\x64-windows\lib" /LIBPATH:"F:\gitP\bitcoin\tools\vcpkg\scripts\buildsystems\msbuild\..\..\..\installed\x64-windows\lib\manual-link" /NATVIS:F:\gitP\nlohmann\json\nlohmann_json.natvis kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib "F:\gitP\bitcoin\tools\vcpkg\scripts\buildsystems\msbuild\..\..\..\installed\x64-windows\lib\*.lib" /MANIFEST /MANIFESTUAC:"level='asInvoker' uiAccess='false'" /manifest:embed /PDB:"F:/gitP/nlohmann/json/build_amd64/tests/Release/test-diagnostics_cpp11.pdb" /SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT /IMPLIB:"F:/gitP/nlohmann/json/build_amd64/tests/Release/test-diagnostics_cpp11.lib" /MACHINE:X64  /machine:x64 "test-diagnostics_cpp11.dir\Release\unit-diagnostics.obj" F:\gitP\nlohmann\json\build_amd64\tests\test_main.dir\Release\unit.obj

2.test-diagnostics_cpp11.exe

Error messages

==86508==ERROR: AddressSanitizer: container-overflow on address 0x00e159afb444 at pc 0x7ffc23ec68fc bp 0x00e159afadb0 sp 0x00e159afa530
WRITE of size 2 at 0x00e159afb444 thread T0
    #0 0x7ffc23ec6929  (C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\clang_rt.asan_dynamic-x86_64.dll+0x180046929)
    #1 0x7ff6ed53b9dc in std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>::replace(unsigned __int64, unsigned __int64, char const *const, unsigned __int64) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x1400bb9dc)
    #2 0x7ff6ed53ad42 in std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>::replace(unsigned __int64, unsigned __int64, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x1400bad42)
    #3 0x7ff6ed4ebefd in nlohmann::detail::replace_substring<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x14006befd)
    #4 0x7ff6ed4d7077 in nlohmann::detail::escape<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x140057077)
    #5 0x7ff6ed4bc83a in std::accumulate<class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, class <lambda_0b07af80a1b5e289a8e1fd0031570a68>>(class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, class <lambda_0b07af80a1b5e289a8e1fd0031570a68>) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x14003c83a)
    #6 0x7ff6ed4d40af in nlohmann::detail::exception::diagnostics<class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>>>(class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>> const *) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x1400540af)
    #7 0x7ff6ed4ca21a in nlohmann::detail::type_error::create<class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>> const *, 0>(int, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &, class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>> const *) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x14004a21a)
    #8 0x7ff6ed4d7642 in nlohmann::detail::from_json<class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>>>(class nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>> const &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x140057642)
    #9 0x7ff6ed4d8aa1 in nlohmann::basic_json<class std::map, class std::vector, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, bool, __int64, unsigned __int64, double, class std::allocator, struct nlohmann::adl_serializer, class std::vector<unsigned char, class std::allocator<unsigned char>>>::get_impl<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, 0>(struct nlohmann::detail::priority_tag<0>) const (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x140058aa1)
    #10 0x7ff6ed485740  (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x140005740)
    #11 0x7ff6ed55dc93 in doctest::Context::run(void) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x1400ddc93)
    #12 0x7ff6ed58b627 in main (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x14010b627)
    #13 0x7ff6ed5dd20f in __scrt_common_main_seh D:\a01\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #14 0x7ffc55c57973  (C:\Windows\System32\KERNEL32.DLL+0x180017973)
    #15 0x7ffc58a0a2f0  (C:\Windows\SYSTEM32\ntdll.dll+0x18005a2f0)

Address 0x00e159afb444 is located in stack of thread T0 at offset 164 in frame
    #0 0x7ff6ed4bbfaf in std::accumulate<class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, class <lambda_0b07af80a1b5e289a8e1fd0031570a68>>(class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::reverse_iterator<class std::_Vector_iterator<class std::_Vector_val<struct std::_Simple_types<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>>>>, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>, class <lambda_0b07af80a1b5e289a8e1fd0031570a68>) (F:\gitP\nlohmann\json\build_amd64\tests\Release\test-diagnostics_cpp11.exe+0x14003bfaf)

  This frame has 8 object(s):
    [32, 40) '_UFirst'
    [48, 56) '_ULast'
    [64, 96) 'compiler temporary'
    [80, 88) '_Right'
    [96, 104) '_Right'
    [112, 144) 'compiler temporary'
    [128, 160) 'compiler temporary'
    [144, 145) 'compiler temporary' <== Memory access at offset 164 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp, SEH and C++ exceptions *are* supported)
HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_container_overflow=0.
If you suspect a false positive see also: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow.
SUMMARY: AddressSanitizer: container-overflow (C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\clang_rt.asan_dynamic-x86_64.dll+0x180046929) 
Shadow bytes around the buggy address:
  0x02f027bdf630: 00 00 f1 f1 f1 f1 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8
  0x02f027bdf640: f8 f8 f2 f2 f2 f2 03 fc 00 00 f2 f2 f2 f2 02 fc
  0x02f027bdf650: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
  0x02f027bdf660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x02f027bdf670: 00 00 00 00 f1 f1 f1 f1 00 f2 00 f2 00 00 00 00
=>0x02f027bdf680: f2 f2 f2 f2 00 f2 00 f2[04]fc 00 00 f2 f2 f2 f2
  0x02f027bdf690: 00 00 00 00 f2 f2 f2 f2 01 f3 f3 f3 f3 00 00 00
  0x02f027bdf6a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x02f027bdf6b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x02f027bdf6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x02f027bdf6d0: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==86508==ABORTING

Compiler and operating system

Visual Studio 2019, Window10

Library version

git commit : af34396

Validation

Vincent9802 commented 2 years ago

JSON_amd64.zip build and test log file

nlohmann commented 2 years ago

Thanks for reporting. Can you please try Release mode just to be sure?

falbrechtskirchinger commented 2 years ago

The error seems to be happening in replace_substring which I've recently modified. Could you re-run your test with the latest release version (3.10.5) and let me know if the error exists there as well?

falbrechtskirchinger commented 2 years ago

Thanks for reporting. Can you please try Release mode just to be sure?

Do you mean Debug?

falbrechtskirchinger commented 2 years ago

There's a high likelihood that this is a false positive.

Are you sure what you're doing is supported? I can find documentation that specifically states only x86 is supported (but that may have changed in subsequent releases).

nlohmann commented 2 years ago

Thanks for reporting. Can you please try Release mode just to be sure?

Do you mean Debug?

No, Release. We sometimes had issues that MSVC took too much memory or even had a stack overflow when executing certain tests in Debug mode.

falbrechtskirchinger commented 2 years ago

Thanks for reporting. Can you please try Release mode just to be sure?

Do you mean Debug?

No, Release. We sometimes had issues that MSVC took too much memory or even had a stack overflow when executing certain tests in Debug mode.

I ask because this bug is specific to an amd64 Release build. Also, MSVC ASAN is apparently only supported by i368 Release builds. I'm just not sure if that has changed in subsequent VS 2019 releases (I suspect not, because the MS blog post was last updated in 2021).

Since ASAN is not finding any issues on Linux and container-overflow is known to produce false positives because of missing ASAN annotations or mixing standard library code built with and without ASAN due to inlining, I'm confident that this bug report is invalid.

Additionally, the code in question is not particularly complicated and some of the parameters are literals. I don't see how it could fail.

With StringType = std::string :

template<typename StringType>
inline void replace_substring(StringType& s, const StringType& f,
                              const StringType& t)
{
    JSON_ASSERT(!f.empty());
    for (auto pos = s.find(f);                // find first occurrence of f
            pos != StringType::npos;          // make sure f was found
            s.replace(pos, f.size(), t),      // replace with t, and
            pos = s.find(f, pos + t.size()))  // find next occurrence of f
    {}
}

template<typename StringType>
inline StringType escape(StringType s)
{
    replace_substring(s, StringType{"~"}, StringType{"~0"});
    replace_substring(s, StringType{"/"}, StringType{"~1"});
    return s;
}
Vincent9802 commented 2 years ago

Thanks for reporting. Can you please try Release mode just to be sure?

Do you mean Debug?

No, Release. We sometimes had issues that MSVC took too much memory or even had a stack overflow when executing certain tests in Debug mode.

Thanks for your reply and suggestion. This error was found when compiling this project in the latest development version of VS, and is only reproduced in ASAN mode. the same error is reported in debug mode, I apologize for not clarifying this issue, I am trying to solve the problem by setting ASAN_OPTIONS=detect_container_overflow=0.

nlohmann commented 2 years ago

Please try also in Release mode.

falbrechtskirchinger commented 2 years ago

Please try also in Release mode.

Niels, he did. It's in the title. :-) That's why I had to ask for clarification the last time.

nlohmann commented 2 years ago

Oh boy, sorry for the confusion…

spacelg commented 2 years ago

The error seems to be happening in replace_substring which I've recently modified. Could you re-run your test with the latest release version (3.10.5) and let me know if the error exists there as well?

@falbrechtskirchinger I've tried the latest release version b5364fa of release/3.10.5 branch. But the error still exist. And the failed error are same.

As you said 'The error seems to be happening in replace_substring which I've recently modified.' Can you provide a link to your commit for your changes? We want to try to see if the commit before this modification has this problem.

Thanks, Lin

falbrechtskirchinger commented 2 years ago

@falbrechtskirchinger I've tried the latest release version b5364fa of release/3.10.5 branch. But the error still exist. And the failed error are same.

As you said 'The error seems to be happening in replace_substring which I've recently modified.' Can you provide a link to your commit for your changes? We want to try to see if the commit before this modification has this problem.

If you've tested 3.10.5, you've already done so. My change is unreleased and also immaterial. I still think this bug report is (probably) invalid for the reasons stated above and I'm only qualifying my response because I haven't personally checked the MSSTL/libstdc++ sources for ASan annotations.

gregmarr commented 2 years ago

The only thing I can see is that asan may be tripping on replacing the last character of the string with a two-character string. If it's not detecting that the string container has already been expanded to accept that, it would detect that as an overflow error.

falbrechtskirchinger commented 2 years ago

But that would still be an issue outside the library, right? AFAICT, std::string::replace should deal with resizing the string.

I did grep through libstdc++ trunk, and std::vector is still the only annotated container. Maybe I'll check MSSTL next.

Edit: MSSTL does appear to have annotations. The unit test for string annotations tests virtually every string function but replace().

gregmarr commented 2 years ago

Correct, I don't see anything wrong with the library code, just trying to figure out what it might be detecting.