Log Message
GetByVal to GetById conversion in the DFG is incorrect for getters with control flow https://bugs.webkit.org/show_bug.cgi?id=163629
Reviewed by Yusuke Suzuki. JSTests: * stress/get-by-val-to-id-with-getter.js: Added. (foo): (o.get hello): Source/_javascript_Core: This patch fixes a bug in the DFG when attempt to convert a GetByVal into a GetById. While converting the GetByVal, during handleGetById in the Bytecode parser, we would mistakenly use the opcode length of op_get_by_id rather than op_get_by_val. This causes the new basic block we create to point to the wrong offset. In the added test this will cause us to infinite loop. * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleGetById): (JSC::DFG::ByteCodeParser::parseBlock):
Modified Paths
- trunk/JSTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (207499 => 207500)
--- trunk/JSTests/ChangeLog 2016-10-18 23:33:22 UTC (rev 207499)
+++ trunk/JSTests/ChangeLog 2016-10-18 23:40:10 UTC (rev 207500)
@@ -1,3 +1,14 @@
+2016-10-18 Keith Miller <keith_mil...@apple.com>
+
+ GetByVal to GetById conversion in the DFG is incorrect for getters with control flow
+ https://bugs.webkit.org/show_bug.cgi?id=163629
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/get-by-val-to-id-with-getter.js: Added.
+ (foo):
+ (o.get hello):
+
2016-10-15 Filip Pizlo <fpi...@apple.com>
DFG and FTL should be able to use DirectCall ICs when they proved the callee or its executable
Added: trunk/JSTests/stress/get-by-val-to-id-with-getter.js (0 => 207500)
--- trunk/JSTests/stress/get-by-val-to-id-with-getter.js (rev 0)
+++ trunk/JSTests/stress/get-by-val-to-id-with-getter.js 2016-10-18 23:40:10 UTC (rev 207500)
@@ -0,0 +1,25 @@
+
+// Test GetByVal => GetById conversion works correctly when inlining a getter in the DFG.
+function foo(obj, val) {
+ if (obj[val]) {
+ return 1;
+ }
+ return 0;
+}
+noInline(foo);
+
+
+o = { num: 0,
+ get hello() {
+ if (this.num === 1)
+ return true;
+ return false;
+ }
+ };
+
+for(i = 0; i < 100000; ++i) {
+ let num = i % 2;
+ o.num = num;
+ if (foo(o, "hello") !== num)
+ throw new Error("bad result on iteration: " + i);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (207499 => 207500)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-18 23:33:22 UTC (rev 207499)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-18 23:40:10 UTC (rev 207500)
@@ -1,3 +1,21 @@
+2016-10-18 Keith Miller <keith_mil...@apple.com>
+
+ GetByVal to GetById conversion in the DFG is incorrect for getters with control flow
+ https://bugs.webkit.org/show_bug.cgi?id=163629
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch fixes a bug in the DFG when attempt to convert a
+ GetByVal into a GetById. While converting the GetByVal, during
+ handleGetById in the Bytecode parser, we would mistakenly use the
+ opcode length of op_get_by_id rather than op_get_by_val. This causes
+ the new basic block we create to point to the wrong offset. In the
+ added test this will cause us to infinite loop.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleGetById):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+
2016-10-18 Dean Jackson <d...@apple.com>
Remove CSS_SHAPES feature definition. This should always be on.
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (207499 => 207500)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-10-18 23:33:22 UTC (rev 207499)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-10-18 23:40:10 UTC (rev 207500)
@@ -239,7 +239,7 @@
Node* store(Node* base, unsigned identifier, const PutByIdVariant&, Node* value);
void handleGetById(
- int destinationOperand, SpeculatedType, Node* base, unsigned identifierNumber, GetByIdStatus, AccessType);
+ int destinationOperand, SpeculatedType, Node* base, unsigned identifierNumber, GetByIdStatus, AccessType, unsigned instructionSize);
void emitPutById(
Node* base, unsigned identifierNumber, Node* value, const PutByIdStatus&, bool isDirect);
void handlePutById(
@@ -3272,7 +3272,7 @@
void ByteCodeParser::handleGetById(
int destinationOperand, SpeculatedType prediction, Node* base, unsigned identifierNumber,
- GetByIdStatus getByIdStatus, AccessType type)
+ GetByIdStatus getByIdStatus, AccessType type, unsigned instructionSize)
{
// Attempt to reduce the set of things in the GetByIdStatus.
if (base->op() == NewObject) {
@@ -3434,7 +3434,7 @@
addToGraph(ExitOK);
handleCall(
- destinationOperand, Call, InlineCallFrame::GetterCall, OPCODE_LENGTH(op_get_by_id),
+ destinationOperand, Call, InlineCallFrame::GetterCall, instructionSize,
getter, numberOfParameters - 1, registerOffset, *variant.callLinkStatus(), prediction);
}
@@ -4210,7 +4210,7 @@
}
if (compiledAsGetById)
- handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);
+ handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get, OPCODE_LENGTH(op_get_by_val));
else {
ArrayMode arrayMode = getArrayMode(currentInstruction[4].u.arrayProfile, Array::Read);
Node* getByVal = addToGraph(GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction), base, property);
@@ -4348,9 +4348,11 @@
m_inlineStackTop->m_stubInfos, m_dfgStubInfos,
currentCodeOrigin(), uid);
AccessType type = op_try_get_by_id == opcodeID ? AccessType::GetPure : AccessType::Get;
-
+
+ unsigned opcodeLength = opcodeID == op_try_get_by_id ? OPCODE_LENGTH(op_try_get_by_id) : OPCODE_LENGTH(op_get_by_id);
+
handleGetById(
- currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, type);
+ currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, type, opcodeLength);
if (op_try_get_by_id == opcodeID)
NEXT_OPCODE(op_try_get_by_id); // Opcode's length is different from others in this case.
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
