Bug 132146

Summary: Crash in platform/mac/accessibility/table-visible-rows.html
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: Tools / TestsAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, mark.lam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
mark.lam: review+
Updated patch with speculative Windows fix fpizlo: review+

Description Mark Lam 2014-04-24 15:03:30 PDT
Build a debug build of ToT (any revision after r167772), and run the following:
$ JSC_slowPathAllocsBetweenGCs=1 DumpRenderTree LayoutTests/platform/mac/accessibility/table-visible-rows.html

You will see the following crash:

(lldb) r LayoutTests/platform/mac/accessibility/table-visible-rows.html
Process 60125 launched: '/Volumes/Data/ws4/OpenSource/WebKitBuild/Debug/DumpRenderTree' (x86_64)
Process 60125 stopped
* thread #1: tid = 0xe97def, 0x000000010033c81c JavaScriptCore`JSC::JSCell::structure(this=0x0000000000000000, vm=0x000000010b842c00) const + 44 at JSCellInlines.h:104, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010033c81c JavaScriptCore`JSC::JSCell::structure(this=0x0000000000000000, vm=0x000000010b842c00) const + 44 at JSCellInlines.h:104
   101 	
   102 	inline Structure* JSCell::structure(VM& vm) const
   103 	{
-> 104 	    return vm.heap.structureIDTable().get(m_structureID);
   105 	}
   106 	
   107 	inline void JSCell::visitChildren(JSCell* cell, SlotVisitor& visitor)

(lldb) bt
* thread #1: tid = 0xe97def, 0x000000010033c81c JavaScriptCore`JSC::JSCell::structure(this=0x0000000000000000, vm=0x000000010b842c00) const + 44 at JSCellInlines.h:104, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010033c81c JavaScriptCore`JSC::JSCell::structure(this=0x0000000000000000, vm=0x000000010b842c00) const + 44 at JSCellInlines.h:104
    frame #1: 0x0000000100359f76 JavaScriptCore`JSC::JSCell::methodTable(this=0x000000011951f8b0) const + 70 at JSCellInlines.h:192
    frame #2: 0x000000010073bdde JavaScriptCore`toJS(exec=0x00007fff5fbfb920, v=0x000000011951f8b0) + 158 at APICast.h:81
    frame #3: 0x00000001007cfc2d JavaScriptCore`JSObjectMakeArray(ctx=0x00007fff5fbfb920, argumentCount=3, arguments=0x0000000114814b60, exception=0x00007fff5fbfb4d0) + 189 at JSObjectRef.cpp:177
    frame #4: 0x000000010000a1a6 DumpRenderTree`convertElementsToObjectArray(context=0x00007fff5fbfb920, elements=0x00007fff5fbfb3e0, exception=0x00007fff5fbfb4d0) + 566 at AccessibilityUIElement.cpp:526
    frame #5: 0x0000000100007f6e DumpRenderTree`uiElementArrayAttributeValueCallback(context=0x00007fff5fbfb920, function=0x000000011951f930, thisObject=0x000000011951f990, argumentCount=1, arguments=0x00007fff5fbfb500, exception=0x00007fff5fbfb4d0) + 190 at AccessibilityUIElement.cpp:552
    frame #6: 0x000000010075acb7 JavaScriptCore`long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(exec=0x00007fff5fbfb920) + 455 at APICallbackFunction.h:61
    frame #7: 0x000000010086dd62 JavaScriptCore`JSC::LLInt::handleHostCall(execCallee=0x00007fff5fbfb920, pc=0x000000010f8064f8, callee=JSValue at 0x00007fff5fbfb718, kind=CodeForCall) + 306 at LLIntSlowPaths.cpp:1035
    frame #8: 0x000000010086ebdc JavaScriptCore`JSC::LLInt::setUpCall(execCallee=0x00007fff5fbfb920, pc=0x000000010f8064f8, kind=CodeForCall, calleeAsValue=JSValue at 0x00007fff5fbfb840, callLinkInfo=0x0000000110612fa0) + 92 at LLIntSlowPaths.cpp:1079
    frame #9: 0x000000010086eb6c JavaScriptCore`JSC::LLInt::genericCall(exec=0x00007fff5fbfb980, pc=0x000000010f8064f8, kind=CodeForCall) + 236 at LLIntSlowPaths.cpp:1145
    frame #10: 0x000000010086b26c JavaScriptCore`llint_slow_path_call(exec=0x00007fff5fbfb980, pc=0x000000010f8064f8) + 60 at LLIntSlowPaths.cpp:1151
    frame #11: 0x00000001008784d9 JavaScriptCore`llint_entry + 26147
    frame #12: 0x0000000100871c44 JavaScriptCore`callToJavaScript + 356
    frame #13: 0x00007fff70a2e850 Foundation`__block_descriptor_tmp234 + 32

(lldb) up
frame #1: 0x0000000100359f76 JavaScriptCore`JSC::JSCell::methodTable(this=0x000000011951f8b0) const + 70 at JSCellInlines.h:192
   189 	{
   190 	    VM& vm = *Heap::heap(this)->vm();
   191 	    Structure* structure = this->structure(vm);
-> 192 	    if (Structure* rootStructure = structure->structure(vm))
   193 	        RELEASE_ASSERT(rootStructure == rootStructure->structure(vm));
   194 	
   195 	    return &structure->classInfo()->methodTable;
Comment 1 Radar WebKit Bug Importer 2014-04-24 15:05:06 PDT
<rdar://problem/16718045>
Comment 2 Michael Saboff 2014-04-25 13:00:11 PDT
Created attachment 230192 [details]
Patch
Comment 3 Mark Lam 2014-04-25 13:16:20 PDT
Comment on attachment 230192 [details]
Patch

r=me.  But can you do a quick grep for JSObjectMakeArray to see if anyone in our system is doing the wrong thing, but happens to be uncaught by tests?
Comment 4 Michael Saboff 2014-04-25 13:17:16 PDT
(In reply to comment #3)
> (From update of attachment 230192 [details])
> r=me.  But can you do a quick grep for JSObjectMakeArray to see if anyone in our system is doing the wrong thing, but happens to be uncaught by tests?

I already did and the patch contains the other instances.
Comment 5 Michael Saboff 2014-04-25 13:35:20 PDT
Committed r167819: <http://trac.webkit.org/changeset/167819>
Comment 6 Csaba Osztrogonác 2014-04-27 03:26:51 PDT
Comment on attachment 230192 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230192&action=review

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:522
> -    auto valueElements = std::make_unique<JSValueRef[]>(elementCount);
> +    JSValueRef valueElements[elementCount];

It broke the Apple Windows build:
   1>..\..\AccessibilityUIElement.cpp(522): error C2057: expected constant expression
     1>..\..\AccessibilityUIElement.cpp(522): error C2466: cannot allocate an array of constant size 0
     1>..\..\AccessibilityUIElement.cpp(522): error C2133: 'valueElements' : unknown size

Unfortunately you weren't able to catch this build failure, because others broke
the Windows build previously and didn't care about the buildfix at all.
Comment 7 Brent Fulgham 2014-04-28 14:38:43 PDT
Unfortunately, Visual Studio isn't smart about this logic:

    size_t elementCount = elements.size();
    JSValueRef valueElements[elementCount];

... giving the following errors:

         AccessibilityUIElement.cpp
         TestRunner.cpp
     1>..\..\AccessibilityUIElement.cpp(522): error C2057: expected constant expression
     1>..\..\AccessibilityUIElement.cpp(522): error C2466: cannot allocate an array of constant size 0
     1>..\..\AccessibilityUIElement.cpp(522): error C2133: 'valueElements' : unknown size
     1>Done Building Project "C:\cygwin\home\buildbot\slave\win-debug\build\Tools\DumpRenderTree\DumpRenderTree.vcxproj\DumpRenderTree\DumpRenderTree.vcxproj" (Build target(s)) -- FAILED.
Comment 8 Michael Saboff 2014-04-28 16:28:25 PDT
Will post an updated fix that should work for windows.
Comment 9 Michael Saboff 2014-04-28 16:44:07 PDT
Created attachment 230336 [details]
Updated patch with speculative Windows fix
Comment 10 Michael Saboff 2014-04-28 17:32:17 PDT
Committed r167913: <http://trac.webkit.org/changeset/167913>