RESOLVED FIXED 123848
crash when executing Thunk checks on ARM
https://bugs.webkit.org/show_bug.cgi?id=123848
Summary crash when executing Thunk checks on ARM
Mandeep Singh Baines
Reported 2013-11-05 19:06:00 PST
I'm getting a crash when executing the thunk checks from code generated by virtualForThunkGenerator. I think this is a regression introduced when JIT was migrated to use the same thunks as DFG: Author: msaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc > Date: Wed Oct 9 13:29:00 2013 +0000 Transition call and construct JITStubs to CCallHelper functions https://bugs.webkit.org/show_bug.cgi?id=122453 Reviewed by Geoffrey Garen. Transitioned cti_op_call_eval to operationCallEval. Migrated baseline JIT to use the same call thunks as the DFG. Eliminated all of the "oldStyle" thunks and related functions. ... git-svn-id: http://svn.webkit.org/repository/webkit/trunk@157164 268f45cc-cd0 9-0410-ab3c-d52691b4dbfc The check is generated here: <snip> static MacroAssemblerCodeRef virtualForThunkGenerator( VM* vm, CodeSpecializationKind kind) { // The return address is on the stack, or in the link register. We will henc\ e // jump to the callee, or save the return address to the call frame while we // make a C++ function call to the appropriate JIT operation. CCallHelpers jit(vm); CCallHelpers::JumpList slowCase; // FIXME: we should have a story for eliminating these checks. In many cases\ , // the DFG knows that the value is definitely a cell, or definitely a functi\ on. #if USE(JSVALUE64) slowCase.append( jit.branchTest64( CCallHelpers::NonZero, GPRInfo::nonArgGPR0, GPRInfo::tagMaskRegister\ )); #else slowCase.append( jit.branch32( CCallHelpers::NotEqual, GPRInfo::nonArgGPR1, CCallHelpers::TrustedImm32(JSValue::CellTag))); #endif jit.loadPtr(CCallHelpers::Address(GPRInfo::nonArgGPR0, JSCell::structureOffs\ et()), GPRInfo::nonArgGPR2); slowCase.append( jit.branchPtr( CCallHelpers::NotEqual, CCallHelpers::Address(GPRInfo::nonArgGPR2, Structure::classInfoOffse\ t()), CCallHelpers::TrustedImmPtr(JSFunction::info()))); </snip> It is expecting the callee to be stored in nonArgGPR1 and nonArgGPR0. However, the JIT path is storing the callee is in regT1 and regT0 if you look at JIT::compileOpCall. This used to work because the check used to be generated by oldStyleLinkForGenerator which checked regT0 and regT1. -static MacroAssemblerCodeRef oldStyleVirtualForGenerator(VM* vm, FunctionPtr compile, FunctionPtr notJSFunction, const char* name, CodeSpecializationKind kind) -{ - JSInterfaceJIT jit(vm); - - JSInterfaceJIT::JumpList slowCase; - -#if USE(JSVALUE64) - slowCase.append(jit.emitJumpIfNotJSCell(JSInterfaceJIT::regT0)); -#else // USE(JSVALUE64) - slowCase.append(jit.branch32(JSInterfaceJIT::NotEqual, JSInterfaceJIT::regT1, JSInterfaceJIT::TrustedImm32(JSValue::CellTag))); -#endif // USE(JSVALUE64) - slowCase.append(jit.emitJumpIfNotType(JSInterfaceJIT::regT0, JSInterfaceJIT::regT1, JSFunctionType)); - I think the correct fix is to modify virtualForThunkGenerator to use regT0 and regT1.
Attachments
the patch (6.34 KB, patch)
2013-11-06 11:08 PST, Mandeep Singh Baines
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (389.77 KB, application/zip)
2013-11-07 08:18 PST, Build Bot
no flags
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (630.97 KB, application/zip)
2013-11-08 07:13 PST, Build Bot
no flags
Mandeep Singh Baines
Comment 1 2013-11-06 11:08:06 PST
Created attachment 216194 [details] the patch This crash affects all architectures. Due to random luck, I was only see a crash on ARM. Attached is a fix which changes the thunk to use the regT* convention for storing the callee that was previously used by the baseline JIT thunk.
Build Bot
Comment 2 2013-11-07 08:18:04 PST
Comment on attachment 216194 [details] the patch Attachment 216194 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22718211 New failing tests: js/regress/emscripten-cube2hash.html cssom/cssvalue-comparison.html js/dom/JSON-parse.html fast/canvas/canvas-blending-pattern-over-pattern.html inspector-protocol/debugger/call-frame-this-strict.html js/mozilla/strict/8.12.5.html fast/canvas/canvas-blending-image-over-image.html cssom/cssstyledeclaration-csstext-final-delimiter.html fast/canvas/webgl/array-unit-tests.html cssom/cssimportrule-media.html fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml js/mozilla/strict/13.1.html http/tests/canvas/webgl/origin-clean-conformance.html fast/regions/webkit-region-syntax-space.html fast/media/mq-color-index-02.html inspector-protocol/dom/remove-multiple-nodes.html fast/media/w3c/test_media_queries.html fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport.html jquery/attributes.html fast/xpath/xpath-iterator-result-should-mark-its-nodeset.html fast/media/mq-js-update-media.html inspector-protocol/debugger/setBreakpoint-condition.html js/mozilla/strict/11.1.5.html inspector-protocol/debugger/call-frame-this-nonstrict.html fast/canvas/webgl/arraybuffer-transfer-of-control.html fast/regions/webkit-flow-into-parsing.html inspector-protocol/debugger/setBreakpoint-actions.html js/dom/JSON-stringify.html jquery/core.html inspector-protocol/model/content-flow-content-nodes.html
Build Bot
Comment 3 2013-11-07 08:18:06 PST
Created attachment 216303 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Julien Brianceau
Comment 4 2013-11-08 04:47:44 PST
What ARM architecture do you use? If it's ARM_TRADITIONAL, the last stable revision I know is r158882 + changeset r158915
Build Bot
Comment 5 2013-11-08 07:13:43 PST
Comment on attachment 216194 [details] the patch Attachment 216194 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/22598522 New failing tests: cssom/cssvalue-comparison.html js/dom/JSON-parse.html fast/canvas/canvas-blending-pattern-over-pattern.html inspector-protocol/debugger/call-frame-this-strict.html js/mozilla/strict/8.12.5.html inspector-protocol/debugger/setBreakpoint-options-exception.html fast/canvas/canvas-blending-image-over-image.html cssom/cssstyledeclaration-csstext-final-delimiter.html fast/canvas/webgl/array-unit-tests.html cssom/cssimportrule-media.html fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml http/tests/canvas/webgl/origin-clean-conformance.html fast/regions/webkit-region-syntax-space.html fast/media/mq-color-index-02.html inspector-protocol/dom/remove-multiple-nodes.html fast/media/w3c/test_media_queries.html fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport.html jquery/attributes.html jquery/data.html fast/media/mq-js-update-media.html inspector-protocol/debugger/setBreakpoint-condition.html js/mozilla/strict/11.1.5.html inspector-protocol/debugger/call-frame-this-nonstrict.html fast/canvas/webgl/arraybuffer-transfer-of-control.html fast/regions/webkit-flow-into-parsing.html inspector-protocol/debugger/setBreakpoint-actions.html js/dom/JSON-stringify.html jquery/core.html inspector-protocol/model/content-flow-content-nodes.html jquery/css.html
Build Bot
Comment 6 2013-11-08 07:13:45 PST
Created attachment 216395 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Mandeep Singh Baines
Comment 7 2013-11-08 09:39:42 PST
Hi Julien, Thanks for the reply. I'm building for ARMv7. I'm very new to WebKit. This is actually the first time I've sent a patch. Does the patch look reasonable (minus the tests that seem to be failing)? Still getting my head around JSC. So there are stable versions? Where is the list of stable versions posted? Regards, Mandeep
Julien Brianceau
Comment 8 2013-11-10 08:58:58 PST
(In reply to comment #7) > I'm very new to WebKit. This is actually the first time I've sent a patch. That's nice to read ! > Does the patch look reasonable (minus the tests that seem to be failing)? Still getting my head around JSC. Honestly, I don't think so, see https://bugs.webkit.org/show_bug.cgi?id=123277 for further information about this. Moreover, Mark's already fixed your issue with changeset 158315 (http://trac.webkit.org/changeset/158315). > So there are stable versions? Yes, fortunately ;) > Where is the list of stable versions posted? As far as I know, there's no such list. If you want to get this information, you might want to set up a buildbot running jsc tests for ARMv7 architecture to track build status and regressions. You also might want to add your bot in the webkit waterfall (http://build.webkit.org/waterfall).
Csaba Osztrogonác
Comment 9 2015-02-05 04:53:05 PST
close, based on Comment8
Note You need to log in before you can comment on or make changes to this bug.