[deoptimizer] Materialize objects with top-most stub frame.

This makes sure the deoptimizer properly materializes heap objects, even
when the top-most frame happens to be a stub-frame. Without this step
the {arguments_marker} would leak into user-land and most likely be
treated as an undefined value.

R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-769852
BUG=chromium:769852

Change-Id: I4ba17501c5d7e68d1f402b7c2cc5ccb0fb7bfb05
Reviewed-on: https://chromium-review.googlesource.com/691996
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48262}

src/deoptimizer.cc

@@ -1781,6 +1781,15 @@ void Deoptimizer::DoComputeBuiltinContinuation(
     }
   }
 
+  // Clear the context register. The context might be a de-materialized object
+  // and will be materialized by {Runtime_NotifyStubFailure}. For additional
+  // safety we use Smi(0) instead of the potential {arguments_marker} here.
+  if (is_topmost) {
+    intptr_t context_value = reinterpret_cast<intptr_t>(Smi::kZero);
+    Register context_reg = JavaScriptFrame::context_register();
+    output_frame->SetRegister(context_reg.code(), context_value);
+  }
+
   // Ensure the frame pointer register points to the callee's frame. The builtin
   // will build its own frame once we continue to it.
   Register fp_reg = JavaScriptFrame::fp_register();

src/runtime/runtime-compiler.cc

@@ -128,12 +128,35 @@ RUNTIME_FUNCTION(Runtime_InstantiateAsmJs) {
   return Smi::kZero;
 }
 
+namespace {
+
+void MaterializeHeapObjectsAndDeleteDeoptimizer(Isolate* isolate,
+                                                Deoptimizer* deoptimizer) {
+  DCHECK(AllowHeapAllocation::IsAllowed());
+  DCHECK_NULL(isolate->context());
+  // TODO(turbofan): We currently need the native context to materialize
+  // the arguments object, but only to get to its map.
+  isolate->set_context(deoptimizer->function()->native_context());
+
+  // Make sure to materialize objects before causing any allocation.
+  deoptimizer->MaterializeHeapObjects();
+  delete deoptimizer;
+
+  // Ensure the context register is updated for materialized objects.
+  JavaScriptFrameIterator top_it(isolate);
+  JavaScriptFrame* top_frame = top_it.frame();
+  isolate->set_context(Context::cast(top_frame->context()));
+}
+
+}  // namespace
+
 RUNTIME_FUNCTION(Runtime_NotifyStubFailure) {
   HandleScope scope(isolate);
   DCHECK_EQ(0, args.length());
   Deoptimizer* deoptimizer = Deoptimizer::Grab(isolate);
-  DCHECK(AllowHeapAllocation::IsAllowed());
-  delete deoptimizer;
+  DCHECK(deoptimizer->compiled_code()->kind() == Code::OPTIMIZED_FUNCTION);
+  DCHECK(deoptimizer->compiled_code()->is_turbofanned());
+  MaterializeHeapObjectsAndDeleteDeoptimizer(isolate, deoptimizer);
   return isolate->heap()->undefined_value();
 }
 
@@ -144,35 +167,24 @@ RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
   Deoptimizer::BailoutType type =
       static_cast<Deoptimizer::BailoutType>(type_arg);
   Deoptimizer* deoptimizer = Deoptimizer::Grab(isolate);
-  DCHECK(AllowHeapAllocation::IsAllowed());
-  TimerEventScope<TimerEventDeoptimizeCode> timer(isolate);
-  TRACE_EVENT0("v8", "V8.DeoptimizeCode");
-
-  Handle<JSFunction> function = deoptimizer->function();
-
   DCHECK(deoptimizer->compiled_code()->kind() == Code::OPTIMIZED_FUNCTION);
   DCHECK(deoptimizer->compiled_code()->is_turbofanned());
+
+  TimerEventScope<TimerEventDeoptimizeCode> timer(isolate);
+  TRACE_EVENT0("v8", "V8.DeoptimizeCode");
+  Handle<JSFunction> function = deoptimizer->function();
   DCHECK(type == deoptimizer->bailout_type());
-  DCHECK_NULL(isolate->context());
-  // TODO(turbofan): We currently need the native context to materialize
-  // the arguments object, but only to get to its map.
-  isolate->set_context(function->native_context());
 
-  // Make sure to materialize objects before causing any allocation.
-  deoptimizer->MaterializeHeapObjects();
-  delete deoptimizer;
+  MaterializeHeapObjectsAndDeleteDeoptimizer(isolate, deoptimizer);
 
-  // Ensure the context register is updated for materialized objects.
-  JavaScriptFrameIterator top_it(isolate);
-  JavaScriptFrame* top_frame = top_it.frame();
-  isolate->set_context(Context::cast(top_frame->context()));
-
-  if (type == Deoptimizer::LAZY) {
-    return isolate->heap()->undefined_value();
+  // TODO(mstarzinger): The marking of the function for deoptimization is the
+  // only difference to {Runtime_NotifyStubFailure} by now and we should also
+  // do this if the top-most frame is a builtin stub to avoid deoptimization
+  // loops. This would also unify the two runtime functions.
+  if (type != Deoptimizer::LAZY) {
+    Deoptimizer::DeoptimizeFunction(*function);
   }
 
-  Deoptimizer::DeoptimizeFunction(*function);
-
   return isolate->heap()->undefined_value();
 }
 

test/mjsunit/regress/regress-crbug-769852.js

@@ -0,0 +1,14 @@
+// Copyright 2017 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: --allow-natives-syntax
+
+function f(o) {
+  function g() {}
+  Object.keys(o).forEach(suite => g());
+}
+assertDoesNotThrow(() => f({}));
+assertDoesNotThrow(() => f({ x:0 }));
+%OptimizeFunctionOnNextCall(f);
+assertDoesNotThrow(() => f({ x:0 }));