diff --git a/src/type-info.cc b/src/type-info.cc index af8a8ae828..790d77cb19 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -85,7 +85,8 @@ bool TypeFeedbackOracle::LoadIsMonomorphicNormal(Property* expr) { return code->is_keyed_load_stub() && code->ic_state() == MONOMORPHIC && Code::ExtractTypeFromFlags(code->flags()) == NORMAL && - code->FindFirstMap() != NULL; + code->FindFirstMap() != NULL && + !CanRetainOtherContext(code->FindFirstMap(), *global_context_); } return false; } @@ -111,7 +112,9 @@ bool TypeFeedbackOracle::StoreIsMonomorphicNormal(Expression* expr) { Handle code = Handle::cast(map_or_code); return code->is_keyed_store_stub() && code->ic_state() == MONOMORPHIC && - Code::ExtractTypeFromFlags(code->flags()) == NORMAL; + Code::ExtractTypeFromFlags(code->flags()) == NORMAL && + code->FindFirstMap() != NULL && + !CanRetainOtherContext(code->FindFirstMap(), *global_context_); } return false; } @@ -144,7 +147,9 @@ Handle TypeFeedbackOracle::LoadMonomorphicReceiverType(Property* expr) { Handle code = Handle::cast(map_or_code); Map* first_map = code->FindFirstMap(); ASSERT(first_map != NULL); - return Handle(first_map); + return CanRetainOtherContext(first_map, *global_context_) + ? Handle::null() + : Handle(first_map); } return Handle::cast(map_or_code); } @@ -155,7 +160,11 @@ Handle TypeFeedbackOracle::StoreMonomorphicReceiverType(Expression* expr) { Handle map_or_code = GetInfo(expr->id()); if (map_or_code->IsCode()) { Handle code = Handle::cast(map_or_code); - return Handle(code->FindFirstMap()); + Map* first_map = code->FindFirstMap(); + ASSERT(first_map != NULL); + return CanRetainOtherContext(first_map, *global_context_) + ? Handle::null() + : Handle(first_map); } return Handle::cast(map_or_code); } @@ -288,7 +297,11 @@ Handle TypeFeedbackOracle::GetCompareMap(CompareOperation* expr) { if (state != CompareIC::KNOWN_OBJECTS) { return Handle::null(); } - return Handle(code->FindFirstMap()); + Map* first_map = code->FindFirstMap(); + ASSERT(first_map != NULL); + return CanRetainOtherContext(first_map, *global_context_) + ? Handle::null() + : Handle(first_map); } @@ -451,20 +464,23 @@ void TypeFeedbackOracle::CollectReceiverTypes(unsigned ast_id, // retaining objects from different tabs in Chrome via optimized code. bool TypeFeedbackOracle::CanRetainOtherContext(Map* map, Context* global_context) { - Object* constructor = map->constructor(); - ASSERT(constructor != NULL); - while (!constructor->IsJSFunction()) { - // If the constructor is not null or a JSFunction, we have to - // conservatively assume that it may retain a global context. - if (!constructor->IsNull()) return true; - - // If both, constructor and prototype are null, we conclude - // that no global context will be retained by this map. - if (map->prototype()->IsNull()) return false; - - map = JSObject::cast(map->prototype())->map(); + Object* constructor = NULL; + while (!map->prototype()->IsNull()) { constructor = map->constructor(); + if (!constructor->IsNull()) { + // If the constructor is not null or a JSFunction, we have to + // conservatively assume that it may retain a global context. + if (!constructor->IsJSFunction()) return true; + // Check if the constructor directly references a foreign context. + if (CanRetainOtherContext(JSFunction::cast(constructor), + global_context)) { + return true; + } + } + map = HeapObject::cast(map->prototype())->map(); } + constructor = map->constructor(); + if (constructor->IsNull()) return false; JSFunction* function = JSFunction::cast(constructor); return CanRetainOtherContext(function, global_context); } @@ -498,7 +514,10 @@ void TypeFeedbackOracle::CollectKeyedReceiverTypes(unsigned ast_id, RelocInfo* info = it.rinfo(); Object* object = info->target_object(); if (object->IsMap()) { - AddMapIfMissing(Handle(Map::cast(object)), types); + Map* map = Map::cast(object); + if (!CanRetainOtherContext(map, *global_context_)) { + AddMapIfMissing(Handle(map), types); + } } } } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 6d36aa6740..2de0afba17 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -52,10 +52,6 @@ test-profile-generator/RecordStackTraceAtStartProfiling: PASS || FAIL # We do not yet shrink weak maps after they have been emptied by the GC test-weakmaps/Shrinking: FAIL -# TODO(1823): Fails without snapshot. Temporarily disabled until fixed. -test-heap/LeakGlobalContextViaMap: SKIP -test-heap/LeakGlobalContextViaFunction: SKIP - ############################################################################## [ $arch == arm ] diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index 0e09ee38e1..42b5789d4d 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -1333,6 +1333,7 @@ static int NumberOfGlobalObjects() { // Test that we don't embed maps from foreign contexts into // optimized code. TEST(LeakGlobalContextViaMap) { + i::FLAG_allow_natives_syntax = true; v8::HandleScope outer_scope; v8::Persistent ctx1 = v8::Context::New(); v8::Persistent ctx2 = v8::Context::New(); @@ -1349,7 +1350,8 @@ TEST(LeakGlobalContextViaMap) { ctx2->Global()->Set(v8_str("o"), v); v8::Local res = CompileRun( "function f() { return o.x; }" - "for (var i = 0; i < 1000000; ++i) f();" + "for (var i = 0; i < 10; ++i) f();" + "%OptimizeFunctionOnNextCall(f);" "f();"); CHECK_EQ(42, res->Int32Value()); ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0)); @@ -1368,6 +1370,7 @@ TEST(LeakGlobalContextViaMap) { // Test that we don't embed functions from foreign contexts into // optimized code. TEST(LeakGlobalContextViaFunction) { + i::FLAG_allow_natives_syntax = true; v8::HandleScope outer_scope; v8::Persistent ctx1 = v8::Context::New(); v8::Persistent ctx2 = v8::Context::New(); @@ -1384,7 +1387,8 @@ TEST(LeakGlobalContextViaFunction) { ctx2->Global()->Set(v8_str("o"), v); v8::Local res = CompileRun( "function f(x) { return x(); }" - "for (var i = 0; i < 1000000; ++i) f(o);" + "for (var i = 0; i < 10; ++i) f(o);" + "%OptimizeFunctionOnNextCall(f);" "f(o);"); CHECK_EQ(42, res->Int32Value()); ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0)); @@ -1398,3 +1402,77 @@ TEST(LeakGlobalContextViaFunction) { HEAP->CollectAllAvailableGarbage(); CHECK_EQ(0, NumberOfGlobalObjects()); } + + +TEST(LeakGlobalContextViaMapKeyed) { + i::FLAG_allow_natives_syntax = true; + v8::HandleScope outer_scope; + v8::Persistent ctx1 = v8::Context::New(); + v8::Persistent ctx2 = v8::Context::New(); + ctx1->Enter(); + + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(4, NumberOfGlobalObjects()); + + { + v8::HandleScope inner_scope; + CompileRun("var v = [42, 43]"); + v8::Local v = ctx1->Global()->Get(v8_str("v")); + ctx2->Enter(); + ctx2->Global()->Set(v8_str("o"), v); + v8::Local res = CompileRun( + "function f() { return o[0]; }" + "for (var i = 0; i < 10; ++i) f();" + "%OptimizeFunctionOnNextCall(f);" + "f();"); + CHECK_EQ(42, res->Int32Value()); + ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0)); + ctx2->Exit(); + ctx1->Exit(); + ctx1.Dispose(); + } + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(2, NumberOfGlobalObjects()); + ctx2.Dispose(); + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(0, NumberOfGlobalObjects()); +} + + +TEST(LeakGlobalContextViaMapProto) { + i::FLAG_allow_natives_syntax = true; + v8::HandleScope outer_scope; + v8::Persistent ctx1 = v8::Context::New(); + v8::Persistent ctx2 = v8::Context::New(); + ctx1->Enter(); + + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(4, NumberOfGlobalObjects()); + + { + v8::HandleScope inner_scope; + CompileRun("var v = { y: 42}"); + v8::Local v = ctx1->Global()->Get(v8_str("v")); + ctx2->Enter(); + ctx2->Global()->Set(v8_str("o"), v); + v8::Local res = CompileRun( + "function f() {" + " var p = {x: 42};" + " p.__proto__ = o;" + " return p.x;" + "}" + "for (var i = 0; i < 10; ++i) f();" + "%OptimizeFunctionOnNextCall(f);" + "f();"); + CHECK_EQ(42, res->Int32Value()); + ctx2->Global()->Set(v8_str("o"), v8::Int32::New(0)); + ctx2->Exit(); + ctx1->Exit(); + ctx1.Dispose(); + } + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(2, NumberOfGlobalObjects()); + ctx2.Dispose(); + HEAP->CollectAllAvailableGarbage(); + CHECK_EQ(0, NumberOfGlobalObjects()); +}