Fix arrow functions requiring context without slots.

This fixes a corner-case where arrow functions that require a context
allocate none, because there are no additional slots allocated. Note
that this didn't happen with true function scopes because they always
had at least the receiver slot.

The outcome was a context chain that no longer was in sync with the
scope chain, hence context slot loads were bogus. This is observable
using the DYNAMIC_LOCAL optimization in all compilers.

R=rossberg@chromium.org,wingo@igalia.com
TEST=mjsunit/harmony/regress/regress-4160
BUG=v8:4160
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#28788}

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index ad74239043554742f07271b549c621f19771e575..a4c6be8aea5f9eccc30f99d6565c3367f653fac2 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -191,17 +191,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     // Argument to NewContext is the function, which is still in r1.
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     if (info->scope()->is_script_scope()) {
       __ push(r1);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc
index c41c05e0381f8fa9cb0b842aa846e003ecbd1cb4..507ce3dab8c9dda37b4ad7e213b9bd72f2155b6b 100644 (file)
--- a/src/arm64/full-codegen-arm64.cc
+++ b/src/arm64/full-codegen-arm64.cc
@@ -192,17 +192,17 @@ void FullCodeGenerator::Generate() {
 
   bool function_in_register_x1 = true;
 
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     // Argument to NewContext is the function, which is still in x1.
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     if (info->scope()->is_script_scope()) {
       __ Mov(x10, Operand(info->scope()->GetScopeInfo(info->isolate())));
       __ Push(x1, x10);
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/code-stubs.h b/src/code-stubs.h
index eca7c76c22365bc03fb6829791cf3e4fdb75dac3..77a2f8bba329faf769b7bc1fff5fa0c94adfed4c 100644 (file)
--- a/src/code-stubs.h
+++ b/src/code-stubs.h
@@ -732,7 +732,7 @@ class FastNewContextStub final : public HydrogenCodeStub {
   static const int kMaximumSlots = 64;
 
   FastNewContextStub(Isolate* isolate, int slots) : HydrogenCodeStub(isolate) {
-    DCHECK(slots > 0 && slots <= kMaximumSlots);
+    DCHECK(slots >= 0 && slots <= kMaximumSlots);
     set_sub_minor_key(SlotsBits::encode(slots));
   }
 

diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 11ac9edf6362c0c44a43d0291fa9f7308d1ca186..d1103d7c907b662d0092f98d9fcb80bde29ebdad 100644 (file)
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -525,8 +525,7 @@ bool AstGraphBuilder::CreateGraph(bool constant_context, bool stack_check) {
   }
 
   // Build function context only if there are context allocated variables.
-  int heap_slots = info()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info()->num_heap_slots() > 0) {
     // Push a new inner context scope for the function.
     Node* inner_context =
         BuildLocalFunctionContext(function_context_.get(), patched_receiver);

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index ffc83df5cddf9cd9dbde5949d09853e9aea0cf20..87aa3a225cdf6cb4e013ec4e5eefe0622f6bff72 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -184,17 +184,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     // Argument to NewContext is the function, which is still in edi.
     if (info->scope()->is_script_scope()) {
       __ push(edi);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc
index 47975c1d8b1b5c413fa9fe6f9c0f20c1badf3cf4..df0cdfa0d03e29d1cb652e336e5553c75a676566 100644 (file)
--- a/src/mips/full-codegen-mips.cc
+++ b/src/mips/full-codegen-mips.cc
@@ -200,17 +200,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate context");
     // Argument to NewContext is the function, which is still in a1.
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     if (info->scope()->is_script_scope()) {
       __ push(a1);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc
index ac56219cea8edef17e99b7a208b93a44802e5027..b7659c2d446db6afacf8b9ce4174c77831d41926 100644 (file)
--- a/src/mips64/full-codegen-mips64.cc
+++ b/src/mips64/full-codegen-mips64.cc
@@ -197,17 +197,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate context");
     // Argument to NewContext is the function, which is still in a1.
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     if (info->scope()->is_script_scope()) {
       __ push(a1);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/ppc/full-codegen-ppc.cc b/src/ppc/full-codegen-ppc.cc
index 6ae6f106fde0f1658e612199af68bb12968ea555..78132a06ccfc0050543db143d1e9a9d2b29a05b8 100644 (file)
--- a/src/ppc/full-codegen-ppc.cc
+++ b/src/ppc/full-codegen-ppc.cc
@@ -196,17 +196,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     // Argument to NewContext is the function, which is still in r4.
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     if (info->scope()->is_script_scope()) {
       __ push(r4);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index 059626531df7f3f769ea479f31ba9c23c1f00dd1..91d40ced070c2006f182d95030da560f7c242f62 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -183,17 +183,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     // Argument to NewContext is the function, which is still in rdi.
     if (info->scope()->is_script_scope()) {
       __ Push(rdi);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc
index cb4173f8039504de5ce752a25ede0fdc465da7a5..5b3f8e0ee7601f45532b9f431f4864c02e08f1df 100644 (file)
--- a/src/x87/full-codegen-x87.cc
+++ b/src/x87/full-codegen-x87.cc
@@ -184,17 +184,17 @@ void FullCodeGenerator::Generate() {
   bool function_in_register = true;
 
   // Possibly allocate a local context.
-  int heap_slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
-  if (heap_slots > 0) {
+  if (info->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
+    int slots = info->scope()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     // Argument to NewContext is the function, which is still in edi.
     if (info->scope()->is_script_scope()) {
       __ push(edi);
       __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
-    } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
-      FastNewContextStub stub(isolate(), heap_slots);
+    } else if (slots <= FastNewContextStub::kMaximumSlots) {
+      FastNewContextStub stub(isolate(), slots);
       __ CallStub(&stub);
       // Result of FastNewContextStub is always in new space.
       need_write_barrier = false;

diff --git a/test/mjsunit/harmony/regress/regress-4160.js b/test/mjsunit/harmony/regress/regress-4160.js
new file mode 100644 (file)
index 0000000..b02daee
--- /dev/null
+++ b/test/mjsunit/harmony/regress/regress-4160.js
@@ -0,0 +1,29 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-arrow-functions --allow-natives-syntax
+
+(function(x) {
+  (function(x) {
+    var boom = (() => eval(x));
+    assertEquals(23, boom());
+    assertEquals(23, boom());
+    %OptimizeFunctionOnNextCall(boom);
+    assertEquals(23, boom());
+    assertEquals("23", x);
+  })("23");
+  assertEquals("42", x);
+})("42");
+
+(function(x) {
+  (function(x) {
+    var boom = (() => (eval("var x = 66"), x));
+    assertEquals(66, boom());
+    assertEquals(66, boom());
+    %OptimizeFunctionOnNextCall(boom);
+    assertEquals(66, boom());
+    assertEquals("23", x);
+  })("23");
+  assertEquals("42", x);
+})("42");