Bug 116315

Summary: DFG_OPERATIONs with double arguments generate incorrect code on ARM soft-fp
Product: WebKit Reporter: Roman Zhuykov <zhroma>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: darin, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108645    
Attachments:
Description Flags
Proposed patch none

Roman Zhuykov
Reported 2013-05-17 07:36:35 PDT
I found that on ARMv7 Linux, with -mfloat-abi=softfp, v8-splay.js test crashes (segmentation fault). Also stanford-crypto-aes test from Kraken shows wrong results. The problem happens when calling DFG_OPERATIONs operationArrayPushDouble, operationPutDoubleByValBeyondArrayBoundsStrict, operationPutDoubleByValBeyondArrayBoundsNonStrict. One of their arguments is double, and all of them receive wrong last argument from DFG assembly. I fixed the setupArgumentsWithExecState function to prepare arguments in a proper way (as expected by AAPCS and GCC). There are two layout tests in the patch: array-with-double-dfg-push checks push(double) operations, array-with-double-dfg-assign checks assignment of double element beyond the array bounds.
Attachments
Proposed patch (6.60 KB, patch)
2013-05-17 07:41 PDT, Roman Zhuykov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Roman Zhuykov
Comment 1 2013-05-17 07:41:37 PDT
Created attachment 202085 [details] Proposed patch
Mark Hahnenberg
Comment 2 2013-11-04 10:40:17 PST
Comment on attachment 202085 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=202085&action=review Sorry, could you rebase? It makes getting context for the review easier. > Source/JavaScriptCore/dfg/DFGCCallHelpers.h:580 > #endif // CPU(ARM_HARDFP) I'm no expert, but is this the #define for ARM_HARDFP? Why are we assuming the softfp ABI inside of this #define?
Csaba Osztrogonác
Comment 3 2013-11-04 10:49:51 PST
Comment on attachment 202085 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=202085&action=review >> Source/JavaScriptCore/dfg/DFGCCallHelpers.h:580 >> #endif // CPU(ARM_HARDFP) > > I'm no expert, but is this the #define for ARM_HARDFP? Why are we assuming the softfp ABI inside of this #define? This #endif comment is very very misleading, because it is the end of the _else_ case of CPU(ARM_HARDFP). (Otherwise this file is moved to Source/JavaScriptCore/jit/CCallHelpers.h.)
Roman Zhuykov
Comment 4 2013-11-19 04:40:05 PST
(In reply to comment #2) > Sorry, could you rebase? It makes getting context for the review easier. This bug was already fixed here https://bugs.webkit.org/show_bug.cgi?id=117281
Csaba Osztrogonác
Comment 5 2013-11-19 04:57:01 PST
*** This bug has been marked as a duplicate of bug 117281 ***
Note You need to log in before you can comment on or make changes to this bug.