Created attachment 243680 [details] Crash log for 'dom/html/level2/html/HTMLIFrameElement03.html' A number of tests fail (unreliably) due to some kind of heap corruption that manifests as a crash in WebActionPropertyBag: STACK_COMMAND: .ecxr ; kb ; dps 774842a8 ; kb FOLLOWUP_IP: WebKit!WebActionPropertyBag::Read+28 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\webactionpropertybag.cpp @ 115] 021da358 b9e4a1c802 mov ecx,offset WebKit!piOverFourDouble+0xd4 (02c8a1e4) FAULT_INSTR_CODE: c8a1e4b9 FAULTING_SOURCE_LINE: c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\webactionpropertybag.cpp FAULTING_SOURCE_FILE: c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\webactionpropertybag.cpp FAULTING_SOURCE_LINE_NUMBER: 115 FAULTING_SOURCE_CODE: 111: return E_POINTER; 112: 113: VariantClear(pVar); 114: > 115: if (isEqual(pszPropName, WebActionNavigationTypeKey)) { 116: V_VT(pVar) = VT_I4; 117: V_I4(pVar) = m_action.type(); 118: return S_OK; 119: } 120: if (isEqual(pszPropName, WebActionElementKey)) { See the attached debug dump.
<rdar://problem/19337296>
The stack trace for these errors typically looks like the following: STACK_TEXT: 77c742a8 77bf9cb1 ntdll!RtlSizeHeap+0x69 77c742ac 75c6625c ole32!CRetailMalloc_GetSize+0x21 77c742b0 773b443a oleaut32!APP_DATA::FreeCachedMem+0x30 77c742b4 773b3ea3 oleaut32!SysFreeString+0x6b 77c742b8 773b4870 oleaut32!VariantClear+0xc3 77c742bc 022ef828 webkit!WebActionPropertyBag::Read+0x28 77c742c0 022e4642 webkit!DefaultPolicyDelegate::decidePolicyForNavigationAction+0x22 77c742c4 022fa206 webkit!WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction+0xf6 77c742c8 029ef146 webkit!WebCore::PolicyChecker::checkNavigationPolicy+0x226 77c742cc 023ad33b webkit!WebCore::FrameLoader::loadWithDocumentLoader+0x30b 77c742d0 023ad483 webkit!WebCore::FrameLoader::load+0xd3 77c742d4 023a6c2c webkit!WebCore::FrameLoader::load+0x19c 77c742d8 02306805 webkit!WebFrame::loadRequest+0x95 77c742dc 10015c12 dumprendertree!runTest+0x672 77c742e0 100163e0 dumprendertree!main+0x2d0 77c742e4 1001652e dumprendertree!dllLauncherEntryPoint+0xe 77c742e8 00401c35 dumprendertree!main+0x4b5 77c742ec 004042f9 dumprendertree!__tmainCRTStartup+0xfe 77c742f0 76fe338a kernel32!BaseThreadInitThunk+0xe 77c742f4 77ba9f72 ntdll!__RtlUserThreadStart+0x70 77c742f8 77ba9f45 ntdll!_RtlUserThreadStart+0x1b
MSDN clearly states: "Do not use VariantClear on unitialized variants; use VariantInit to initialize a new VARIANTARG or VARIANT" In the call stack for these crashes, I typically see an uninitialized variant value. We should either (a) switch from raw VARIANT values to _variant_t classes (much like _bstr_t), or (b) ensure that ::VariantInit is called each time we create a new VARIANT variable.
Calling ::VariantClear on uninitialized memory is bad, because it attempts to interpret the meaning of the current contents of the VARIANT passed to it. If this is uninitialized memory, it can make bad assumptions about the content of the VARIANT.
Created attachment 244912 [details] Patch
Attachment 244912 [details] did not pass style-queue: ERROR: Source/WebKit/win/DefaultPolicyDelegate.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 244912 [details] Patch Could we use COMVariant here?
(In reply to comment #7) > Comment on attachment 244912 [details] > Patch > > Could we use COMVariant here? Yes, except the changes I need to make are mostly in DumpRenderTree. COMVariant is internal to WebKit, and is not available to DRT (unless we change the set of files we ship with WebKit).
Committed r178669: <http://trac.webkit.org/changeset/178669>