Fixed global object leak caused by overwriting the global receiver (the global proxy) in the global object with the global object itself.

This CL additionally removes the API function to reattach a global proxy to a
global object.

BUG=324812
LOG=y
R=dcarney@chromium.org, titzer@chromium.org

Review URL: https://chromiumcodereview.appspot.com/101733002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18299 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
verwaest@chromium.org 2013-12-11 13:51:48 +00:00
parent cc401095fb
commit d5787278bc
12 changed files with 113 additions and 116 deletions

View File

@ -5150,20 +5150,16 @@ class V8_EXPORT ExtensionConfiguration {
class V8_EXPORT Context {
public:
/**
* Returns the global proxy object or global object itself for
* detached contexts.
* Returns the global proxy object.
*
* Global proxy object is a thin wrapper whose prototype points to
* actual context's global object with the properties like Object, etc.
* This is done that way for security reasons (for more details see
* Global proxy object is a thin wrapper whose prototype points to actual
* context's global object with the properties like Object, etc. This is done
* that way for security reasons (for more details see
* https://wiki.mozilla.org/Gecko:SplitWindow).
*
* Please note that changes to global proxy object prototype most probably
* would break VM---v8 expects only global object as a prototype of
* global proxy object.
*
* If DetachGlobal() has been invoked, Global() would return actual global
* object until global is reattached with ReattachGlobal().
* would break VM---v8 expects only global object as a prototype of global
* proxy object.
*/
Local<Object> Global();
@ -5173,18 +5169,6 @@ class V8_EXPORT Context {
*/
void DetachGlobal();
/**
* Reattaches a global object to a context. This can be used to
* restore the connection between a global object and a context
* after DetachGlobal has been called.
*
* \param global_object The global object to reattach to the
* context. For this to work, the global object must be the global
* object that was associated with this context before a call to
* DetachGlobal.
*/
void ReattachGlobal(Handle<Object> global_object);
/**
* Creates a new context and returns a handle to the newly allocated
* context.

View File

@ -5409,6 +5409,12 @@ v8::Local<v8::Object> Context::Global() {
i::Handle<i::Context> context = Utils::OpenHandle(this);
i::Isolate* isolate = context->GetIsolate();
i::Handle<i::Object> global(context->global_proxy(), isolate);
// TODO(dcarney): This should always return the global proxy
// but can't presently as calls to GetProtoype will return the wrong result.
if (i::Handle<i::JSGlobalProxy>::cast(
global)->IsDetachedFrom(context->global_object())) {
global = i::Handle<i::Object>(context->global_object(), isolate);
}
return Utils::ToLocal(i::Handle<i::JSObject>::cast(global));
}
@ -5421,16 +5427,6 @@ void Context::DetachGlobal() {
}
void Context::ReattachGlobal(Handle<Object> global_object) {
i::Handle<i::Context> context = Utils::OpenHandle(this);
i::Isolate* isolate = context->GetIsolate();
ENTER_V8(isolate);
i::Handle<i::JSGlobalProxy> global_proxy =
i::Handle<i::JSGlobalProxy>::cast(Utils::OpenHandle(*global_object));
isolate->bootstrapper()->ReattachGlobal(context, global_proxy);
}
void Context::AllowCodeGenerationFromStrings(bool allow) {
i::Handle<i::Context> context = Utils::OpenHandle(this);
i::Isolate* isolate = context->GetIsolate();

View File

@ -341,17 +341,6 @@ void Bootstrapper::DetachGlobal(Handle<Context> env) {
Handle<JSGlobalProxy> global_proxy(JSGlobalProxy::cast(env->global_proxy()));
global_proxy->set_native_context(*factory->null_value());
SetObjectPrototype(global_proxy, factory->null_value());
env->set_global_proxy(env->global_object());
env->global_object()->set_global_receiver(env->global_object());
}
void Bootstrapper::ReattachGlobal(Handle<Context> env,
Handle<JSGlobalProxy> global_proxy) {
env->global_object()->set_global_receiver(*global_proxy);
env->set_global_proxy(*global_proxy);
SetObjectPrototype(global_proxy, Handle<JSObject>(env->global_object()));
global_proxy->set_native_context(*env);
}

View File

@ -105,9 +105,6 @@ class Bootstrapper {
// Detach the environment from its outer global object.
void DetachGlobal(Handle<Context> env);
// Reattach an outer global object to an environment.
void ReattachGlobal(Handle<Context> env, Handle<JSGlobalProxy> global_proxy);
// Traverses the pointers for memory management.
void Iterate(ObjectVisitor* v);

View File

@ -5891,16 +5891,13 @@ PropertyAttributes JSReceiver::GetElementAttribute(uint32_t index) {
}
// TODO(504): this may be useful in other places too where JSGlobalProxy
// is used.
Object* JSObject::BypassGlobalProxy() {
if (IsJSGlobalProxy()) {
Object* proto = GetPrototype();
if (proto->IsNull()) return GetHeap()->undefined_value();
ASSERT(proto->IsJSGlobalObject());
return proto;
}
return this;
bool JSGlobalObject::IsDetached() {
return JSGlobalProxy::cast(global_receiver())->IsDetachedFrom(this);
}
bool JSGlobalProxy::IsDetachedFrom(GlobalObject* global) {
return GetPrototype() != global;
}

View File

@ -878,6 +878,7 @@ class DictionaryElementsAccessor;
class ElementsAccessor;
class Failure;
class FixedArrayBase;
class GlobalObject;
class ObjectVisitor;
class StringStream;
class Type;
@ -7412,6 +7413,8 @@ class JSGlobalProxy : public JSObject {
// Casting.
static inline JSGlobalProxy* cast(Object* obj);
inline bool IsDetachedFrom(GlobalObject* global);
// Dispatched behavior.
DECLARE_PRINTER(JSGlobalProxy)
DECLARE_VERIFIER(JSGlobalProxy)
@ -7481,6 +7484,8 @@ class JSGlobalObject: public GlobalObject {
static Handle<PropertyCell> EnsurePropertyCell(Handle<JSGlobalObject> global,
Handle<Name> name);
inline bool IsDetached();
// Dispatched behavior.
DECLARE_PRINTER(JSGlobalObject)
DECLARE_VERIFIER(JSGlobalObject)

View File

@ -9624,6 +9624,16 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GlobalReceiver) {
}
RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAttachedGlobal) {
SealHandleScope shs(isolate);
ASSERT(args.length() == 1);
Object* global = args[0];
if (!global->IsJSGlobalObject()) return isolate->heap()->false_value();
return isolate->heap()->ToBoolean(
!JSGlobalObject::cast(global)->IsDetached());
}
RUNTIME_FUNCTION(MaybeObject*, Runtime_ParseJson) {
HandleScope scope(isolate);
ASSERT_EQ(1, args.length());

View File

@ -275,6 +275,7 @@ namespace internal {
\
/* Eval */ \
F(GlobalReceiver, 1, 1) \
F(IsAttachedGlobal, 1, 1) \
F(ResolvePossiblyDirectEval, 5, 2) \
\
F(SetProperty, -1 /* 4 or 5 */, 1) \

View File

@ -170,19 +170,18 @@ function GlobalParseFloat(string) {
function GlobalEval(x) {
if (!IS_STRING(x)) return x;
var global_receiver = %GlobalReceiver(global);
var global_is_detached = (global === global_receiver);
// For consistency with JSC we require the global object passed to
// eval to be the global object from which 'eval' originated. This
// is not mandated by the spec.
// We only throw if the global has been detached, since we need the
// receiver as this-value for the call.
if (global_is_detached) {
if (!%IsAttachedGlobal(global)) {
throw new $EvalError('The "this" value passed to eval must ' +
'be the global object from which eval originated');
}
var global_receiver = %GlobalReceiver(global);
var f = %CompileString(x, false);
if (!IS_FUNCTION(f)) return f;

View File

@ -8527,8 +8527,6 @@ TEST(ContextDetachGlobal) {
// Detach env2's global, and reuse the global object of env2
env2->Exit();
env2->DetachGlobal();
// env2 has a new global object.
CHECK(!env2->Global()->Equals(global2));
v8::Handle<Context> env3 = Context::New(env1->GetIsolate(),
0,
@ -8563,7 +8561,7 @@ TEST(ContextDetachGlobal) {
}
TEST(DetachAndReattachGlobal) {
TEST(DetachGlobal) {
LocalContext env1;
v8::HandleScope scope(env1->GetIsolate());
@ -8628,16 +8626,58 @@ TEST(DetachAndReattachGlobal) {
// so access should be blocked.
result = CompileRun("other.p");
CHECK(result->IsUndefined());
}
// Detach the global for env3 and reattach it to env2.
env3->DetachGlobal();
env2->ReattachGlobal(global2);
// Check that we have access to other.p again in env1. |other| is now
// the global object for env2 which has the same security token as env1.
result = CompileRun("other.p");
CHECK(result->IsInt32());
CHECK_EQ(42, result->Int32Value());
TEST(DetachedAccesses) {
LocalContext env1;
v8::HandleScope scope(env1->GetIsolate());
// Create second environment.
v8::Handle<Context> env2 = Context::New(env1->GetIsolate());
Local<Value> foo = v8_str("foo");
// Set same security token for env1 and env2.
env1->SetSecurityToken(foo);
env2->SetSecurityToken(foo);
{
v8::Context::Scope scope(env2);
CompileRun(
"var x = 'x';"
"function get_x() { return this.x; }"
"function get_x_w() { return get_x(); }"
"");
env1->Global()->Set(v8_str("get_x"), CompileRun("get_x"));
env1->Global()->Set(v8_str("get_x_w"), CompileRun("get_x_w"));
}
Local<Object> env2_global = env2->Global();
env2_global->TurnOnAccessCheck();
env2->DetachGlobal();
Local<Value> result;
result = CompileRun("get_x()");
CHECK(result->IsUndefined());
result = CompileRun("get_x_w()");
CHECK(result->IsUndefined());
// Reattach env2's proxy
env2 = Context::New(env1->GetIsolate(),
0,
v8::Handle<v8::ObjectTemplate>(),
env2_global);
env2->SetSecurityToken(foo);
{
v8::Context::Scope scope(env2);
CompileRun("var x = 'x2';");
}
result = CompileRun("get_x()");
CHECK(result->IsUndefined());
result = CompileRun("get_x_w()");
CHECK_EQ(v8_str("x2"), result);
}
@ -14251,8 +14291,10 @@ THREADED_TEST(TurnOnAccessCheck) {
}
// Detach the global and turn on access check.
Local<Object> hidden_global = Local<Object>::Cast(
context->Global()->GetPrototype());
context->DetachGlobal();
context->Global()->TurnOnAccessCheck();
hidden_global->TurnOnAccessCheck();
// Failing access check to property get results in undefined.
CHECK(f1->Call(global, 0, NULL)->IsUndefined());
@ -14336,8 +14378,10 @@ THREADED_TEST(TurnOnAccessCheckAndRecompile) {
// Detach the global and turn on access check now blocking access to property
// a and function h.
Local<Object> hidden_global = Local<Object>::Cast(
context->Global()->GetPrototype());
context->DetachGlobal();
context->Global()->TurnOnAccessCheck();
hidden_global->TurnOnAccessCheck();
// Failing access check to property get results in undefined.
CHECK(f1->Call(global, 0, NULL)->IsUndefined());
@ -14353,11 +14397,11 @@ THREADED_TEST(TurnOnAccessCheckAndRecompile) {
// Now compile the source again. And get the newly compiled functions, except
// for h for which access is blocked.
CompileRun(source);
f1 = Local<Function>::Cast(context->Global()->Get(v8_str("f1")));
f2 = Local<Function>::Cast(context->Global()->Get(v8_str("f2")));
g1 = Local<Function>::Cast(context->Global()->Get(v8_str("g1")));
g2 = Local<Function>::Cast(context->Global()->Get(v8_str("g2")));
CHECK(context->Global()->Get(v8_str("h"))->IsUndefined());
f1 = Local<Function>::Cast(hidden_global->Get(v8_str("f1")));
f2 = Local<Function>::Cast(hidden_global->Get(v8_str("f2")));
g1 = Local<Function>::Cast(hidden_global->Get(v8_str("g1")));
g2 = Local<Function>::Cast(hidden_global->Get(v8_str("g2")));
CHECK(hidden_global->Get(v8_str("h"))->IsUndefined());
// Failing access check to property get results in undefined.
CHECK(f1->Call(global, 0, NULL)->IsUndefined());
@ -15224,23 +15268,6 @@ THREADED_TEST(ReplaceConstantFunction) {
}
// Regression test for http://crbug.com/16276.
THREADED_TEST(Regress16276) {
LocalContext context;
v8::HandleScope scope(context->GetIsolate());
// Force the IC in f to be a dictionary load IC.
CompileRun("function f(obj) { return obj.x; }\n"
"var obj = { x: { foo: 42 }, y: 87 };\n"
"var x = obj.x;\n"
"delete obj.y;\n"
"for (var i = 0; i < 5; i++) f(obj);");
// Detach the global object to make 'this' refer directly to the
// global object (not the proxy), and make sure that the dictionary
// load IC doesn't mess up loading directly from the global object.
context->DetachGlobal();
CHECK_EQ(42, CompileRun("f(this).foo")->Int32Value());
}
static void CheckElementValue(i::Isolate* isolate,
int expected,
i::Handle<i::Object> obj,

View File

@ -648,9 +648,9 @@ class ExistsInHiddenPrototypeContext: public DeclarationContext {
virtual void PostInitializeContext(Handle<Context> context) {
Local<Object> global_object = context->Global();
Local<Object> hidden_proto = hidden_proto_->GetFunction()->NewInstance();
context->DetachGlobal();
context->Global()->SetPrototype(hidden_proto);
context->ReattachGlobal(global_object);
Local<Object> inner_global =
Local<Object>::Cast(global_object->GetPrototype());
inner_global->SetPrototype(hidden_proto);
}
// Use the hidden prototype as the holder for the interceptors.

View File

@ -242,7 +242,6 @@ TEST(GlobalObjectObservation) {
LocalContext context(isolate.GetIsolate());
HandleScope scope(isolate.GetIsolate());
Handle<Object> global_proxy = context->Global();
Handle<Object> inner_global = global_proxy->GetPrototype().As<Object>();
CompileRun(
"var records = [];"
"var global = this;"
@ -255,33 +254,26 @@ TEST(GlobalObjectObservation) {
context->DetachGlobal();
CompileRun("global.bar = 'goodbye';");
CHECK_EQ(1, CompileRun("records.length")->Int32Value());
// Mutating the global object directly still has an effect...
CompileRun("this.bar = 'goodbye';");
CHECK_EQ(2, CompileRun("records.length")->Int32Value());
CHECK(inner_global->StrictEquals(CompileRun("records[1].object")));
// Reattached, back to global proxy.
context->ReattachGlobal(global_proxy);
CompileRun("global.baz = 'again';");
CHECK_EQ(3, CompileRun("records.length")->Int32Value());
CHECK(global_proxy->StrictEquals(CompileRun("records[2].object")));
CompileRun("this.baz = 'goodbye';");
CHECK_EQ(1, CompileRun("records.length")->Int32Value());
// Attached to a different context, should not leak mutations
// to the old context.
context->DetachGlobal();
{
LocalContext context2(isolate.GetIsolate());
context2->DetachGlobal();
context2->ReattachGlobal(global_proxy);
CompileRun(
"var records2 = [];"
"var global = this;"
"Object.observe(this, function(r) { [].push.apply(records2, r) });"
"this.bat = 'context2';");
"this.v1 = 'context2';");
context2->DetachGlobal();
CompileRun(
"global.v2 = 'context2';"
"this.v3 = 'context2';");
CHECK_EQ(1, CompileRun("records2.length")->Int32Value());
CHECK(global_proxy->StrictEquals(CompileRun("records2[0].object")));
}
CHECK_EQ(3, CompileRun("records.length")->Int32Value());
CHECK_EQ(1, CompileRun("records.length")->Int32Value());
// Attaching by passing to Context::New
{
@ -295,7 +287,7 @@ TEST(GlobalObjectObservation) {
CHECK_EQ(1, CompileRun("records3.length")->Int32Value());
CHECK(global_proxy->StrictEquals(CompileRun("records3[0].object")));
}
CHECK_EQ(3, CompileRun("records.length")->Int32Value());
CHECK_EQ(1, CompileRun("records.length")->Int32Value());
}