Bug 119433

Summary: REGRESSION: ARM Still crashes after change set r153612
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Blocker CC: allan.jensen, barraclough, cgarcia, commit-queue, fpizlo, ggaren, hausmann, jbriance, loki, mark.lam, mhahnenberg, msaboff, oliver, ossy, rgabor, xinchao.peng, zan, zherczeg
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: All   
Bug Depends on: 119140, 119569    
Bug Blocks: 108645    
Attachments:
Description Flags
Patch to set callFrameRegister to value returned by cti_vm_throw_slowpath
none
ctiVMThrowTrampolineSlowpath fix for ARM and ARMv7 architectures. none

Michael Saboff
Reported 2013-08-01 23:55:04 PDT
The fix https://bugs.webkit.org/show_bug.cgi?id=119140 "REGRESSION: Crash beneath cti_vm_throw_slowpath due to invalid CallFrame pointer" didn't completely resolve the crashes on ARM.
Attachments
Patch to set callFrameRegister to value returned by cti_vm_throw_slowpath (2.02 KB, patch)
2013-08-02 00:04 PDT, Michael Saboff 		no flags
DetailsFormatted DiffDiff
ctiVMThrowTrampolineSlowpath fix for ARM and ARMv7 architectures. (3.00 KB, patch)
2013-08-02 01:22 PDT, Julien Brianceau 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2013-08-02 00:04:40 PDT
Created attachment 207987 [details] Patch to set callFrameRegister to value returned by cti_vm_throw_slowpath
WebKit Commit Bot
Comment 2 2013-08-02 00:13:03 PDT
Attachment 207987 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/jit/JITStubsARM.h', u'Source/JavaScriptCore/jit/JITStubsARMv7.h']" exit_code: 1 Source/JavaScriptCore/jit/JITStubsARM.h:211: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Csaba Osztrogonác
Comment 3 2013-08-02 00:54:53 PDT
I tried this patch, but unfortunately it didn't help. - ARM traditional results: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/9252 - ARM Thumb2 results: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/9253 I'll try to collect some debug infos a little bit later.
Julien Brianceau
Comment 4 2013-08-02 01:22:23 PDT
Created attachment 207994 [details] ctiVMThrowTrampolineSlowpath fix for ARM and ARMv7 architectures. I think this would work better. This patch is similar to what I've done for sh4 architecture (in http://trac.webkit.org/changeset/153583).
Csaba Osztrogonác
Comment 5 2013-08-02 06:08:43 PDT
(In reply to comment #4) > Created an attachment (id=207994) [details] > ctiVMThrowTrampolineSlowpath fix for ARM and ARMv7 architectures. > > I think this would work better. This patch is similar to what I've done for sh4 architecture (in http://trac.webkit.org/changeset/153583). It seems it works: - ARM Thumb2: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/9254 (still runs ...) - ARM Traditional: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/9255 (will start in an hour ...)
WebKit Commit Bot
Comment 6 2013-08-02 07:58:17 PDT
Comment on attachment 207994 [details] ctiVMThrowTrampolineSlowpath fix for ARM and ARMv7 architectures. Clearing flags on attachment: 207994 Committed r153648: <http://trac.webkit.org/changeset/153648>
Geoffrey Garen
Comment 7 2013-08-02 08:03:57 PDT
> Update call frame and do not restore registers from JIT stack frame in ARM and ARMv7 implementations of ctiVMThrowTrampolineSlowpath. Can you comment on why we should not restore these registers?
Julien Brianceau
Comment 8 2013-08-02 08:49:00 PDT
(In reply to comment #7) > > Can you comment on why we should not restore these registers? From what I saw with gdb, the handler address retrieved in ctiVMThrowTrampolineSlowpath function in r1 register is likely to be the address of ctiOpThrowNotCaught, where registers are restored from JIT stack frame. So without this patch, we did the work twice, leading to unexpected values and misplaced stack pointer during the "second restore from JIT stack frame" in ctiOpThrowNotCaught.
Csaba Osztrogonác
Comment 9 2013-10-31 13:27:10 PDT
Comment on attachment 207987 [details] Patch to set callFrameRegister to value returned by cti_vm_throw_slowpath The bug is closed and the fix already landed.
Note You need to log in before you can comment on or make changes to this bug.