Changeset 184960 in webkit
- Timestamp:
- May 28, 2015 1:57:48 PM (9 years ago)
- Location:
- trunk/Source/JavaScriptCore
- Files:
-
- 2 added
- 2 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/JavaScriptCore/ChangeLog
r184959 r184960 1 2015-05-28 Benjamin Poulain <benjamin@webkit.org> 2 3 [iOS8][ARMv7(s)] Optimized Object.create in 'use strict' context sometimes breaks. 4 https://bugs.webkit.org/show_bug.cgi?id=138038 5 6 Reviewed by Michael Saboff. 7 8 TL;DR: sometimes the baseline JIT could accidentally nuke the tag before calling 9 to C++, making put_by_id behave erratically. 10 11 The bug was that put_by_id would randomly not work correctly in 32bits. It happened 12 in the baseline JIT if we were unlucky enough: 13 -The code get hot enough and the structure is stable so we get a fast path for 14 put_by_id. 15 -We repatch the fast-path branch with a stub generated by 16 emitPutTransitionStubAndGetOldStructure(). 17 -In emitPutTransitionStubAndGetOldStructure(), we only preserve the payload of the base 18 register, the tag register is ignored. 19 -emitPutTransitionStubAndGetOldStructure() allocate 2 to 3 registers. Any of those 20 could be the one used for the base's tag before the fast path and the value is trashed. 21 -If we hit one of the failure case, we fallback to the slow path, but we destroyed 22 the tag pointer. 23 -We now have unrelated bits in the tag, the most likely value type is now "double" 24 and we fail the put_by_id because we try to set a property on a number. 25 26 The most obvious solution would be to change emitPutTransitionStubAndGetOldStructure() 27 to preserve the tag register in addition to the value register. 28 I decided against that option because of the added complexity. The DFG does not need 29 that case, so I would have to add branches everywhere to distinguish the cases 30 were we need to preserve the tag or not. 31 32 Instead, I just load the tag back from memory in the slow path. The function in the slow 33 path is several order of magnitude slower than a load, it is not worth eliminating it, 34 especially in baseline JIT. 35 36 I also discovered 4 useless loads in the fast path, so even with my extra load, this patch 37 makes the baseline faster :) 38 39 * jit/JITPropertyAccess32_64.cpp: 40 (JSC::JIT::emitSlow_op_put_by_id): 41 (JSC::JIT::emit_op_put_by_id): Deleted. 42 * tests/stress/put-by-id-on-new-object-after-prototype-transition-non-strict.js: Added. 43 (opaqueNewObject): 44 (putValueOnNewObject): 45 * tests/stress/put-by-id-on-new-object-after-prototype-transition-strict.js: Added. 46 (string_appeared_here.opaqueNewObject): 47 (putValueOnNewObject): 48 1 49 2015-05-28 Benjamin Poulain <benjamin@webkit.org> 2 50 -
trunk/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
r184324 r184960 536 536 537 537 emitJumpSlowCaseIfNotJSCell(base, regT1); 538 539 emitLoad(base, regT1, regT0);540 emitLoad(value, regT3, regT2);541 538 542 539 JITPutByIdGenerator gen( … … 560 557 561 558 Label coldPathBegin(this); 562 559 560 // JITPutByIdGenerator only preserve the value and the base's payload, we have to reload the tag. 561 emitLoadTag(base, regT1); 562 563 563 JITPutByIdGenerator& gen = m_putByIds[m_putByIdIndex++]; 564 564
Note: See TracChangeset
for help on using the changeset viewer.