Fix regressions triggered by map invalidation during graph creation.

BUG=
R=jkummerow@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16150 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/assert-scope.h

@@ -41,6 +41,7 @@ enum PerThreadAssertType {
   HANDLE_ALLOCATION_ASSERT,
   HANDLE_DEREFERENCE_ASSERT,
   DEFERRED_HANDLE_DEREFERENCE_ASSERT,
+  CODE_DEPENDENCY_CHANGE_ASSERT,
   LAST_PER_THREAD_ASSERT_TYPE
 };
 
@@ -170,6 +171,14 @@ typedef PerThreadAssertScope<DEFERRED_HANDLE_DEREFERENCE_ASSERT, false>
 typedef PerThreadAssertScope<DEFERRED_HANDLE_DEREFERENCE_ASSERT, true>
     AllowDeferredHandleDereference;
 
+// Scope to document where we do not expect deferred handles to be dereferenced.
+typedef PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT, false>
+    DisallowCodeDependencyChange;
+
+// Scope to introduce an exception to DisallowDeferredHandleDereference.
+typedef PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT, true>
+    AllowCodeDependencyChange;
+
 } }  // namespace v8::internal
 
 #endif  // V8_ASSERT_SCOPE_H_

src/compiler.cc

@@ -120,6 +120,7 @@ void CompilationInfo::Initialize(Isolate* isolate,
     return;
   }
   mode_ = V8::UseCrankshaft() ? mode : NONOPT;
+  abort_due_to_dependency_ = false;
   if (script_->type()->value() == Script::TYPE_NATIVE) {
     MarkAsNative();
   }
@@ -446,6 +447,12 @@ OptimizingCompiler::Status OptimizingCompiler::CreateGraph() {
     }
   }
 
+  if (info()->HasAbortedDueToDependencyChange()) {
+    info_->set_bailout_reason(kBailedOutDueToDependencyChange);
+    info_->AbortOptimization();
+    return SetLastStatus(BAILED_OUT);
+  }
+
   return SetLastStatus(SUCCEEDED);
 }
 
@@ -454,6 +461,7 @@ OptimizingCompiler::Status OptimizingCompiler::OptimizeGraph() {
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
   DisallowHandleDereference no_deref;
+  DisallowCodeDependencyChange no_dependency_change;
 
   ASSERT(last_status() == SUCCEEDED);
   Timer t(this, &time_taken_to_optimize_);
@@ -474,6 +482,8 @@ OptimizingCompiler::Status OptimizingCompiler::OptimizeGraph() {
 
 OptimizingCompiler::Status OptimizingCompiler::GenerateAndInstallCode() {
   ASSERT(last_status() == SUCCEEDED);
+  ASSERT(!info()->HasAbortedDueToDependencyChange());
+  DisallowCodeDependencyChange no_dependency_change;
   {  // Scope for timer.
     Timer timer(this, &time_taken_to_codegen_);
     ASSERT(chunk_ != NULL);
@@ -815,6 +825,7 @@ static bool InstallFullCode(CompilationInfo* info) {
   // was flushed. By setting the code object last we avoid this.
   Handle<SharedFunctionInfo> shared = info->shared_info();
   Handle<Code> code = info->code();
+  CHECK(code->kind() == Code::FUNCTION);
   Handle<JSFunction> function = info->closure();
   Handle<ScopeInfo> scope_info =
       ScopeInfo::Create(info->scope(), info->zone());
@@ -1059,7 +1070,7 @@ void Compiler::InstallOptimizedCode(OptimizingCompiler* optimizing_compiler) {
   // the unoptimized code.
   OptimizingCompiler::Status status = optimizing_compiler->last_status();
   if (info->HasAbortedDueToDependencyChange()) {
-    info->set_bailout_reason(kBailedOutDueToDependentMap);
+    info->set_bailout_reason(kBailedOutDueToDependencyChange);
     status = optimizing_compiler->AbortOptimization();
   } else if (status != OptimizingCompiler::SUCCEEDED) {
     info->set_bailout_reason(kFailedBailedOutLastTime);

src/compiler.h

@@ -298,11 +298,13 @@ class CompilationInfo {
   }
 
   void AbortDueToDependencyChange() {
-    mode_ = DEPENDENCY_CHANGE_ABORT;
+    ASSERT(!isolate()->optimizing_compiler_thread()->IsOptimizerThread());
+    abort_due_to_dependency_ = true;
   }
 
   bool HasAbortedDueToDependencyChange() {
-    return mode_ == DEPENDENCY_CHANGE_ABORT;
+    ASSERT(!isolate()->optimizing_compiler_thread()->IsOptimizerThread());
+    return abort_due_to_dependency_;
   }
 
  protected:
@@ -326,8 +328,7 @@ class CompilationInfo {
     BASE,
     OPTIMIZE,
     NONOPT,
-    STUB,
-    DEPENDENCY_CHANGE_ABORT
+    STUB
   };
 
   void Initialize(Isolate* isolate, Mode mode, Zone* zone);
@@ -401,6 +402,9 @@ class CompilationInfo {
   Mode mode_;
   BailoutId osr_ast_id_;
 
+  // Flag whether compilation needs to be aborted due to dependency change.
+  bool abort_due_to_dependency_;
+
   // The zone from which the compilation pipeline working on this
   // CompilationInfo allocates.
   Zone* zone_;

src/objects.cc

@@ -11354,6 +11354,7 @@ bool DependentCode::Contains(DependencyGroup group, Code* code) {
 void DependentCode::DeoptimizeDependentCodeGroup(
     Isolate* isolate,
     DependentCode::DependencyGroup group) {
+  ASSERT(AllowCodeDependencyChange::IsAllowed());
   DisallowHeapAllocation no_allocation_scope;
   DependentCode::GroupStartIndexes starts(this);
   int start = starts.at(group);

src/objects.h

@@ -1076,7 +1076,7 @@ class MaybeObject BASE_EMBEDDED {
     "bad value context for arguments object value")                           \
   V(kBadValueContextForArgumentsValue,                                        \
     "bad value context for arguments value")                                  \
-  V(kBailedOutDueToDependentMap, "bailed out due to dependent map")           \
+  V(kBailedOutDueToDependencyChange, "bailed out due to dependency change")   \
   V(kBailoutWasNotPrepared, "bailout was not prepared")                       \
   V(kBinaryStubGenerateFloatingPointCode,                                     \
     "BinaryStub_GenerateFloatingPointCode")                                   \

test/mjsunit/regress/regress-map-invalidation-1.js

@@ -0,0 +1,48 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+var c = { x: 2, y: 1 };
+
+function h() {
+  %MigrateInstance(c);
+  return 2;
+}
+%NeverOptimizeFunction(h);
+
+function f() {
+  for (var i = 0; i < 100000; i++) {
+    var n = c.x + h();
+    assertEquals(4, n);
+  }
+  var o2 = [{ x: 2.5, y:1 }];
+  return o2;
+}
+
+f();
+

test/mjsunit/regress/regress-map-invalidation-2.js

@@ -0,0 +1,49 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+var c = { x: 2, y: 1 };
+
+function g() {
+  var outer = { foo: 1 };
+  function f() {
+    var n = outer.foo;
+    for (var i = 0; i < 100000; i++) {
+      n += c.x + outer.foo;
+    }
+    var o2 = [{ x: 1.5, y: 1 }];
+    return o2;
+  }
+  return f;
+}
+
+var fun = g();
+fun();
+assertOptimized(fun);
+fun();
+