Bug 188893 – REGRESSION(r234879): Hundreds of layout tests are timing out in WinCairo Debug

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Ross Kirsling]

Hundreds of layout tests are timing out in WinCairo Debug since Bug 188582.

Not sure of the root cause yet, but reverting HashTable.h:840 does make the timeouts go away:
https://github.com/WebKit/webkit/commit/c17c5e21de940574a5ed795c9a9b7431680fdb14#diff-09bcabb3919151180e9d2bfc7f97dc56

[Ross Kirsling]

Still digging, but two more points for now...

Commenting out this function also makes the problem go away.
https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/HashTraits.h#L317-L322
> template <typename>
> static void constructEmptyValue(TraitType& slot)
> {
>    KeyTraits::template constructEmptyValue<KeyTraits>(slot.key);
>    ValueTraits::template constructEmptyValue<ValueTraits>(slot.value);
> }

The failing tests don't seem to be random, but calling DRT directly doesn't seem to repro(?).

[Fujii Hironori]

I can reproduce with a following command.

> python ./Tools/Scripts/run-webkit-tests --debug --wincairo --dump-render-tree --no-new-test-results --no-retry-failures --no-timeout  css1/basic/class_as_selector.html

It never exit this loop.

https://github.com/WebKit/webkit/blob/c17c5e21de940574a5ed795c9a9b7431680fdb14/Source/WTF/wtf/HashTable.h#L892

Callstack:

> WebKit.dll!WTF::HashTable<WebCore::Color,WTF::KeyValuePair<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > >,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > > >,WTF::ColorHash,WTF::HashMap<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> >,WTF::ColorHash,WTF::HashTraits<WebCore::Color>,WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > > >::KeyValuePairTraits,WTF::HashTraits<WebCore::Color> >::add<WTF::HashMapEnsureTranslator<WTF::HashMap<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> >,WTF::ColorHash,WTF::HashTraits<WebCore::Color>,WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > > >::KeyValuePairTraits,WTF::ColorHash>,WebCore::Color const &,WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) >(const WebCore::Color & key, WebCore::CSSValuePool::createColorValue::__l2::WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) && extra) Line 892	C++
> WebKit.dll!WTF::HashMap<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> >,WTF::ColorHash,WTF::HashTraits<WebCore::Color>,WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > > >::inlineEnsure<WebCore::Color const &,WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) >(const WebCore::Color & key, WebCore::CSSValuePool::createColorValue::__l2::WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) && functor) Line 353	C++
> WebKit.dll!WTF::HashMap<WebCore::Color,WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> >,WTF::ColorHash,WTF::HashTraits<WebCore::Color>,WTF::HashTraits<WTF::RefPtr<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > > >::ensure<WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) >(const WebCore::Color & key, WebCore::CSSValuePool::createColorValue::__l2::WTF::Ref<WebCore::CSSPrimitiveValue,WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > <lambda>(void) && functor) Line 409	C++
> WebKit.dll!WebCore::CSSValuePool::createColorValue(const WebCore::Color & color) Line 92	C++
> WebKit.dll!WebCore::HTMLElement::addHTMLColorToStyle(WebCore::MutableStyleProperties & style, WebCore::CSSPropertyID propertyID, const WTF::String & attributeValue) Line 1024	C++
> WebKit.dll!WebCore::HTMLTablePartElement::collectStyleForPresentationAttribute(const WebCore::QualifiedName & name, const WTF::AtomicString & value, WebCore::MutableStyleProperties & style) Line 54	C++
> WebKit.dll!WebCore::HTMLTableCellElement::collectStyleForPresentationAttribute(const WebCore::QualifiedName & name, const WTF::AtomicString & value, WebCore::MutableStyleProperties & style) Line 118	C++
> WebKit.dll!WebCore::StyledElement::rebuildPresentationAttributeStyle() Line 239	C++
> WebKit.dll!WebCore::StyledElement::presentationAttributeStyle() Line 97	C++
> WebKit.dll!WebCore::ElementRuleCollector::matchAllRules(bool matchAuthorAndUserStyles, bool includeSMILProperties) Line 510	C++
> WebKit.dll!WebCore::StyleResolver::styleForElement(const WebCore::Element & element, const WebCore::RenderStyle * parentStyle, const WebCore::RenderStyle * parentBoxStyle, WebCore::RuleMatchingBehavior matchingBehavior, const WebCore::SelectorFilter * selectorFilter) Line 386	C++
> WebKit.dll!WebCore::Style::TreeResolver::styleForElement(WebCore::Element & element, const WebCore::RenderStyle & inheritedStyle) Line 133	C++
> WebKit.dll!WebCore::Style::TreeResolver::resolveElement(WebCore::Element & element) Line 203	C++
> WebKit.dll!WebCore::Style::TreeResolver::resolveComposedTree() Line 500	C++
> WebKit.dll!WebCore::Style::TreeResolver::resolve() Line 558	C++
> WebKit.dll!WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType type) Line 1856	C++
> WebKit.dll!WebCore::Document::updateStyleIfNeeded() Line 1973	C++
> WebKit.dll!WebCore::Document::implicitClose() Line 2884	C++
> WebKit.dll!WebCore::FrameLoader::checkCallImplicitClose() Line 957	C++
> WebKit.dll!WebCore::FrameLoader::checkCompleted() Line 899	C++
> WebKit.dll!WebCore::FrameLoader::loadDone(WebCore::LoadCompletionType type) Line 800	C++
> WebKit.dll!WebCore::CachedResourceLoader::loadDone(WebCore::LoadCompletionType type, bool shouldPerformPostLoadActions) Line 1317	C++
> WebKit.dll!WebCore::SubresourceLoader::notifyDone(WebCore::LoadCompletionType type) Line 743	C++
> WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(const WebCore::NetworkLoadMetrics & networkLoadMetrics) Line 644	C++
> WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal) Line 683	C++
> WebKit.dll!WebCore::CurlResourceHandleDelegate::curlDidComplete(WebCore::CurlRequest & request) Line 171	C++
> WebKit.dll!WebCore::CurlRequest::didCompleteTransfer::__l11::<lambda>(WebCore::CurlRequest & request, WebCore::CurlRequestClient & client) Line 411	C++
> WebKit.dll!WTF::Function<void __cdecl(WebCore::CurlRequest &,WebCore::CurlRequestClient &)>::CallableWrapper<void <lambda>(WebCore::CurlRequest &, WebCore::CurlRequestClient &) >::call(WebCore::CurlRequest & <in_0>, WebCore::CurlRequestClient & <in_1>) Line 101	C++
> WebKit.dll!WTF::Function<void __cdecl(WebCore::CurlRequest &,WebCore::CurlRequestClient &)>::operator()(WebCore::CurlRequest & <in_0>, WebCore::CurlRequestClient & <in_1>) Line 57	C++
> WebKit.dll!WebCore::CurlRequest::callClient::__l2::<lambda>() Line 136	C++
> WebKit.dll!WTF::Function<void __cdecl(void)>::CallableWrapper<void <lambda>(void) >::call() Line 101	C++
> WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 57	C++
> WTF.dll!WTF::dispatchFunctionsFromMainThread() Line 132	C++
> WTF.dll!WTF::ThreadingWindowWndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 47	C++
> [External Code]	
> DumpRenderTreeLib.dll!runTest(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & inputLine) Line 1246	C++
> DumpRenderTreeLib.dll!main(int argc, const char * * argv) Line 1619	C++
> DumpRenderTreeLib.dll!dllLauncherEntryPoint(int argc, const char * * argv) Line 1660	C++
> DumpRenderTree.exe!main(int argc, const char * * argv) Line 228	C++
> [External Code]

[Saam Barati]

This is super weird. It'd be interesting to see what things got compiled too. Not sure what specifically would make windows itself barf here. Maybe MSVC has a bad time with this code?

[Fujii Hironori]

The HashTable was filled with Color::invalidRGBAColor (0x0000000000000001), not Color::emptyHashValue(0xFFFFFFFFFFFFFFFB).
Then, HashTable::add can't find an empty slot in the loop.

It seems that HashTable::allocateTable initializes slots with
WTF::GenericHashTraits<WebCore::Color>::emptyValue() not
HashTraits<WebCore::Color>::emptyValue().

[Fujii Hironori]

Created attachment 347998 [details]
WIP patch

[Saam Barati]

(In reply to Fujii Hironori from comment #4)
> The HashTable was filled with Color::invalidRGBAColor (0x0000000000000001),
> not Color::emptyHashValue(0xFFFFFFFFFFFFFFFB).
> Then, HashTable::add can't find an empty slot in the loop.
> 
> It seems that HashTable::allocateTable initializes slots with
> WTF::GenericHashTraits<WebCore::Color>::emptyValue() not
> HashTraits<WebCore::Color>::emptyValue().

Interesting. This sounds like a more general issue. Why don’t we see the implemented HashTraits? Do you know why we end up picking up GenericHashTraits instead of the custom one?

[Fujii Hironori]

I don't understand what is going on in MSVC yet.

You added a template method constructEmptyValue in Bug 188582.
constructEmptyValue is a template method in order to override emptyValue().
We need to use constructEmptyValue with Traits because it overrides emptyValue.
IMHO, this code is weird. We should find a better solution for Bug 188582.

[Fujii Hironori]

I can't reproduce this issue with msbuild. It happens only by using Ninja. So weird.

[Fujii Hironori]

I compared disassembled code of GenericHashTraits<Color>::constructEmptyValue of built by msbuild and Ninja.

>	WebKit.dll!WTF::GenericHashTraits<WebCore::Color>::constructEmptyValue<WTF::HashTraits<WebCore::Color> >(WebCore::Color & slot) Line 76	C++

MSBuild:

> --- c:\webkit\ga\webkitbuild\debug\derivedsources\forwardingheaders\wtf\hashtraits.h 
> 
>     template <typename Traits>
>     static void constructEmptyValue(T& slot)
>     {
> 00007FFB1F07DE80  mov         qword ptr [rsp+8],rcx  
> 00007FFB1F07DE85  push        rdi  
> 00007FFB1F07DE86  sub         rsp,30h  
> 00007FFB1F07DE8A  mov         rdi,rsp  
> 00007FFB1F07DE8D  mov         ecx,0Ch  
> 00007FFB1F07DE92  mov         eax,0CCCCCCCCh  
> 00007FFB1F07DE97  rep stos    dword ptr [rdi]  
> 00007FFB1F07DE99  mov         rcx,qword ptr [slot]  
>         new (NotNull, std::addressof(slot)) T(Traits::emptyValue());
> 00007FFB1F07DE9E  mov         rcx,qword ptr [slot]  
> 00007FFB1F07DEA3  call        std::addressof<WebCore::Color> (07FFB1F07C650h)  
> 00007FFB1F07DEA8  mov         r8,rax  
> 00007FFB1F07DEAB  xor         edx,edx  
> 00007FFB1F07DEAD  mov         ecx,8  
> 00007FFB1F07DEB2  call        WebCore::Color::operator new (07FFB1F08EF40h)  
> 00007FFB1F07DEB7  mov         qword ptr [rsp+20h],rax  
> 00007FFB1F07DEBC  cmp         qword ptr [rsp+20h],0  
> 00007FFB1F07DEC2  je          WTF::GenericHashTraits<WebCore::Color>::constructEmptyValue<WTF::HashTraits<WebCore::Color> >+55h (07FFB1F07DED5h)  
> 00007FFB1F07DEC4  mov         rcx,qword ptr [rsp+20h]  
> 00007FFB1F07DEC9  call        WTF::GenericHashTraits<WebCore::Color>::emptyValue (07FFB1F09BDE0h)  
> 00007FFB1F07DECE  mov         qword ptr [rsp+28h],rax  
> 00007FFB1F07DED3  jmp         WTF::GenericHashTraits<WebCore::Color>::constructEmptyValue<WTF::HashTraits<WebCore::Color> >+5Eh (07FFB1F07DEDEh)  
> 00007FFB1F07DED5  mov         qword ptr [rsp+28h],0  
>     }
> 00007FFB1F07DEDE  add         rsp,30h  
> 00007FFB1F07DEE2  pop         rdi  
> 00007FFB1F07DEE3  ret  


Ninja

> --- c:\webkit\gb\webkitbuild\debug\derivedsources\forwardingheaders\wtf\hashtraits.h 
> 
>     template <typename Traits>
>     static void constructEmptyValue(T& slot)
>     {
> 00007FFB1F338830  mov         qword ptr [rsp+8],rcx  
> 00007FFB1F338835  push        rdi  
> 00007FFB1F338836  sub         rsp,30h  
> 00007FFB1F33883A  mov         rdi,rsp  
> 00007FFB1F33883D  mov         ecx,0Ch  
> 00007FFB1F338842  mov         eax,0CCCCCCCCh  
> 00007FFB1F338847  rep stos    dword ptr [rdi]  
> 00007FFB1F338849  mov         rcx,qword ptr [slot]  
>         new (NotNull, std::addressof(slot)) T(Traits::emptyValue());
> 00007FFB1F33884E  mov         rcx,qword ptr [slot]  
> 00007FFB1F338853  call        std::addressof<WebCore::Color> (07FFB1F3360C0h)  
> 00007FFB1F338858  mov         r8,rax  
> 00007FFB1F33885B  xor         edx,edx  
> 00007FFB1F33885D  mov         ecx,8  
> 00007FFB1F338862  call        WebCore::Color::operator new (07FFB1F3410F0h)  
> 00007FFB1F338867  mov         qword ptr [rsp+20h],rax  
> 00007FFB1F33886C  cmp         qword ptr [rsp+20h],0  
> 00007FFB1F338872  je          WTF::GenericHashTraits<WebCore::Color>::constructEmptyValue<WTF::HashTraits<WebCore::Color> >+55h (07FFB1F338885h)  
> 00007FFB1F338874  mov         rcx,qword ptr [rsp+20h]  
> 00007FFB1F338879  call        WTF::HashTraits<WebCore::Color>::emptyValue (07FFB1F34A7D0h)  
> 00007FFB1F33887E  mov         qword ptr [rsp+28h],rax  
> 00007FFB1F338883  jmp         WTF::GenericHashTraits<WebCore::Color>::constructEmptyValue<WTF::HashTraits<WebCore::Color> >+5Eh (07FFB1F33888Eh)  
> 00007FFB1F338885  mov         qword ptr [rsp+28h],0  
>     }
> 00007FFB1F33888E  add         rsp,30h  
> 00007FFB1F338892  pop         rdi  
> 00007FFB1F338893  ret  

WTF::GenericHashTraits<WebCore::Color>::emptyValue is called in one built by MSBuild.
But, WTF::HashTraits<WebCore::Color>::emptyValue is called in one built by Ninja.

[Fujii Hironori]

Created attachment 348131 [details]
Patch

[Saam Barati]

I wonder if there is anything we can do to make diagnosing such issues easier or even produce build errors somehow? I don’t see how we could make it produce a build error but maybe someone else does

[Fujii Hironori]

Comment on attachment 348131 [details]
Patch

Good point!

Indeed, JavaScriptCore is doing that. JSC has a lot of *Inlines.h
not to include unnecessary inline functions.

For example, the template declaration of Heap::forEachProtectedCell is in Heap.h.
https://github.com/WebKit/webkit/blob/5b3fecdfc4dfae0b39a57cece903ff0c96f0c423/Source/JavaScriptCore/heap/Heap.h#L237

And, its template definition is in HeapInlines.h.
https://github.com/WebKit/webkit/blob/5b3fecdfc4dfae0b39a57cece903ff0c96f0c423/Source/JavaScriptCore/heap/HeapInlines.h#L155

I will add a template declaration in Color.h.

> namespace WTF {
> template<> struct HashTraits<WebCore::Color>;
> }

Then, I get a following compilation error.

> 1>------ Build started: Project: WebCore (WebCore\WebCore), Configuration: Debug x64 ------
> 1>UnifiedSource304.cpp
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(221): error C2672: 'WTF::HashMap<WTF::String,WebCore::Color,WTF::DefaultHash<WTF::String>::Hash,WTF::HashTraits<WTF::String>,WTF::HashTraits<WebCore::Color>>::get': no matching overloaded function found
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(221): error C2893: Failed to specialize function template 'std::enable_if::type WTF::HashMap<WTF::String,WebCore::Color,WTF::DefaultHash<WTF::String>::Hash,WTF::HashTraits<WTF::String>,WTF::HashTraits<WebCore::Color>>::get(GetPtrHelper<K>::PtrType) const'
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(221): note: With the following template arguments:
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(221): note: 'K=WTF::String'
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(227): error C2672: 'WTF::HashMap<WTF::String,WebCore::Color,WTF::DefaultHash<WTF::String>::Hash,WTF::HashTraits<WTF::String>,WTF::HashTraits<WebCore::Color>>::get': no matching overloaded function found
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(227): error C2893: Failed to specialize function template 'std::enable_if::type WTF::HashMap<WTF::String,WebCore::Color,WTF::DefaultHash<WTF::String>::Hash,WTF::HashTraits<WTF::String>,WTF::HashTraits<WebCore::Color>>::get(GetPtrHelper<K>::PtrType) const'
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(227): note: With the following template arguments:
> 1>c:\webkit\gb\source\webcore\page\debugpageoverlays.cpp(227): note: 'K=WTF::String'
> 1>Done building project "WebCore.vcxproj" -- FAILED.
> ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

I discard the previous patch and will upload a new patch.

[Fujii Hironori]

Filed another ticket.

  Bug 189044 – Add specialize template declarations of HashTraits and DefaultHash to detect misuse

[Fujii Hironori]

Fixed this issue in Bug 189044.