Changeset 207500 in webkit

Timestamp:

Oct 18, 2016 4:40:10 PM (8 years ago)

Author:

keith_miller@apple.com

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/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/get-by-val-to-id-with-getter.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (modified) (5 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2016-10-18  Keith Miller  <keith_miller@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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-10-18  Keith Miller  <keith_miller@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  <dino@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -240 +240 @@
 
     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);
@@ -3273 +3273 @@
 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.
@@ -3435 +3435 @@
     
     handleCall(
-        destinationOperand, Call, InlineCallFrame::GetterCall, OPCODE_LENGTH(op_get_by_id),
+        destinationOperand, Call, InlineCallFrame::GetterCall, instructionSize,
         getter, numberOfParameters - 1, registerOffset, *variant.callLinkStatus(), prediction);
 }
@@ -4211 +4211 @@
 
             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);
@@ -4349 +4349 @@
                 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)