From 9adaeb6a175f8d32419aa0e9fa6a9f38b1bb7048 Mon Sep 17 00:00:00 2001 From: "mmaly@chromium.org" Date: Mon, 14 Feb 2011 23:41:47 +0000 Subject: [PATCH] Strict mode delete of non-configurable property. Strict mode flag is passed to runtime DELETE function and then to JSObject::Delete(Property/Element) as STRICT_DELETION enum. When deleting non-configurable property/eleemnt, TypeError is thrown. Adding mozilla test to .gitignore. Incorporate CR feedback. Review URL: http://codereview.chromium.org/6515005/ git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6782 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- .gitignore | 1 + src/arm/codegen-arm.cc | 9 +++++++-- src/arm/full-codegen-arm.cc | 12 +++++++++--- src/arm/lithium-codegen-arm.cc | 4 +++- src/arm/lithium-codegen-arm.h | 4 ++++ src/builtins.h | 2 +- src/ia32/codegen-ia32.cc | 9 +++++++-- src/ia32/full-codegen-ia32.cc | 5 +++++ src/ia32/lithium-codegen-ia32.cc | 1 + src/ia32/lithium-codegen-ia32.h | 4 ++++ src/messages.js | 1 + src/objects.cc | 23 ++++++++++++++++++++--- src/objects.h | 7 ++++++- src/runtime.cc | 7 +++++-- src/runtime.h | 2 +- src/runtime.js | 4 ++-- src/x64/codegen-x64.cc | 9 +++++++-- src/x64/full-codegen-x64.cc | 5 +++++ test/es5conform/es5conform.status | 10 +++------- test/mjsunit/strict-mode.js | 32 +++++++++++++++++++++++++++++++- 20 files changed, 123 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 2eab942..db57d1b 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ shell shell_g /obj/ /test/es5conform/data/ +/test/mozilla/data/ /test/sputnik/sputniktests/ /tools/oom_dump/oom_dump /tools/oom_dump/oom_dump.o diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index c827110..a3921d8 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -5844,15 +5844,20 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { if (property != NULL) { Load(property->obj()); Load(property->key()); - frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 2); + frame_->EmitPush(Operand(Smi::FromInt(strict_mode_flag()))); + frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 3); frame_->EmitPush(r0); } else if (variable != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); Slot* slot = variable->AsSlot(); if (variable->is_global()) { LoadGlobal(); frame_->EmitPush(Operand(variable->name())); - frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 2); + frame_->EmitPush(Operand(Smi::FromInt(kNonStrictMode))); + frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 3); frame_->EmitPush(r0); } else if (slot != NULL && slot->type() == Slot::LOOKUP) { diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 801296b..29a1971 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -3062,14 +3062,20 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { } else { VisitForStackValue(prop->obj()); VisitForStackValue(prop->key()); + __ mov(r1, Operand(Smi::FromInt(strict_mode_flag()))); + __ push(r1); __ InvokeBuiltin(Builtins::DELETE, CALL_JS); context()->Plug(r0); } } else if (var != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); if (var->is_global()) { - __ ldr(r1, GlobalObjectOperand()); - __ mov(r0, Operand(var->name())); - __ Push(r1, r0); + __ ldr(r2, GlobalObjectOperand()); + __ mov(r1, Operand(var->name())); + __ mov(r0, Operand(Smi::FromInt(kNonStrictMode))); + __ Push(r2, r1, r0); __ InvokeBuiltin(Builtins::DELETE, CALL_JS); context()->Plug(r0); } else if (var->AsSlot() != NULL && diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 057ac24..4a30d7c 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -3888,7 +3888,9 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) { void LCodeGen::DoDeleteProperty(LDeleteProperty* instr) { Register object = ToRegister(instr->object()); Register key = ToRegister(instr->key()); - __ Push(object, key); + Register strict = scratch0(); + __ mov(strict, Operand(Smi::FromInt(strict_mode_flag()))); + __ Push(object, key, strict); ASSERT(instr->HasPointerMap() && instr->HasDeoptimizationEnvironment()); LPointerMap* pointers = instr->pointer_map(); LEnvironment* env = instr->deoptimization_environment(); diff --git a/src/arm/lithium-codegen-arm.h b/src/arm/lithium-codegen-arm.h index 7bc6689..732db44 100644 --- a/src/arm/lithium-codegen-arm.h +++ b/src/arm/lithium-codegen-arm.h @@ -129,6 +129,10 @@ class LCodeGen BASE_EMBEDDED { bool is_done() const { return status_ == DONE; } bool is_aborted() const { return status_ == ABORTED; } + int strict_mode_flag() const { + return info_->is_strict() ? kStrictMode : kNonStrictMode; + } + LChunk* chunk() const { return chunk_; } Scope* scope() const { return scope_; } HGraph* graph() const { return chunk_->graph(); } diff --git a/src/builtins.h b/src/builtins.h index ada23a7..2733410 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -214,7 +214,7 @@ enum BuiltinExtraArguments { V(SHL, 1) \ V(SAR, 1) \ V(SHR, 1) \ - V(DELETE, 1) \ + V(DELETE, 2) \ V(IN, 1) \ V(INSTANCE_OF, 1) \ V(GET_KEYS, 0) \ diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index b977db8..02e2919 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -8225,19 +8225,24 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { if (property != NULL) { Load(property->obj()); Load(property->key()); - Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 2); + frame_->Push(Smi::FromInt(strict_mode_flag())); + Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 3); frame_->Push(&answer); return; } Variable* variable = node->expression()->AsVariableProxy()->AsVariable(); if (variable != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); Slot* slot = variable->AsSlot(); if (variable->is_global()) { LoadGlobal(); frame_->Push(variable->name()); + frame_->Push(Smi::FromInt(kNonStrictMode)); Result answer = frame_->InvokeBuiltin(Builtins::DELETE, - CALL_FUNCTION, 2); + CALL_FUNCTION, 3); frame_->Push(&answer); return; diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index ada7b3d..8427983 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -3730,13 +3730,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { } else { VisitForStackValue(prop->obj()); VisitForStackValue(prop->key()); + __ push(Immediate(Smi::FromInt(strict_mode_flag()))); __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION); context()->Plug(eax); } } else if (var != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); if (var->is_global()) { __ push(GlobalObjectOperand()); __ push(Immediate(var->name())); + __ push(Immediate(Smi::FromInt(kNonStrictMode))); __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION); context()->Plug(eax); } else if (var->AsSlot() != NULL && diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 7d5de29..a25d678 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -3735,6 +3735,7 @@ void LCodeGen::DoDeleteProperty(LDeleteProperty* instr) { pointers, env->deoptimization_index()); __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset)); + __ push(Immediate(Smi::FromInt(strict_mode_flag()))); __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, &safepoint_generator); } diff --git a/src/ia32/lithium-codegen-ia32.h b/src/ia32/lithium-codegen-ia32.h index 3ac3a41..977fbcd 100644 --- a/src/ia32/lithium-codegen-ia32.h +++ b/src/ia32/lithium-codegen-ia32.h @@ -120,6 +120,10 @@ class LCodeGen BASE_EMBEDDED { bool is_done() const { return status_ == DONE; } bool is_aborted() const { return status_ == ABORTED; } + int strict_mode_flag() const { + return info_->is_strict() ? kStrictMode : kNonStrictMode; + } + LChunk* chunk() const { return chunk_; } Scope* scope() const { return scope_; } HGraph* graph() const { return chunk_->graph(); } diff --git a/src/messages.js b/src/messages.js index ac7962c..b7e57aa 100644 --- a/src/messages.js +++ b/src/messages.js @@ -225,6 +225,7 @@ function FormatMessage(message) { strict_lhs_prefix: ["Prefix increment/decrement may not have eval or arguments operand in strict mode"], strict_reserved_word: ["Use of future reserved word in strict mode"], strict_delete: ["Delete of an unqualified identifier in strict mode."], + strict_delete_property: ["Cannot delete property '", "%0", "' of ", "%1"], }; } var message_type = %MessageGetType(message); diff --git a/src/objects.cc b/src/objects.cc index 5003b4f..31566ff 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1399,7 +1399,7 @@ MaybeObject* JSObject::AddProperty(String* name, if (!map()->is_extensible()) { Handle args[1] = {Handle(name)}; return Top::Throw(*Factory::NewTypeError("object_not_extensible", - HandleVector(args, 1))); + HandleVector(args, 1))); } if (HasFastProperties()) { // Ensure the descriptor array does not get too big. @@ -2620,7 +2620,17 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) { NumberDictionary* dictionary = element_dictionary(); int entry = dictionary->FindEntry(index); if (entry != NumberDictionary::kNotFound) { - return dictionary->DeleteProperty(entry, mode); + Object* result = dictionary->DeleteProperty(entry, mode); + if (mode == STRICT_DELETION && result == Heap::false_value()) { + // In strict mode, deleting a non-configurable property throws + // exception. dictionary->DeleteProperty will return false_value() + // if a non-configurable property is being deleted. + HandleScope scope; + Handle i = Factory::NewNumberFromUint(index); + Handle args[2] = { i, Handle(this) }; + return Top::Throw(*Factory::NewTypeError("strict_delete_property", + HandleVector(args, 2))); + } } break; } @@ -2659,6 +2669,13 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) { if (!result.IsProperty()) return Heap::true_value(); // Ignore attributes if forcing a deletion. if (result.IsDontDelete() && mode != FORCE_DELETION) { + if (mode == STRICT_DELETION) { + // Deleting a non-configurable property in strict mode. + HandleScope scope; + Handle args[2] = { Handle(name), Handle(this) }; + return Top::Throw(*Factory::NewTypeError("strict_delete_property", + HandleVector(args, 2))); + } return Heap::false_value(); } // Check for interceptor. @@ -9345,7 +9362,7 @@ Object* Dictionary::DeleteProperty(int entry, JSObject::DeleteMode mode) { PropertyDetails details = DetailsAt(entry); // Ignore attributes if forcing a deletion. - if (details.IsDontDelete() && mode == JSObject::NORMAL_DELETION) { + if (details.IsDontDelete() && mode != JSObject::FORCE_DELETION) { return Heap::false_value(); } SetEntry(entry, Heap::null_value(), Heap::null_value(), Smi::FromInt(0)); diff --git a/src/objects.h b/src/objects.h index 264cc0b..559ada6 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1286,7 +1286,12 @@ class HeapNumber: public HeapObject { // caching. class JSObject: public HeapObject { public: - enum DeleteMode { NORMAL_DELETION, FORCE_DELETION }; + enum DeleteMode { + NORMAL_DELETION, + STRICT_DELETION, + FORCE_DELETION + }; + enum ElementsKind { // The only "fast" kind. FAST_ELEMENTS, diff --git a/src/runtime.cc b/src/runtime.cc index 60e5e38..b83164e 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -3914,11 +3914,14 @@ static MaybeObject* Runtime_IgnoreAttributesAndSetProperty(Arguments args) { static MaybeObject* Runtime_DeleteProperty(Arguments args) { NoHandleAllocation ha; - ASSERT(args.length() == 2); + ASSERT(args.length() == 3); CONVERT_CHECKED(JSObject, object, args[0]); CONVERT_CHECKED(String, key, args[1]); - return object->DeleteProperty(key, JSObject::NORMAL_DELETION); + CONVERT_SMI_CHECKED(strict, args[2]); + return object->DeleteProperty(key, strict == kStrictMode + ? JSObject::STRICT_DELETION + : JSObject::NORMAL_DELETION); } diff --git a/src/runtime.h b/src/runtime.h index 8551196..06437ef 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -45,7 +45,7 @@ namespace internal { /* Property access */ \ F(GetProperty, 2, 1) \ F(KeyedGetProperty, 2, 1) \ - F(DeleteProperty, 2, 1) \ + F(DeleteProperty, 3, 1) \ F(HasLocalProperty, 2, 1) \ F(HasProperty, 2, 1) \ F(HasElement, 2, 1) \ diff --git a/src/runtime.js b/src/runtime.js index 2cdbbde..66d839b 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -338,8 +338,8 @@ function SHR(y) { */ // ECMA-262, section 11.4.1, page 46. -function DELETE(key) { - return %DeleteProperty(%ToObject(this), %ToString(key)); +function DELETE(key, strict) { + return %DeleteProperty(%ToObject(this), %ToString(key), strict); } diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index fe90567..150ed66 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -7230,19 +7230,24 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { if (property != NULL) { Load(property->obj()); Load(property->key()); - Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 2); + frame_->Push(Smi::FromInt(strict_mode_flag())); + Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 3); frame_->Push(&answer); return; } Variable* variable = node->expression()->AsVariableProxy()->AsVariable(); if (variable != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); Slot* slot = variable->AsSlot(); if (variable->is_global()) { LoadGlobal(); frame_->Push(variable->name()); + frame_->Push(Smi::FromInt(kNonStrictMode)); Result answer = frame_->InvokeBuiltin(Builtins::DELETE, - CALL_FUNCTION, 2); + CALL_FUNCTION, 3); frame_->Push(&answer); return; diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 24a8524..a28bcb7 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -3069,13 +3069,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { } else { VisitForStackValue(prop->obj()); VisitForStackValue(prop->key()); + __ Push(Smi::FromInt(strict_mode_flag())); __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION); context()->Plug(rax); } } else if (var != NULL) { + // Delete of an unqualified identifier is disallowed in strict mode + // so this code can only be reached in non-strict mode. + ASSERT(strict_mode_flag() == kNonStrictMode); if (var->is_global()) { __ push(GlobalObjectOperand()); __ Push(var->name()); + __ Push(Smi::FromInt(kNonStrictMode)); __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION); context()->Plug(rax); } else if (var->AsSlot() != NULL && diff --git a/test/es5conform/es5conform.status b/test/es5conform/es5conform.status index 4cf0118..d7b10a8 100644 --- a/test/es5conform/es5conform.status +++ b/test/es5conform/es5conform.status @@ -347,15 +347,11 @@ chapter11/11.13/11.13.1/11.13.1-4-26-s: FAIL # in strict mode (Global.undefined) chapter11/11.13/11.13.1/11.13.1-4-27-s: FAIL -# delete operator throws TypeError when deleting a non-configurable data -# property in strict mode -chapter11/11.4/11.4.1/11.4.1-4.a-3-s: FAIL # delete operator throws TypeError when when deleting a non-configurable # data property in strict mode (Global.NaN) -chapter11/11.4/11.4.1/11.4.1-4.a-4-s: FAIL -# delete operator throws TypeError when deleting a non-configurable data -# property in strict mode (Math.LN2) -chapter11/11.4/11.4.1/11.4.1-4.a-9-s: FAIL +# Invalid test case - "this" is not a global object within the test case. +# (http://es5conform.codeplex.com/workitem/29151) +chapter11/11.4/11.4.1/11.4.1-4.a-4-s: FAIL_OK # delete operator throws ReferenceError when deleting a direct reference # to a var in strict mode diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js index c6e6ca4..fead64a 100644 --- a/test/mjsunit/strict-mode.js +++ b/test/mjsunit/strict-mode.js @@ -280,7 +280,7 @@ CheckStrictMode("function strict() { print(--arguments); }", SyntaxError); CheckStrictMode("function strict() { var x = --eval; }", SyntaxError); CheckStrictMode("function strict() { var x = --arguments; }", SyntaxError); -// Delete of an unqialified identifier +// Delete of an unqualified identifier CheckStrictMode("delete unqualified;", SyntaxError); CheckStrictMode("function strict() { delete unqualified; }", SyntaxError); CheckStrictMode("function function_name() { delete function_name; }", @@ -406,3 +406,33 @@ delete possibly_undefined_variable_for_strict_mode_test; repeat(10, function() { testAssignToUndefined(true); }); possibly_undefined_variable_for_strict_mode_test = undefined; repeat(10, function() { testAssignToUndefined(false); }); + +(function testDeleteNonConfigurable() { + function delete_property(o) { + "use strict"; + delete o.property; + } + function delete_element(o, i) { + "use strict"; + delete o[i]; + } + + var object = {}; + + Object.defineProperty(object, "property", { value: "property_value" }); + Object.defineProperty(object, "1", { value: "one" }); + Object.defineProperty(object, 7, { value: "seven" }); + Object.defineProperty(object, 3.14, { value: "pi" }); + + assertThrows(function() { delete_property(object); }, TypeError); + assertEquals(object.property, "property_value"); + assertThrows(function() { delete_element(object, "1"); }, TypeError); + assertThrows(function() { delete_element(object, 1); }, TypeError); + assertEquals(object[1], "one"); + assertThrows(function() { delete_element(object, "7"); }, TypeError); + assertThrows(function() { delete_element(object, 7); }, TypeError); + assertEquals(object[7], "seven"); + assertThrows(function() { delete_element(object, "3.14"); }, TypeError); + assertThrows(function() { delete_element(object, 3.14); }, TypeError); + assertEquals(object[3.14], "pi"); +})(); -- 2.7.4