Changeset 194021 in webkit
- Timestamp:
- Dec 13, 2015 6:52:51 PM (8 years ago)
- Location:
- trunk/Source/JavaScriptCore
- Files:
-
- 1 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/JavaScriptCore/ChangeLog
r194017 r194021 1 2015-12-13 Yusuke Suzuki <utatane.tea@gmail.com> 2 3 [JSC] Should not emit get_by_id for indexed property access 4 https://bugs.webkit.org/show_bug.cgi?id=151354 5 6 Reviewed by Darin Adler. 7 8 Before this patch, `a["1"]` is converted to `a.1` get_by_id operation in the bytecode compiler. 9 get_by_id emits IC. IC rely on the fact that Structure transition occur when adding / removing object's properties. 10 However, it's not true for indexed element properties. They are stored in the element storage and Structure transition does not occur. 11 12 For example, in the following case, 13 14 function getOne(a) { return a['1']; } 15 16 for (var i = 0; i < 36; ++i) 17 getOne({2: true}); 18 19 if (!getOne({1: true})) 20 throw new Error("OUT"); 21 22 In this case, `a['1']` creates get_by_id. `getOne({2: true})` calls makes getOne's get_by_id to create IC says that, 23 "when comming this structure chain, there is no property in "1", so we should return `undefined`". 24 25 After that, we call `getOne({1: true})`. But in this case, `{2: true}` and `{1: true}` have the same structure chain, 26 because indexed property addition does not occur structure transition. 27 So previous IC fast path is used and return `undefined`. But the correct answer is returning `true`. 28 29 This patch fixes the above issue. When there is string bracket access, we only emits get_by_id if the given string is not an index. 30 There are bugs in get_by_id, put_by_id, put_by_id (direct). But only get_by_id poses user observable issue. 31 Because in the put_by_id case, the generic path just says "this put is uncacheable". 32 33 * bytecompiler/BytecodeGenerator.cpp: 34 (JSC::BytecodeGenerator::emitGetById): 35 (JSC::BytecodeGenerator::emitPutById): 36 (JSC::BytecodeGenerator::emitDirectPutById): 37 * bytecompiler/NodesCodegen.cpp: 38 (JSC::isNonIndexStringElement): 39 (JSC::BracketAccessorNode::emitBytecode): 40 (JSC::FunctionCallBracketNode::emitBytecode): 41 (JSC::AssignBracketNode::emitBytecode): 42 (JSC::ObjectPatternNode::bindValue): 43 * tests/stress/element-property-get-should-not-handled-with-get-by-id.js: Added. 44 (getOne): 45 1 46 2015-12-13 Andreas Kling <akling@apple.com> 2 47 -
trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
r193974 r194021 2285 2285 RegisterID* BytecodeGenerator::emitGetById(RegisterID* dst, RegisterID* base, const Identifier& property) 2286 2286 { 2287 ASSERT_WITH_MESSAGE(!parseIndex(property), "Indexed properties should be handled with get_by_val."); 2288 2287 2289 m_codeBlock->addPropertyAccessInstruction(instructions().size()); 2288 2290 … … 2301 2303 RegisterID* BytecodeGenerator::emitPutById(RegisterID* base, const Identifier& property, RegisterID* value) 2302 2304 { 2305 ASSERT_WITH_MESSAGE(!parseIndex(property), "Indexed properties should be handled with put_by_val."); 2306 2303 2307 unsigned propertyIndex = addConstant(property); 2304 2308 … … 2322 2326 RegisterID* BytecodeGenerator::emitDirectPutById(RegisterID* base, const Identifier& property, RegisterID* value, PropertyNode::PutType putType) 2323 2327 { 2324 ASSERT(!parseIndex(property)); 2328 ASSERT_WITH_MESSAGE(!parseIndex(property), "Indexed properties should be handled with put_by_val(direct)."); 2329 2325 2330 unsigned propertyIndex = addConstant(property); 2326 2331 -
trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
r193974 r194021 603 603 // ------------------------------ BracketAccessorNode -------------------------------- 604 604 605 static bool isNonIndexStringElement(ExpressionNode& element) 606 { 607 return element.isString() && !parseIndex(static_cast<StringNode&>(element).value()); 608 } 609 605 610 RegisterID* BracketAccessorNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst) 606 611 { 607 612 if (m_base->isSuperNode()) { 608 613 // FIXME: Should we generate the profiler info? 609 if ( m_subscript->isString()) {614 if (isNonIndexStringElement(*m_subscript)) { 610 615 const Identifier& id = static_cast<StringNode*>(m_subscript)->value(); 611 616 return generator.emitGetById(generator.finalDestination(dst), emitSuperBaseForCallee(generator), id); … … 617 622 RegisterID* finalDest = generator.finalDestination(dst); 618 623 619 if ( m_subscript->isString()) {624 if (isNonIndexStringElement(*m_subscript)) { 620 625 RefPtr<RegisterID> base = generator.emitNode(m_base); 621 626 ret = generator.emitGetById(finalDest, base.get(), static_cast<StringNode*>(m_subscript)->value()); … … 847 852 { 848 853 bool baseIsSuper = m_base->isSuperNode(); 849 bool subscriptIs String = m_subscript->isString();854 bool subscriptIsNonIndexString = isNonIndexStringElement(*m_subscript); 850 855 851 856 RefPtr<RegisterID> base; … … 853 858 base = emitSuperBaseForCallee(generator); 854 859 else { 855 if (subscriptIs String)860 if (subscriptIsNonIndexString) 856 861 base = generator.emitNode(m_base); 857 862 else … … 860 865 861 866 RefPtr<RegisterID> function; 862 if (subscriptIs String) {867 if (subscriptIsNonIndexString) { 863 868 generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd()); 864 869 function = generator.emitGetById(generator.tempDestination(dst), base.get(), static_cast<StringNode*>(m_subscript)->value()); … … 1978 1983 RegisterID* forwardResult = (dst == generator.ignoredResult()) ? result.get() : generator.moveToDestinationIfNeeded(generator.tempDestination(result.get()), result.get()); 1979 1984 1980 if ( m_subscript->isString())1985 if (isNonIndexStringElement(*m_subscript)) 1981 1986 generator.emitPutById(base.get(), static_cast<StringNode*>(m_subscript)->value(), forwardResult); 1982 1987 else … … 3486 3491 auto& target = m_targetPatterns[i]; 3487 3492 RefPtr<RegisterID> temp = generator.newTemporary(); 3488 if (!target.propertyExpression) 3489 generator.emitGetById(temp.get(), rhs, target.propertyName); 3490 else { 3493 if (!target.propertyExpression) { 3494 // Should not emit get_by_id for indexed ones. 3495 Optional<uint32_t> optionalIndex = parseIndex(target.propertyName); 3496 if (!optionalIndex) 3497 generator.emitGetById(temp.get(), rhs, target.propertyName); 3498 else { 3499 RefPtr<RegisterID> index = generator.emitLoad(generator.newTemporary(), jsNumber(optionalIndex.value())); 3500 generator.emitGetByVal(temp.get(), rhs, index.get()); 3501 } 3502 } else { 3491 3503 RefPtr<RegisterID> propertyName = generator.emitNode(target.propertyExpression); 3492 3504 generator.emitGetByVal(temp.get(), rhs, propertyName.get());
Note: See TracChangeset
for help on using the changeset viewer.