Bug 157067 – [WinCairo] Invalid address specified to RtlValidateHeap at std::ctype<char>::_Tidy() when finishing MiniBrowser

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Fujii Hironori]

[WinCairo] Invalid address specified to RtlValidateHeap at std::ctype<char>::_Tidy() when finishing MiniBrowser

trunk@200119, WinCairo port, Debug build

1) Launch MiniBrowser
2) Close the window
3) Invalid address specified to RtlValidateHeap

This issue doesn't happen in every builds.
I think this doesn't happen with a clean build, but a incremental build.
If a build have this issue, this steps reproduces every time.

Log:

> HEAP[MiniBrowser.exe]: Invalid address specified to RtlValidateHeap( 000001A0F2C60000, 000001A0F812E7E0 )
> MiniBrowser.exe has triggered a breakpoint.

call stack:

> ntdll.dll!00007ffa314101c5()	Unknown
> ntdll.dll!00007ffa313e128f()	Unknown
> ntdll.dll!00007ffa31395702()	Unknown
> KernelBase.dll!00007ffa2dc4464a()	Unknown
> ucrtbased.dll!00007ffa1e5cdeb1()	Unknown
> ucrtbased.dll!00007ffa1e5cc259()	Unknown
> ucrtbased.dll!00007ffa1e5cf8a5()	Unknown
> ucrtbased.dll!00007ffa1e5cff88()	Unknown
> WebKit.dll!std::ctype<char>::_Tidy() Line 2503	C++
> WebKit.dll!std::ctype<char>::~ctype<char>() Line 2493	C++
> WebKit.dll!std::ctype<char>::`scalar deleting destructor'(unsigned int)	C++
> WebKit.dll!std::_Fac_node::`scalar deleting destructor'(unsigned int)	C++
> WebKit.dll!std::`dynamic atexit destructor for '_Fac_tidy_reg''()	C++
> ucrtbased.dll!00007ffa1e5f9b27()	Unknown
> ucrtbased.dll!00007ffa1e5f95a5()	Unknown
> ucrtbased.dll!00007ffa1e5f969c()	Unknown
> ucrtbased.dll!00007ffa1e5f9cb5()	Unknown
> WebKit.dll!__scrt_dllmain_uninitialize_c() Line 392	C++
> WebKit.dll!dllmain_crt_process_detach(const bool is_terminating) Line 109	C++
> WebKit.dll!dllmain_crt_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 134	C++
> WebKit.dll!dllmain_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 209	C++
> WebKit.dll!_DllMainCRTStartup(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 251	C++
> ntdll.dll!00007ffa31335208()	Unknown
> ntdll.dll!00007ffa3137b2aa()	Unknown
> ntdll.dll!00007ffa3137b13a()	Unknown
> kernel32.dll!00007ffa2f944d8a()	Unknown
> MiniBrowser.exe!exit_or_terminate_process(const unsigned int return_code) Line 129	C++
> MiniBrowser.exe!common_exit(const int return_code, const _crt_exit_cleanup_mode cleanup_mode, const _crt_exit_return_mode return_mode) Line 269	C++
> [External Code]

[Fujii Hironori]

Sometimes I see a different log message with a similar call stack.

Log:

> Debug Assertion Failed!
> 
> Program: ...8\work\webkit\trunk_c\WebKitBuild\Debug\bin64\MiniBrowser.exe
> File: minkernel\crts\ucrt\src\appcrt\heap\debug_heap.cpp
> Line: 892
> 
> Expression: is_block_type_valid(header->_block_use)
> 
> For information on how your program can cause an assertion
> failure, see the Visual C++ documentation on asserts.
> 
> (Press Retry to debug the application)
> MiniBrowser.exe has triggered a breakpoint.

Call stack:

> ucrtbased.dll!00007ff9f9dfc2f1()	Unknown
> ucrtbased.dll!00007ff9f9dff8a5()	Unknown
> ucrtbased.dll!00007ff9f9dfff88()	Unknown
> WebKit.dll!std::ctype<char>::_Tidy() Line 2503	C++
> WebKit.dll!std::ctype<char>::~ctype<char>() Line 2493	C++
> WebKit.dll!std::ctype<char>::`scalar deleting destructor'(unsigned int)	C++
> WebKit.dll!std::_Fac_node::`scalar deleting destructor'(unsigned int)	C++
> WebKit.dll!std::`dynamic atexit destructor for '_Fac_tidy_reg''()	C++
> ucrtbased.dll!00007ff9f9e29b27()	Unknown
> ucrtbased.dll!00007ff9f9e295a5()	Unknown
> ucrtbased.dll!00007ff9f9e2969c()	Unknown
> ucrtbased.dll!00007ff9f9e29cb5()	Unknown
> WebKit.dll!__scrt_dllmain_uninitialize_c() Line 392	C++
> WebKit.dll!dllmain_crt_process_detach(const bool is_terminating) Line 109	C++
> WebKit.dll!dllmain_crt_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 134	C++
> WebKit.dll!dllmain_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 209	C++
> WebKit.dll!_DllMainCRTStartup(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 251	C++
> ntdll.dll!00007ffa31335208()	Unknown
> ntdll.dll!00007ffa3137b2aa()	Unknown
> ntdll.dll!00007ffa3137b13a()	Unknown
> kernel32.dll!00007ffa2f944d8a()	Unknown
> MiniBrowser.exe!exit_or_terminate_process(const unsigned int return_code) Line 129	C++
> MiniBrowser.exe!common_exit(const int return_code, const _crt_exit_cleanup_mode cleanup_mode, const _crt_exit_return_mode return_mode) Line 269	C++
> [External Code]

[Fujii Hironori]

I see this issue with following steps today.

> svn up -r 200070
> perl Tools\Scripts\build-webkit --debug --wincairo --64-bit
> svn up -r 200119
> perl Tools\Scripts\build-webkit --debug --wincairo --64-bit

[Fujii Hironori]

A DLL ucrtbased.dll is observed in the call stack of comment 0.
According to a following document, ucrtbased.dll is just for dynamically linked CRT.

  CRT Library Features
  https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

This is strange because WebKit is compiled with /MT switch to use static CRT.
By using "dumpbin /imports WebKit.dll", it can be observed that vcruntime140d.dll and ucrtbased.dll are linked.

In Source/WebKit/PlatformWin.cmake:

> if (${WTF_PLATFORM_WIN_CAIRO})
>     set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /NODEFAULTLIB:LIBCMT /NODEFAULTLIB:LIBCMTD")
> else ()
>     set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /NODEFAULTLIB:MSVCRT /NODEFAULTLIB:MSVCRTD")
> endif ()

LIBCMT and LIBCMTD are ignored for WinCairo port.
But, they are the static CRT.
This code have been introduced in Bug 147851.

[Fujii Hironori]

Created attachment 278247 [details]
Patch

[Alex Christensen]

There is one more similar check somewhere else. I'm pretty sure they were necessary to link successfully, but maybe not.

[Brent Fulgham]

(In reply to comment #3)
> A DLL ucrtbased.dll is observed in the call stack of comment 0.
> According to a following document, ucrtbased.dll is just for dynamically
> linked CRT.
 
[...]

> LIBCMT and LIBCMTD are ignored for WinCairo port.
> But, they are the static CRT.
> This code have been introduced in Bug 147851.

This reminds me that we should switch back to a dynamic runtime. That was a workaround for something that was happening a year or so ago, and it would be good to use a single shared runtime again.

For internal reasons, we probably can't make this change until the fall, though.

[Alex Christensen]

Wincairo can change whenever though

[Fujii Hironori]

Comment on attachment 278247 [details]
Patch

I noticed one problem in my patch.
Some modules (libpng, libjpeg, ICU) in WinCairoRequirements.zip are static libraries, and compiled with /MD to use DLL CRT.
Then, WinCairo port must be compiled with /MD instread of /MT to link with the static libraries.

Do you want to change WinCairo port to compile with /MD (DLL CRT) before AppleWin port will change in this year's fall?

(In reply to comment #5)
> There is one more similar check somewhere else.

I cann't find. Could you tell me where?

[Fujii Hironori]

Created attachment 278398 [details]
WIP patch to make WinCairo port use DLL CRT

[Alex Christensen]

(In reply to comment #8)
> Comment on attachment 278247 [details]
> Patch
> 
> I noticed one problem in my patch.
> Some modules (libpng, libjpeg, ICU) in WinCairoRequirements.zip are static
> libraries, and compiled with /MD to use DLL CRT.
> Then, WinCairo port must be compiled with /MD instread of /MT to link with
> the static libraries.
> 
> Do you want to change WinCairo port to compile with /MD (DLL CRT) before
> AppleWin port will change in this year's fall?
This should be a change to https://github.com/peavo/WinCairoRequirements

[Fujii Hironori]

I created a PR:
https://github.com/peavo/WinCairoRequirements/pull/1

[Fujii Hironori]

I updated WinCairoRequirements.zip at
<http://trac.webkit.org/changeset/214328>.
Its libpng and libjpeg are DLLs.

My first patch (comment 4) has no problem now.

[Fujii Hironori]

Created attachment 305963 [details]
Patch

[Per Arne Vollan]

Comment on attachment 305963 [details]
Patch

r=me. Please check WinCairo 32-bit build as well :)

[Alex Christensen]

Comment on attachment 305963 [details]
Patch

This now matches the AppleWin port's flags and the flags in Tools/DumpRenderTree/PlatformWin.cmake and Tools/MiniBrowser/win/CMakeLists.txt

[WebKit Commit Bot]

Comment on attachment 305963 [details]
Patch

Clearing flags on attachment: 305963

Committed r214943: <http://trac.webkit.org/changeset/214943>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

Thank you for review, Per and Alex.
I did tests of 32bit, too.

I found a bug of 32bit of WinCairoRequirements and fixed it.
https://github.com/fujii/WinCairoRequirements/commit/589e8db5a73e98a1b50b616714bbe8c19eb01b6e