AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Fix a bug in delete for lookup slots.

Browse Source 
The function Runtime_LookupContext searches the context chain for a
LOOKUP slot and returns the object holding the slot.  It returned the
global context if the slot was not found or if it was found in a
function's context or arguments object.  This is not the correct
object to use for 'delete'.

Since this lookup function is only ever used when deleting LOOKUP
slots (those that have to go through a with or a scope with eval), it
is simply replaced with a Runtime_DeleteContextSlot function that does
the appropriate thing for all kinds of context lookups.

This fixes Chromium bug 70066.
http://code.google.com/p/chromium/issues/detail?id=70066

Review URL: http://codereview.chromium.org/6280013

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6442 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
kmillikin@chromium.org 2011-01-24 14:03:30 +00:00
parent 811e778592
commit 9c2d52eb0e
9 changed files with 188 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/arm/codegen-arm.cc
View File

@ -5849,14 +5849,10 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
frame_->EmitPush(r0);
} else if (slot != NULL && slot->type() == Slot::LOOKUP) {
// lookup the context holding the named variable
// Delete from the context holding the named variable.
frame_->EmitPush(cp);
frame_->EmitPush(Operand(variable->name()));
frame_->CallRuntime(Runtime::kLookupContext, 2);
// r0: context
frame_->EmitPush(r0);
frame_->EmitPush(Operand(variable->name()));
frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 2);
frame_->CallRuntime(Runtime::kDeleteContextSlot, 2);
frame_->EmitPush(r0);
} else {

12
src/arm/full-codegen-arm.cc
View File

@ -2992,22 +2992,20 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
if (prop != NULL) {
VisitForStackValue(prop->obj());
VisitForStackValue(prop->key());
__ InvokeBuiltin(Builtins::DELETE, CALL_JS);
} else if (var->is_global()) {
__ ldr(r1, GlobalObjectOperand());
__ mov(r0, Operand(var->name()));
__ Push(r1, r0);
__ InvokeBuiltin(Builtins::DELETE, CALL_JS);
} else {
// Non-global variable. Call the runtime to look up the context
// where the variable was introduced.
// Non-global variable. Call the runtime to delete from the
// context where the variable was introduced.
__ push(context_register());
__ mov(r2, Operand(var->name()));
__ push(r2);
__ CallRuntime(Runtime::kLookupContext, 2);
__ push(r0);
__ mov(r2, Operand(var->name()));
__ push(r2);
__ CallRuntime(Runtime::kDeleteContextSlot, 2);
}
__ InvokeBuiltin(Builtins::DELETE, CALL_JS);
context()->Plug(r0);
}
break;

10
src/ia32/codegen-ia32.cc
View File

@ -8230,19 +8230,13 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
return;
} else if (slot != NULL && slot->type() == Slot::LOOKUP) {
// Call the runtime to look up the context holding the named
// Call the runtime to delete from the context holding the named
// variable. Sync the virtual frame eagerly so we can push the
// arguments directly into place.
frame_->SyncRange(0, frame_->element_count() - 1);
frame_->EmitPush(esi);
frame_->EmitPush(Immediate(variable->name()));
Result context = frame_->CallRuntime(Runtime::kLookupContext, 2);
ASSERT(context.is_register());
frame_->EmitPush(context.reg());
context.Unuse();
frame_->EmitPush(Immediate(variable->name()));
Result answer = frame_->InvokeBuiltin(Builtins::DELETE,
CALL_FUNCTION, 2);
Result answer = frame_->CallRuntime(Runtime::kDeleteContextSlot, 2);
frame_->Push(&answer);
return;
}

11
src/ia32/full-codegen-ia32.cc
View File

@ -3689,19 +3689,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
if (prop != NULL) {
VisitForStackValue(prop->obj());
VisitForStackValue(prop->key());
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
} else if (var->is_global()) {
__ push(GlobalObjectOperand());
__ push(Immediate(var->name()));
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
} else {
// Non-global variable. Call the runtime to look up the context
// where the variable was introduced.
// Non-global variable. Call the runtime to delete from the
// context where the variable was introduced.
__ push(context_register());
__ push(Immediate(var->name()));
__ CallRuntime(Runtime::kLookupContext, 2);
__ push(eax);
__ push(Immediate(var->name()));
__ CallRuntime(Runtime::kDeleteContextSlot, 2);
}
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
context()->Plug(eax);
}
break;

37
src/runtime.cc
View File

@ -7049,7 +7049,7 @@ static MaybeObject* Runtime_PushCatchContext(Arguments args) {
}
static MaybeObject* Runtime_LookupContext(Arguments args) {
static MaybeObject* Runtime_DeleteContextSlot(Arguments args) {
HandleScope scope;
ASSERT(args.length() == 2);
@ -7059,16 +7059,31 @@ static MaybeObject* Runtime_LookupContext(Arguments args) {
int index;
PropertyAttributes attributes;
ContextLookupFlags flags = FOLLOW_CHAINS;
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes);
Handle<Object> holder = context->Lookup(name, flags, &index, &attributes);
if (index < 0 && !holder.is_null()) {
ASSERT(holder->IsJSObject());
return *holder;
// If the slot was not found the result is true.
if (holder.is_null()) {
return Heap::true_value();
}
// No intermediate context found. Use global object by default.
return Top::context()->global();
// If the slot was found in a context, it should be DONT_DELETE.
if (holder->IsContext()) {
return Heap::false_value();
}
// The slot was found in a JSObject, either a context extension object,
// the global object, or an arguments object. Try to delete it
// (respecting DONT_DELETE). For consistency with V8's usual behavior,
// which allows deleting all parameters in functions that mention
// 'arguments', we do this even for the case of slots found on an
// arguments object. The slot was found on an arguments object if the
// index is non-negative.
Handle<JSObject> object = Handle<JSObject>::cast(holder);
if (index >= 0) {
return object->DeleteElement(index, JSObject::NORMAL_DELETION);
} else {
return object->DeleteProperty(*name, JSObject::NORMAL_DELETION);
}
}
@ -7141,8 +7156,7 @@ static ObjectPair LoadContextSlotHelper(Arguments args, bool throw_error) {
int index;
PropertyAttributes attributes;
ContextLookupFlags flags = FOLLOW_CHAINS;
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes);
Handle<Object> holder = context->Lookup(name, flags, &index, &attributes);
// If the index is non-negative, the slot has been found in a local
// variable or a parameter. Read it from the context object or the
@ -7209,8 +7223,7 @@ static MaybeObject* Runtime_StoreContextSlot(Arguments args) {
int index;
PropertyAttributes attributes;
ContextLookupFlags flags = FOLLOW_CHAINS;
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes);
Handle<Object> holder = context->Lookup(name, flags, &index, &attributes);
if (index >= 0) {
if (holder->IsContext()) {

2
src/runtime.h
View File

@ -284,7 +284,7 @@ namespace internal {
F(NewContext, 1, 1) \
F(PushContext, 1, 1) \
F(PushCatchContext, 1, 1) \
F(LookupContext, 2, 1) \
F(DeleteContextSlot, 2, 1) \
F(LoadContextSlot, 2, 2) \
F(LoadContextSlotNoReferenceError, 2, 2) \
F(StoreContextSlot, 3, 1) \

10
src/x64/codegen-x64.cc
View File

@ -7235,19 +7235,13 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
return;
} else if (slot != NULL && slot->type() == Slot::LOOKUP) {
// Call the runtime to look up the context holding the named
// Call the runtime to delete from the context holding the named
// variable. Sync the virtual frame eagerly so we can push the
// arguments directly into place.
frame_->SyncRange(0, frame_->element_count() - 1);
frame_->EmitPush(rsi);
frame_->EmitPush(variable->name());
Result context = frame_->CallRuntime(Runtime::kLookupContext, 2);
ASSERT(context.is_register());
frame_->EmitPush(context.reg());
context.Unuse();
frame_->EmitPush(variable->name());
Result answer = frame_->InvokeBuiltin(Builtins::DELETE,
CALL_FUNCTION, 2);
Result answer = frame_->CallRuntime(Runtime::kDeleteContextSlot, 2);
frame_->Push(&answer);
return;
}

11
src/x64/full-codegen-x64.cc
View File

@ -3006,19 +3006,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
if (prop != NULL) {
VisitForStackValue(prop->obj());
VisitForStackValue(prop->key());
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
} else if (var->is_global()) {
__ push(GlobalObjectOperand());
__ Push(var->name());
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
} else {
// Non-global variable. Call the runtime to look up the context
// where the variable was introduced.
// Non-global variable. Call the runtime to delete from the
// context where the variable was introduced.
__ push(context_register());
__ Push(var->name());
__ CallRuntime(Runtime::kLookupContext, 2);
__ push(rax);
__ Push(var->name());
__ CallRuntime(Runtime::kDeleteContextSlot, 2);
}
__ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
context()->Plug(rax);
}
break;

141
test/mjsunit/regress/regress-70066.js Normal file
View File

@ -0,0 +1,141 @@
// Copyright 2011 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.
// Regression test for Chromium issue 70066. Delete should work properly
// from inside 'with' scopes.
// http://code.google.com/p/chromium/issues/detail?id=70066
x = 0;
// Delete on a slot from a function's own context.
function test1() {
var value = 1;
var status;
with ({}) { status = delete value; }
return value + ":" + status;
}
assertEquals("1:false", test1(), "test1");
assertEquals(0, x, "test1"); // Global x is undisturbed.
// Delete on a slot from an outer context.
function test2() {
function f() {
with ({}) { return delete value; }
}
var value = 2;
var status = f();
return value + ":" + status;
}
assertEquals("2:false", test2(), "test2");
assertEquals(0, x, "test2"); // Global x is undisturbed.
// Delete on an argument (should be the same code paths as test1 and test2).
function test3(value) {
var status;
with ({}) { status = delete value; }
return value + ":" + status;
}
assertEquals("3:false", test3(3), "test3");
assertEquals(0, x, "test3"); // Global x is undisturbed.
function test4(value) {
function f() {
with ({}) { return delete value; }
}
var status = f();
return value + ":" + status;
}
assertEquals("4:false", test4(4), "test4");
assertEquals(0, x, "test4"); // Global x is undisturbed.
// Delete on an argument found in the arguments object. Such properties are
// normally DONT_DELETE in JavaScript but deletion is allowed by V8.
function test5(value) {
var status;
with ({}) { status = delete value; }
return arguments[0] + ":" + status;
}
assertEquals("undefined:true", test5(5), "test5");
assertEquals(0, x, "test5"); // Global x is undisturbed.
function test6(value) {
function f() {
with ({}) { return delete value; }
}
var status = f();
return arguments[0] + ":" + status;
}
assertEquals("undefined:true", test6(6), "test6");
assertEquals(0, x, "test6"); // Global x is undisturbed.
// Delete on a property found on 'with' object.
function test7(object) {
with (object) { return delete value; }
}
var o = {value: 7};
assertEquals(true, test7(o), "test7");
assertEquals(void 0, o.value, "test7");
assertEquals(0, x, "test7"); // Global x is undisturbed.
// Delete on a global property.
function test8() {
with ({}) { return delete x; }
}
assertEquals(true, test8(), "test8");
assertThrows("x", "test8"); // Global x should be deleted.
// Delete on a property that is not found anywhere.
function test9() {
with ({}) { return delete x; }
}
assertThrows("x", "test9"); // Make sure it's not there.
assertEquals(true, test9(), "test9");
// Delete on a DONT_DELETE property of the global object.
var y = 10;
function test10() {
with ({}) { return delete y; }
}
assertEquals(false, test10(), "test10");
assertEquals(10, y, "test10");