Revision: 10317
Author: [email protected]
Date: Mon Jan 2 01:46:56 2012
Log: Merge r10257 and r10277 to the 3.7 branch: Fix leak of global
objects from optimized code.
Original CLs:
http://codereview.chromium.org/8892002/
http://codereview.chromium.org/8974009
BUG=v8:1823, 102895
Review URL: http://codereview.chromium.org/8974013
http://code.google.com/p/v8/source/detail?r=10317
Modified:
/branches/3.7/src/stub-cache.cc
/branches/3.7/src/stub-cache.h
/branches/3.7/src/type-info.cc
/branches/3.7/src/type-info.h
/branches/3.7/src/version.cc
/branches/3.7/test/cctest/test-heap.cc
=======================================
--- /branches/3.7/src/stub-cache.cc Mon Nov 7 02:14:12 2011
+++ /branches/3.7/src/stub-cache.cc Mon Jan 2 01:46:56 2012
@@ -877,7 +877,8 @@
void StubCache::CollectMatchingMaps(SmallMapList* types,
String* name,
- Code::Flags flags) {
+ Code::Flags flags,
+ Handle<Context> global_context) {
for (int i = 0; i < kPrimaryTableSize; i++) {
if (primary_[i].key == name) {
Map* map = primary_[i].value->FindFirstMap();
@@ -886,7 +887,8 @@
if (map == NULL) continue;
int offset = PrimaryOffset(name, flags, map);
- if (entry(primary_, offset) == &primary_[i]) {
+ if (entry(primary_, offset) == &primary_[i] &&
+ !TypeFeedbackOracle::CanRetainOtherContext(map,
*global_context)) {
types->Add(Handle<Map>(map));
}
}
@@ -909,7 +911,8 @@
// Lookup in secondary table and add matches.
int offset = SecondaryOffset(name, flags, primary_offset);
- if (entry(secondary_, offset) == &secondary_[i]) {
+ if (entry(secondary_, offset) == &secondary_[i] &&
+ !TypeFeedbackOracle::CanRetainOtherContext(map,
*global_context)) {
types->Add(Handle<Map>(map));
}
}
=======================================
--- /branches/3.7/src/stub-cache.h Mon Nov 7 02:14:12 2011
+++ /branches/3.7/src/stub-cache.h Mon Jan 2 01:46:56 2012
@@ -248,7 +248,8 @@
// Collect all maps that match the name and flags.
void CollectMatchingMaps(SmallMapList* types,
String* name,
- Code::Flags flags);
+ Code::Flags flags,
+ Handle<Context> global_context);
// Generate code for probing the stub cache table.
// Arguments extra and extra2 may be used to pass additional scratch
=======================================
--- /branches/3.7/src/type-info.cc Mon Nov 21 05:51:57 2011
+++ /branches/3.7/src/type-info.cc Mon Jan 2 01:46:56 2012
@@ -85,7 +85,8 @@
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 @@
Handle<Code> code = Handle<Code>::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<Code> code = Handle<Code>::cast(map_or_code);
Map* first_map = code->FindFirstMap();
ASSERT(first_map != NULL);
- return Handle<Map>(first_map);
+ return CanRetainOtherContext(first_map, *global_context_)
+ ? Handle<Map>::null()
+ : Handle<Map>(first_map);
}
return Handle<Map>::cast(map_or_code);
}
@@ -155,7 +160,11 @@
Handle<Object> map_or_code = GetInfo(expr->id());
if (map_or_code->IsCode()) {
Handle<Code> code = Handle<Code>::cast(map_or_code);
- return Handle<Map>(code->FindFirstMap());
+ Map* first_map = code->FindFirstMap();
+ ASSERT(first_map != NULL);
+ return CanRetainOtherContext(first_map, *global_context_)
+ ? Handle<Map>::null()
+ : Handle<Map>(first_map);
}
return Handle<Map>::cast(map_or_code);
}
@@ -423,9 +432,46 @@
Handle<Code>::cast(object)->ic_state() == MEGAMORPHIC) {
types->Reserve(4);
ASSERT(object->IsCode());
- isolate_->stub_cache()->CollectMatchingMaps(types, *name, flags);
+ isolate_->stub_cache()->CollectMatchingMaps(types,
+ *name,
+ flags,
+ global_context_);
}
}
+
+
+// Check if a map originates from a given global context. We use this
+// information to filter out maps from different context to avoid
+// retaining objects from different tabs in Chrome via optimized code.
+bool TypeFeedbackOracle::CanRetainOtherContext(Map* map,
+ Context* global_context) {
+ 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);
+}
+
+
+bool TypeFeedbackOracle::CanRetainOtherContext(JSFunction* function,
+ Context* global_context) {
+ return function->context()->global() != global_context->global()
+ && function->context()->global() != global_context->builtins();
+}
static void AddMapIfMissing(Handle<Map> map, SmallMapList* list) {
@@ -449,7 +495,10 @@
RelocInfo* info = it.rinfo();
Object* object = info->target_object();
if (object->IsMap()) {
- AddMapIfMissing(Handle<Map>(Map::cast(object)), types);
+ Map* map = Map::cast(object);
+ if (!CanRetainOtherContext(map, *global_context_)) {
+ AddMapIfMissing(Handle<Map>(map), types);
+ }
}
}
}
@@ -524,7 +573,12 @@
SetInfo(ast_id, Smi::FromInt(target->check_type()));
} else {
Object* map = target->FindFirstMap();
- SetInfo(ast_id, map == NULL ? static_cast<Object*>(target) :
map);
+ if (map == NULL) {
+ SetInfo(ast_id, static_cast<Object*>(target));
+ } else if (!CanRetainOtherContext(Map::cast(map),
+ *global_context_)) {
+ SetInfo(ast_id, map);
+ }
}
} else if (target->ic_state() == MEGAMORPHIC) {
SetInfo(ast_id, target);
@@ -550,7 +604,9 @@
if (target->major_key() == CodeStub::CallFunction &&
target->has_function_cache()) {
Object* value =
CallFunctionStub::GetCachedValue(reloc_entry.pc());
- if (value->IsJSFunction()) {
+ if (value->IsJSFunction() &&
+ !CanRetainOtherContext(JSFunction::cast(value),
+ *global_context_)) {
SetInfo(ast_id, value);
}
}
=======================================
--- /branches/3.7/src/type-info.h Mon Nov 21 05:51:57 2011
+++ /branches/3.7/src/type-info.h Mon Jan 2 01:46:56 2012
@@ -256,6 +256,10 @@
void CollectKeyedReceiverTypes(unsigned ast_id,
SmallMapList* types);
+ static bool CanRetainOtherContext(Map* map, Context* global_context);
+ static bool CanRetainOtherContext(JSFunction* function,
+ Context* global_context);
+
CheckType GetCallCheckType(Call* expr);
Handle<JSObject> GetPrototypeForPrimitiveCheck(CheckType check);
=======================================
--- /branches/3.7/src/version.cc Tue Dec 13 10:08:45 2011
+++ /branches/3.7/src/version.cc Mon Jan 2 01:46:56 2012
@@ -35,7 +35,7 @@
#define MAJOR_VERSION 3
#define MINOR_VERSION 7
#define BUILD_NUMBER 12
-#define PATCH_LEVEL 11
+#define PATCH_LEVEL 12
// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
#define IS_CANDIDATE_VERSION 0
=======================================
--- /branches/3.7/test/cctest/test-heap.cc Mon Dec 5 09:22:52 2011
+++ /branches/3.7/test/cctest/test-heap.cc Mon Jan 2 01:46:56 2012
@@ -1289,3 +1289,161 @@
new_capacity = new_space->Capacity();
CHECK(old_capacity == new_capacity);
}
+
+
+static int NumberOfGlobalObjects() {
+ int count = 0;
+ HeapIterator iterator;
+ for (HeapObject* obj = iterator.next(); obj != NULL; obj =
iterator.next()) {
+ if (obj->IsGlobalObject()) count++;
+ }
+ return count;
+}
+
+
+// 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<v8::Context> ctx1 = v8::Context::New();
+ v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+ ctx1->Enter();
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(4, NumberOfGlobalObjects());
+
+ {
+ v8::HandleScope inner_scope;
+ CompileRun("var v = {x: 42}");
+ v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+ ctx2->Enter();
+ ctx2->Global()->Set(v8_str("o"), v);
+ v8::Local<v8::Value> res = CompileRun(
+ "function f() { return o.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());
+}
+
+
+// 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<v8::Context> ctx1 = v8::Context::New();
+ v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+ ctx1->Enter();
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(4, NumberOfGlobalObjects());
+
+ {
+ v8::HandleScope inner_scope;
+ CompileRun("var v = function() { return 42; }");
+ v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+ ctx2->Enter();
+ ctx2->Global()->Set(v8_str("o"), v);
+ v8::Local<v8::Value> res = CompileRun(
+ "function f(x) { return x(); }"
+ "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));
+ ctx2->Exit();
+ ctx1->Exit();
+ ctx1.Dispose();
+ }
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(2, NumberOfGlobalObjects());
+ ctx2.Dispose();
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(0, NumberOfGlobalObjects());
+}
+
+
+TEST(LeakGlobalContextViaMapKeyed) {
+ i::FLAG_allow_natives_syntax = true;
+ v8::HandleScope outer_scope;
+ v8::Persistent<v8::Context> ctx1 = v8::Context::New();
+ v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+ ctx1->Enter();
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(4, NumberOfGlobalObjects());
+
+ {
+ v8::HandleScope inner_scope;
+ CompileRun("var v = [42, 43]");
+ v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+ ctx2->Enter();
+ ctx2->Global()->Set(v8_str("o"), v);
+ v8::Local<v8::Value> 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<v8::Context> ctx1 = v8::Context::New();
+ v8::Persistent<v8::Context> ctx2 = v8::Context::New();
+ ctx1->Enter();
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(4, NumberOfGlobalObjects());
+
+ {
+ v8::HandleScope inner_scope;
+ CompileRun("var v = { y: 42}");
+ v8::Local<v8::Value> v = ctx1->Global()->Get(v8_str("v"));
+ ctx2->Enter();
+ ctx2->Global()->Set(v8_str("o"), v);
+ v8::Local<v8::Value> 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());
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
