Commit cabeb7db authored by jkummerow's avatar jkummerow Committed by Commit bot

Fix deoptimization at ForInStatement::BodyId()

Full-codegen prepared for the bailout in the wrong place, causing side
effects to be replayed when they shouldn't. Crankshaft and Turbofan are
in agreement about where the deopt should jump to.

TEST=mjsunit/for-in-opt
R=jarin@chromium.org
BUG=v8:4381
LOG=y

Review URL: https://codereview.chromium.org/1413923005

Cr-Commit-Position: refs/heads/master@{#31607}
parent 0b14070d
......@@ -1139,7 +1139,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(r1, r0); // Fixed array length (as smi) and initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1192,6 +1191,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1136,7 +1136,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(x1, x0, x2, xzr);
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ Bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1187,6 +1186,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1075,7 +1075,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ push(Immediate(Smi::FromInt(0))); // Initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1125,6 +1124,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1138,7 +1138,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(a1, a0); // Fixed array length (as smi) and initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1190,6 +1189,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1136,7 +1136,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(a1, a0); // Fixed array length (as smi) and initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1188,6 +1187,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1105,7 +1105,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(r4, r3); // Fixed array length (as smi) and initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1160,6 +1159,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1097,7 +1097,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ Push(Smi::FromInt(0)); // Initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1150,6 +1149,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -1067,7 +1067,6 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
__ push(Immediate(Smi::FromInt(0))); // Initial index.
// Generate code for doing the condition check.
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
__ bind(&loop);
SetExpressionAsStatementPosition(stmt->each());
......@@ -1117,6 +1116,8 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) {
PrepareForBailoutForId(stmt->AssignmentId(), NO_REGISTERS);
}
// Both Crankshaft and Turbofan expect BodyId to be right before stmt->body().
PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
// Generate code for the body of the loop.
Visit(stmt->body());
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --harmony-proxies --allow-natives-syntax
// Flags: --harmony-proxies --allow-natives-syntax --expose-debug-as debug
"use strict";
......@@ -84,3 +84,77 @@ f3({__proto__:{x:1}});
%OptimizeFunctionOnNextCall(f3);
f3(undefined);
f3(null);
// Reliable repro for an issue previously flushed out by GC stress.
var handler2 = {
getPropertyDescriptor: function(k) {
has_keys.push(k);
return {value: 10, configurable: true, writable: false, enumerable: true};
}
}
var proxy2 = Proxy.create(handler2);
var o2 = {__proto__: proxy2};
var p = {x: "x"}
function f4(o, p) {
var result = [];
for (var i in o) {
var j = p.x + "str";
result.push(i);
}
return result;
}
function check_f4() {
assertEquals(keys, f4(o, p));
assertEquals(keys, has_keys);
has_keys.length = 0;
}
check_f4();
check_f4();
%OptimizeFunctionOnNextCall(f4);
p.y = "y"; // Change map, cause eager deopt.
check_f4();
// Repro for Turbofan equivalent.
var x;
var count = 0;
var Debug = debug.Debug;
function listener(event, exec_state, event_data, data) {
if (event == Debug.DebugEvent.Break) {
%DeoptimizeFunction(f5);
}
}
var handler3 = {
enumerate: function(target) {
return ["a", "b"];
},
getPropertyDescriptor: function(k) {
if (k == "a") count++;
if (x) %ScheduleBreak();
return {value: 10, configurable: true, writable: false, enumerable: true};
}
};
var proxy3 = Proxy.create(handler3);
var o3 = {__proto__: proxy3};
function f5() {
for (var p in o3) {
print(p);
}
}
x = false;
f5(); f5(); f5();
%OptimizeFunctionOnNextCall(f5);
x = true;
count = 0;
Debug.setListener(listener);
f5();
Debug.setListener(null);
assertEquals(1, count);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment