Bug 19580 – REGRESSION (r34432): PGO-only crash in HTMLCollection::resetCollectionInfo (codegen issue?)

• Component: WebCore Misc.
• Status: RESOLVED FIXED

source:

---
<form name=myForm>
	<textarea name=myText></textarea>
</form>

<script>window.myForm.myText.value='test';</script>
---

debugger outout:
---
Access violation - code c0000005 (!!! second chance !!!)
eax=00000000 ebx=0012f250 ecx=7fd1e3a0 edx=00000020 esi=7fd1e3a0 edi=0012f248
eip=100ad160 esp=0012f1dc ebp=0012f1fc iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000206
WebKit!WebCore__HTMLCollection__resetCollectionInfo+10:
100ad160 8b9800010000     mov     ebx,[eax+0x100]   ds:0023:00000100=????????
0:000> k
ChildEBP RetAddr
0012f1fc 100b592e WebKit!WebCore__HTMLCollection__resetCollectionInfo+0x10
0012f210 100ba061 WebKit!WebCore__HTMLCollection__namedItems+0x1e
0012f230 102cb8c0 WebKit!WebCore__HTMLFormElement__getNamedElements+0x21
0012f258 102669a8 WebKit!WebCore__JSHTMLFormElement__canGetItemsForName+0x40
0012f274 103dc15b WebKit!WebCore__JSHTMLFormElement__getOwnPropertySlot+0x18
0012f2ac 10071956 WebKit!KJS__JSValue__get+0x51
0012f49c 1009dc5f WebKit!KJS__Machine__privateExecute+0x2e86
0012f4f4 1009dad7 WebKit!KJS__Machine__execute+0xcf
0012f53c 1013dfc1 WebKit!KJS__Interpreter__evaluate+0xd7
0012f578 1012ac99 WebKit!WebCore__ScriptController__evaluate+0xb1
0012f594 10023a6e WebKit!WebCore__FrameLoader__executeScript+0x49
0012f758 100326d7 WebKit!WebCore__HTMLTokenizer__scriptExecution+0x10e
0012f7f8 1005e9d9 WebKit!WebCore__HTMLTokenizer__scriptHandler+0x257
0012f830 1005e108 WebKit!WebCore__HTMLTokenizer__parseSpecial+0x369
0012f96c 1005ee32 WebKit!WebCore__HTMLTokenizer__parseTag+0x11a8
7fef7410 00000000 WebKit!WebCore__HTMLTokenizer__write+0x2b2
---

host is safari 4DP(4.526.12.2).

I cannot reproduce the crash on Mac OS X.

[Alexey Proskuryakov]

*** Bug 19763 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

Per bug bug 19763, a crash with this stack trace happens when opening yahoo.com now.

[Alexey Proskuryakov]

<rdar://problem/6033046>

[Matt Lilek]

This doesn't seem to crash with a debug build.

[Matt Lilek]

*** Bug 19818 has been marked as a duplicate of this bug. ***

[Adam Roben (:aroben)]

Looks like m_base->document() is returning 0.

[Adam Roben (:aroben)]

(In reply to comment #7)
> Looks like m_base->document() is returning 0.

This seems to only affect nightly builds, not ToT Safari.

[Adam Roben (:aroben)]

(In reply to comment #8)
> (In reply to comment #7)
> > Looks like m_base->document() is returning 0.
> 
> This seems to only affect nightly builds, not ToT Safari.

Apparently it only affects Release builds, not Debug builds, so my statement above was not quite right (ToT Safari is affected, in Release builds).

[Adam Roben (:aroben)]

data:text/html,<form name=myForm><textarea name=myText></textarea></form><script>window.myForm.myText.value='test';</script>

[Adam Roben (:aroben)]

r34388 does not crash.

[Jon Honeycutt]

<rdar://problem/6029794>

[Adam Roben (:aroben)]

Looks like I narrowed this down incorrectly. It *does* crash in r34503.

[Adam Roben (:aroben)]

It looks like at some point a JSHTMLFormElement is getting passed instead of an HTMLFormElement.

[Adam Roben (:aroben)]

(In reply to comment #14)
> It looks like at some point a JSHTMLFormElement is getting passed instead of an
> HTMLFormElement.

Specifically, HTMLFormElement::elements calls HTMLFormCollection::create and passes in "this", but for some reason "this" is being turned into the JSHTMLFormElement that wraps this HTMLFormElement.

[Adam Roben (:aroben)]

Here's the disassembly for HTMLFormElement::elements:

PassRefPtr<HTMLCollection> HTMLFormElement::elements()
{
00E10710  push        ebp  
00E10711  mov         ebp,esp 
00E10713  push        ecx  
    return HTMLFormCollection::create(this);
00E10714  test        ecx,ecx 
00E10716  push        ecx  
00E10717  mov         dword ptr [esp],ecx 
00E1071A  je          WebCore::HTMLFormElement::elements+0Fh (0E1071Fh) 
00E1071C  inc         dword ptr [ecx+4] 
00E1071F  lea         eax,[ebp-4] 
00E10722  push        esi  
00E10723  push        eax  
00E10724  call        WebCore::HTMLFormCollection::create (0E09F70h) 
00E10729  mov         esi,dword ptr [ebp+8] 
00E1072C  pop         ecx  
00E1072D  pop         ecx  
00E1072E  mov         ecx,dword ptr [eax] 
00E10730  and         dword ptr [eax],0 
00E10733  mov         eax,dword ptr [ebp-4] 
00E10736  test        eax,eax 
00E10738  mov         dword ptr [esi],ecx 
00E1073A  jne         010A4FF6 
00E10740  mov         eax,esi 
00E10742  pop         esi  
}
00E10743  leave            
00E10744  ret         4    

[Adam Roben (:aroben)]

For comparison, here's the disassembly from a Debug build:

PassRefPtr<HTMLCollection> HTMLFormElement::elements()
{
00E318C0  push        ebp  
00E318C1  mov         ebp,esp 
00E318C3  sub         esp,0Ch 
00E318C6  mov         dword ptr [ebp-0Ch],0CCCCCCCCh 
00E318CD  mov         dword ptr [ebp-8],0CCCCCCCCh 
00E318D4  mov         dword ptr [ebp-4],0CCCCCCCCh 
00E318DB  mov         dword ptr [ebp-4],ecx 
    return HTMLFormCollection::create(this);
00E318DE  push        ecx  
00E318DF  mov         ecx,esp 
00E318E1  mov         eax,dword ptr [this] 
00E318E4  push        eax  
00E318E5  call        WTF::PassRefPtr<WebCore::HTMLFormElement>::PassRefPtr<WebCore::HTMLFormElement> (0AE5A06h) 
00E318EA  lea         ecx,[ebp-0Ch] 
00E318ED  push        ecx  
00E318EE  call        WebCore::HTMLFormCollection::create (114B9C0h) 
00E318F3  add         esp,8 
00E318F6  push        eax  
00E318F7  mov         ecx,dword ptr [ebp+8] 
00E318FA  call        WTF::PassRefPtr<WebCore::HTMLCollection>::PassRefPtr<WebCore::HTMLCollection><WebCore::HTMLFormCollection> (0E33E50h) 
00E318FF  lea         ecx,[ebp-0Ch] 
00E31902  call        WTF::PassRefPtr<WebCore::HTMLFormCollection>::~PassRefPtr<WebCore::HTMLFormCollection> (0E32CB0h) 
00E31907  mov         eax,dword ptr [ebp+8] 
}
00E3190A  add         esp,0Ch 
00E3190D  cmp         ebp,esp 
00E3190F  call        WTF::HashTableConstIteratorAdapter<WTF::HashTable<void *,std::pair<void *,PrintJobManager *>,WTF::PairFirstExtractor<std::pair<void *,PrintJobManager *> >,WTF::PtrHash<void *>,WTF::PairHashTraits<WTF::HashTraits<void *>,WTF::HashTraits<PrintJobManager *> >,WTF::HashTraits<void *> >,std::pair<void *,PrintJobManager *> >::operator->+0Ch (6F3FFCh) 
00E31914  mov         esp,ebp 
00E31916  pop         ebp  
00E31917  ret         4    

[Adam Roben (:aroben)]

It looks like in a released version of WebKit.dll HTMLFormElement::elements was inlined within HTMLFormElement::getNamedElements.

[Adam Roben (:aroben)]

(In reply to comment #18)
> It looks like in a released version of WebKit.dll HTMLFormElement::elements was
> inlined within HTMLFormElement::getNamedElements.

Of course that was back before r34432, when HTMLFormCollection::create was added.

[Adam Roben (:aroben)]

(In reply to comment #16)
> Here's the disassembly for HTMLFormElement::elements:
> 
> PassRefPtr<HTMLCollection> HTMLFormElement::elements()
> {
> 00E10710  push        ebp  
> 00E10711  mov         ebp,esp 
> 00E10713  push        ecx  
>     return HTMLFormCollection::create(this);
> 00E10714  test        ecx,ecx 
> 00E10716  push        ecx  
> 00E10717  mov         dword ptr [esp],ecx 
> 00E1071A  je          WebCore::HTMLFormElement::elements+0Fh (0E1071Fh) 
> 00E1071C  inc         dword ptr [ecx+4] 
> 00E1071F  lea         eax,[ebp-4] 
> 00E10722  push        esi  
> 00E10723  push        eax  
> 00E10724  call        WebCore::HTMLFormCollection::create (0E09F70h) 

It looks like the value in esi ends up being the value of the argument passed to HTMLFormCollection::create. If this were working correctly, ecx would be the value passed to HTMLFormCollection::create.

(In reply to comment #20)
I wonder if that isn't push esp....

[Adam Roben (:aroben)]

I reverted the parts of r34432 that are relevant to HTMLFormCollection, and the bug no longer occurs. Here's the disassembly from a PGO build with part of r34432 reverted:

PassRefPtr<HTMLCollection> HTMLFormElement::elements()
{
00E09F20  push        ebp  
00E09F21  mov         ebp,esp 
00E09F23  sub         esp,14h 
00E09F26  push        ebx  
00E09F27  push        esi  
00E09F28  push        edi  
00E09F29  mov         dword ptr [ebp-14h],ecx 
    return new HTMLFormCollection(this);
00E09F2C  call        WTF::TCMalloc_ThreadCache::GetCache (0D80790h) 
00E09F31  push        20h  
00E09F33  mov         edi,eax 
00E09F35  call        WTF::ClassIndex (0DB72D0h) 
00E09F3A  movzx       esi,byte ptr WebCore::CSSStyleSelector::s_styleNotYetAvailable+54h (1213628h)[eax] 
00E09F41  mov         eax,dword ptr WebCore::CSSStyleSelector::s_styleNotYetAvailable+1D4h (12137A8h)[esi*4] 
00E09F48  pop         ecx  
00E09F49  lea         ebx,[edi+esi*8+0Ch] 
00E09F4D  mov         ecx,ebx 
00E09F4F  mov         dword ptr [ebp-8],eax 
00E09F52  call        WTF::RefPtr<KJS::SourceElements>::operator! (0E18310h) 
00E09F57  test        al,al 
00E09F59  jne         010A57DC 
00E09F5F  mov         eax,dword ptr [ebp-8] 
00E09F62  sub         dword ptr [edi],eax 
00E09F64  dec         word ptr [ebx+4] 
00E09F68  movzx       eax,word ptr [ebx+4] 
00E09F6C  cmp         ax,word ptr [ebx+6] 
00E09F70  jb          010A5840 
00E09F76  mov         edi,dword ptr [ebx] 
00E09F78  test        edi,edi 
00E09F7A  mov         eax,dword ptr [edi] 
00E09F7C  mov         dword ptr [ebx],eax 
00E09F7E  je          010A582D 
00E09F84  mov         eax,dword ptr [ebp-14h] 
00E09F87  test        eax,eax 
00E09F89  push        ecx  
00E09F8A  mov         dword ptr [esp],eax 
00E09F8D  je          WebCore::HTMLFormElement::elements+72h (0E09F92h) 
00E09F8F  inc         dword ptr [eax+4] 
00E09F92  call        WebCore::HTMLFormCollection::HTMLFormCollection (0E0E570h) 
00E09F97  test        eax,eax 
00E09F99  mov         ecx,dword ptr [ebp+8] 
00E09F9C  pop         edi  
00E09F9D  pop         esi  
00E09F9E  mov         dword ptr [ecx],eax 
00E09FA0  pop         ebx  
00E09FA1  je          WebCore::HTMLFormElement::elements+86h (0E09FA6h) 
00E09FA3  inc         dword ptr [eax+4] 
00E09FA6  mov         eax,ecx 
}
00E09FA8  leave            
00E09FA9  ret         4    

[Adam Roben (:aroben)]

Disabling LTCG for just HTMLFormElement.cpp does not seem to fix the issue.

(In reply to comment #23)
*At least*, HTMLFormCollection::create() has same problem.
With r34813,

mov     eax, [ebp+0Ch]
and     dword ptr [ebp+0Ch], 0
push    ecx ; PassRefPtr<>(form)
mov     edi, ecx ; edi is fastMalloced space
mov     [esp], eax
call    ??0HTMLFormCollection@WebCore... ; ctor

Oh, HTMLFormCollection::this and form cannot be same.

I wonder if 1st push ecx isn't push esp, again.

// btw, almost all fastMalloc() is inlined (by LTCG) ... is it desired??
// It's pretty large, for each, and may affect mem cache, I feel.
// I haven't check which is finally faster, inlined or not inlined.

(In reply to comment #24. errata)
push    ecx ; *should be* PassRefPtr<>(form)
mov     edi, ecx ; ecx is fastMalloced space, edi should be _this for HTMLFormCollection

[Mark Rowe (bdash)]

*** Bug 19934 has been marked as a duplicate of this bug. ***

[Adam Roben (:aroben)]

*** Bug 19752 has been marked as a duplicate of this bug. ***

[Adam Roben (:aroben)]

Turning off LTCG for all of WebCore fixes the crash. I now suspect that I wasn't building correctly when turning off LTCG for certain files previously.

[Adam Roben (:aroben)]

(In reply to comment #23)
> Disabling LTCG for just HTMLFormElement.cpp does not seem to fix the issue.

Yes it does! You just have to actually get HTMLFormElement.cpp to rebuild!

[Adam Roben (:aroben)]

Created attachment 22224 [details]
patch + changelog

[Cameron Zwarich (cpst)]

Comment on attachment 22224 [details]
patch + changelog

r=me

[Adam Roben (:aroben)]

Landed in r35105