- Revision
- 185627
- Author
- msab...@apple.com
- Date
- 2015-06-16 17:06:14 -0700 (Tue, 16 Jun 2015)
Log Message
Inlining in the DFG trashes ByteCodeParser::m_currentInstruction for the calling function
https://bugs.webkit.org/show_bug.cgi?id=146029
Reviewed by Benjamin Poulain.
Source/_javascript_Core:
Save and restore m_currentInstruction around call to ByteCodeParser::inlineCall() as it will
use m_currentInstruction during its own parsing. This happens because inlineCall() parses the
inlined callee's bytecodes by calling parseCodeBlock() which calls parseBlock() on each block.
It is in parseBlock() that we set m_currentInstruction to an instruction before we parse it.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::attemptToInlineCall):
(JSC::DFG::ByteCodeParser::parseBlock): Added an ASSERT to catch this issue.
LayoutTests:
New regression test.
* js/regress-146029-expected.txt: Added.
* js/regress-146029.html: Added.
* js/script-tests/regress-146029.js: Added.
(myPush):
(myPop):
(foo):
(test):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (185626 => 185627)
--- trunk/LayoutTests/ChangeLog 2015-06-16 23:43:26 UTC (rev 185626)
+++ trunk/LayoutTests/ChangeLog 2015-06-17 00:06:14 UTC (rev 185627)
@@ -1,3 +1,20 @@
+2015-06-16 Michael Saboff <msab...@apple.com>
+
+ Inlining in the DFG trashes ByteCodeParser::m_currentInstruction for the calling function
+ https://bugs.webkit.org/show_bug.cgi?id=146029
+
+ Reviewed by Benjamin Poulain.
+
+ New regression test.
+
+ * js/regress-146029-expected.txt: Added.
+ * js/regress-146029.html: Added.
+ * js/script-tests/regress-146029.js: Added.
+ (myPush):
+ (myPop):
+ (foo):
+ (test):
+
2015-06-16 Brent Fulgham <bfulg...@apple.com>
Unreviewed test update: Add multiple scroll-snap-coordinate test.
Added: trunk/LayoutTests/js/regress-146029-expected.txt (0 => 185627)
--- trunk/LayoutTests/js/regress-146029-expected.txt (rev 0)
+++ trunk/LayoutTests/js/regress-146029-expected.txt 2015-06-17 00:06:14 UTC (rev 185627)
@@ -0,0 +1,10 @@
+Verify that we don't trash m_currentInstruction with an inlined function.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Correctly inlined callee and used m_currentInstruction in caller
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/regress-146029.html (0 => 185627)
--- trunk/LayoutTests/js/regress-146029.html (rev 0)
+++ trunk/LayoutTests/js/regress-146029.html 2015-06-17 00:06:14 UTC (rev 185627)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/script-tests/regress-146029.js (0 => 185627)
--- trunk/LayoutTests/js/script-tests/regress-146029.js (rev 0)
+++ trunk/LayoutTests/js/script-tests/regress-146029.js 2015-06-17 00:06:14 UTC (rev 185627)
@@ -0,0 +1,32 @@
+description("Verify that we don't trash m_currentInstruction with an inlined function.");
+
+function myPush(a, o) {
+ a.push(o);
+}
+
+function myPop(a) {
+ a.pop();
+}
+
+function foo(a) {
+ myPush(a, 42);
+ myPop(a);
+ return a.length;
+}
+
+noInline(foo);
+
+function test() {
+ var myArray = ["one", "two", "three"];
+
+ for (var i = 0; i < 10000; ++i) {
+ if (foo(myArray) != 3) {
+ testFailed("Array changed unexpectedly");
+ return false;
+ }
+ }
+ return true;
+}
+
+if (test())
+ testPassed("Correctly inlined callee and used m_currentInstruction in caller");
Modified: trunk/Source/_javascript_Core/ChangeLog (185626 => 185627)
--- trunk/Source/_javascript_Core/ChangeLog 2015-06-16 23:43:26 UTC (rev 185626)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-06-17 00:06:14 UTC (rev 185627)
@@ -1,3 +1,19 @@
+2015-06-16 Michael Saboff <msab...@apple.com>
+
+ Inlining in the DFG trashes ByteCodeParser::m_currentInstruction for the calling function
+ https://bugs.webkit.org/show_bug.cgi?id=146029
+
+ Reviewed by Benjamin Poulain.
+
+ Save and restore m_currentInstruction around call to ByteCodeParser::inlineCall() as it will
+ use m_currentInstruction during its own parsing. This happens because inlineCall() parses the
+ inlined callee's bytecodes by calling parseCodeBlock() which calls parseBlock() on each block.
+ It is in parseBlock() that we set m_currentInstruction to an instruction before we parse it.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::attemptToInlineCall):
+ (JSC::DFG::ByteCodeParser::parseBlock): Added an ASSERT to catch this issue.
+
2015-06-16 Filip Pizlo <fpi...@apple.com>
Unreviewed, roll out unintended JSC change from https://trac.webkit.org/changeset/185425.
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (185626 => 185627)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-06-16 23:43:26 UTC (rev 185626)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-06-17 00:06:14 UTC (rev 185627)
@@ -1487,8 +1487,10 @@
if (myInliningCost > inliningBalance)
return false;
+ Instruction* savedCurrentInstruction = m_currentInstruction;
inlineCall(callTargetNode, resultOperand, callee, registerOffset, argumentCountIncludingThis, nextOffset, kind, callerLinkability, insertChecks);
inliningBalance -= myInliningCost;
+ m_currentInstruction = savedCurrentInstruction;
return true;
}
@@ -3397,6 +3399,8 @@
case op_call:
handleCall(currentInstruction, Call, CodeForCall);
+ // Verify that handleCall(), which could have inlined the callee, didn't trash m_currentInstruction
+ ASSERT(m_currentInstruction == currentInstruction);
NEXT_OPCODE(op_call);
case op_construct:
