Fix bug with filtering of foreign context maps in the type feedback.
The first attempt did not properly handle keyed loads/stores and did not check the constructors of the objects in the prototype chain. Added two more tests to handle the fixed cases. BUG=v8:1823 TEST=LeakGlobalObjectViaMapKeyed,LeakGlobalContextViaMapProto Review URL: http://codereview.chromium.org/8974009 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10277 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
parent
240e50d6a3
commit
6c0a4f5d45
@ -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> 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<Map> TypeFeedbackOracle::LoadMonomorphicReceiverType(Property* expr) {
|
||||
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<Map> TypeFeedbackOracle::StoreMonomorphicReceiverType(Expression* expr) {
|
||||
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);
|
||||
}
|
||||
@ -288,7 +297,11 @@ Handle<Map> TypeFeedbackOracle::GetCompareMap(CompareOperation* expr) {
|
||||
if (state != CompareIC::KNOWN_OBJECTS) {
|
||||
return Handle<Map>::null();
|
||||
}
|
||||
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);
|
||||
}
|
||||
|
||||
|
||||
@ -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()) {
|
||||
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->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();
|
||||
constructor = map->constructor();
|
||||
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>(Map::cast(object)), types);
|
||||
Map* map = Map::cast(object);
|
||||
if (!CanRetainOtherContext(map, *global_context_)) {
|
||||
AddMapIfMissing(Handle<Map>(map), types);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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 ]
|
||||
|
||||
|
@ -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<v8::Context> ctx1 = v8::Context::New();
|
||||
v8::Persistent<v8::Context> ctx2 = v8::Context::New();
|
||||
@ -1349,7 +1350,8 @@ TEST(LeakGlobalContextViaMap) {
|
||||
ctx2->Global()->Set(v8_str("o"), v);
|
||||
v8::Local<v8::Value> 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<v8::Context> ctx1 = v8::Context::New();
|
||||
v8::Persistent<v8::Context> ctx2 = v8::Context::New();
|
||||
@ -1384,7 +1387,8 @@ TEST(LeakGlobalContextViaFunction) {
|
||||
ctx2->Global()->Set(v8_str("o"), v);
|
||||
v8::Local<v8::Value> 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<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());
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user