From: bmeurer Date: Mon, 27 Jul 2015 18:45:26 +0000 (-0700) Subject: [stubs] Properly handle read-only properties in StoreGlobalViaContextStub. X-Git-Tag: upstream/4.7.83~1159 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cac64b9f634743f7f5311d4dca8d50157b10fab5;p=platform%2Fupstream%2Fv8.git [stubs] Properly handle read-only properties in StoreGlobalViaContextStub. We don't need the hole check and slow runtime mode for read-only properties this way. R=ishell@chromium.org Review URL: https://codereview.chromium.org/1255133002 Cr-Commit-Position: refs/heads/master@{#29881} --- diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index de4aeb96e..e2011341c 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -5120,17 +5120,13 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ add(cell, context, Operand(slot, LSL, kPointerSizeLog2)); __ ldr(cell, ContextOperand(cell)); - // Check that cell value is not the_hole. - __ ldr(cell_value, FieldMemOperand(cell, PropertyCell::kValueOffset)); - __ CompareRoot(cell_value, Heap::kTheHoleValueRootIndex); - __ b(eq, &slow_case); - // Load PropertyDetails for the cell (actually only the cell_type and kind). __ ldr(cell_details, FieldMemOperand(cell, PropertyCell::kDetailsOffset)); __ SmiUntag(cell_details); __ and_(cell_details, cell_details, Operand(PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask)); + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask)); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5154,9 +5150,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Check if PropertyCell value matches the new value (relevant for Constant, // ConstantType and Undefined cells). Label not_same_value; + __ ldr(cell_value, FieldMemOperand(cell, PropertyCell::kValueOffset)); __ cmp(cell_value, value); __ b(ne, ¬_same_value); + // Make sure the PropertyCell is not marked READ_ONLY. + __ tst(cell_details, Operand(PropertyDetails::kAttributesReadOnlyMask)); + __ b(ne, &slow_case); + if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5178,7 +5179,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ cmp(cell_details, Operand(PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | PropertyDetails::KindField::encode(kData))); diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index b62d374b7..d8b153240 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -5553,16 +5553,13 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Add(cell, context, Operand(slot, LSL, kPointerSizeLog2)); __ Ldr(cell, ContextMemOperand(cell)); - // Check that cell value is not the_hole. - __ Ldr(cell_value, FieldMemOperand(cell, PropertyCell::kValueOffset)); - __ JumpIfRoot(cell_value, Heap::kTheHoleValueRootIndex, &slow_case); - // Load PropertyDetails for the cell (actually only the cell_type and kind). __ Ldr(cell_details, UntagSmiFieldMemOperand(cell, PropertyCell::kDetailsOffset)); __ And(cell_details, cell_details, PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask); + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5585,9 +5582,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Check if PropertyCell value matches the new value (relevant for Constant, // ConstantType and Undefined cells). Label not_same_value; + __ Ldr(cell_value, FieldMemOperand(cell, PropertyCell::kValueOffset)); __ Cmp(cell_value, value); __ B(ne, ¬_same_value); + // Make sure the PropertyCell is not marked READ_ONLY. + __ Tst(cell_details, PropertyDetails::kAttributesReadOnlyMask); + __ B(ne, &slow_case); + if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5609,7 +5611,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ Bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ Cmp(cell_details, PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | PropertyDetails::KindField::encode(kData)); diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index a59684851..944206b8a 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -5173,23 +5173,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Load the PropertyCell at the specified slot. __ mov(cell_reg, ContextOperand(context_reg, slot_reg)); - // Check that cell value is not the_hole. - { - // TODO(bmeurer): use ecx (name_reg) when name parameter is removed. - Register cell_value_reg = cell_details_reg; - __ mov(cell_value_reg, FieldOperand(cell_reg, PropertyCell::kValueOffset)); - __ CompareRoot(cell_value_reg, Heap::kTheHoleValueRootIndex); - __ j(equal, &slow_case, FLAG_debug_code ? Label::kFar : Label::kNear); - } - // Load PropertyDetails for the cell (actually only the cell_type and kind). __ mov(cell_details_reg, FieldOperand(cell_reg, PropertyCell::kDetailsOffset)); __ SmiUntag(cell_details_reg); __ and_(cell_details_reg, Immediate(PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask)); - + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask)); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5215,6 +5206,10 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ cmp(value_reg, FieldOperand(cell_reg, PropertyCell::kValueOffset)); __ j(not_equal, ¬_same_value, FLAG_debug_code ? Label::kFar : Label::kNear); + // Make sure the PropertyCell is not marked READ_ONLY. + __ test(cell_details_reg, + Immediate(PropertyDetails::kAttributesReadOnlyMask)); + __ j(not_zero, &slow_case); if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5239,7 +5234,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ cmp(cell_details_reg, Immediate(PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index ae4f3de57..cb6d67f3b 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -5331,18 +5331,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Addu(at, at, Context::SlotOffset(0)); __ lw(cell_reg, MemOperand(at)); - // Check that cell value is not the_hole. - __ lw(cell_value_reg, FieldMemOperand(cell_reg, PropertyCell::kValueOffset)); - __ LoadRoot(at, Heap::kTheHoleValueRootIndex); - __ Branch(&slow_case, eq, cell_value_reg, Operand(at)); - // Load PropertyDetails for the cell (actually only the cell_type and kind). __ lw(cell_details_reg, FieldMemOperand(cell_reg, PropertyCell::kDetailsOffset)); __ SmiUntag(cell_details_reg); __ And(cell_details_reg, cell_details_reg, PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask); + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5364,7 +5360,11 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Check if PropertyCell value matches the new value (relevant for Constant, // ConstantType and Undefined cells). Label not_same_value; + __ lw(cell_value_reg, FieldMemOperand(cell_reg, PropertyCell::kValueOffset)); __ Branch(¬_same_value, ne, value_reg, Operand(cell_value_reg)); + // Make sure the PropertyCell is not marked READ_ONLY. + __ And(at, cell_details_reg, PropertyDetails::kAttributesReadOnlyMask); + __ Branch(&slow_case, ne, at, Operand(zero_reg)); if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5386,7 +5386,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ Branch(&slow_case, ne, cell_details_reg, Operand(PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 20cae9554..fdbae1f66 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -5362,18 +5362,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Daddu(at, at, Context::SlotOffset(0)); __ ld(cell_reg, MemOperand(at)); - // Check that cell value is not the_hole. - __ ld(cell_value_reg, FieldMemOperand(cell_reg, PropertyCell::kValueOffset)); - __ LoadRoot(at, Heap::kTheHoleValueRootIndex); - __ Branch(&slow_case, eq, cell_value_reg, Operand(at)); - // Load PropertyDetails for the cell (actually only the cell_type and kind). __ ld(cell_details_reg, FieldMemOperand(cell_reg, PropertyCell::kDetailsOffset)); __ SmiUntag(cell_details_reg); __ And(cell_details_reg, cell_details_reg, PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask); + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5395,7 +5391,11 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Check if PropertyCell value matches the new value (relevant for Constant, // ConstantType and Undefined cells). Label not_same_value; + __ ld(cell_value_reg, FieldMemOperand(cell_reg, PropertyCell::kValueOffset)); __ Branch(¬_same_value, ne, value_reg, Operand(cell_value_reg)); + // Make sure the PropertyCell is not marked READ_ONLY. + __ And(at, cell_details_reg, PropertyDetails::kAttributesReadOnlyMask); + __ Branch(&slow_case, ne, at, Operand(zero_reg)); if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5417,7 +5417,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ Branch(&slow_case, ne, cell_details_reg, Operand(PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | diff --git a/src/objects.cc b/src/objects.cc index 8095de9df..c6d613b2c 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -15647,6 +15647,8 @@ Handle PropertyCell::InvalidateEntry( } else { cell->set_value(isolate->heap()->the_hole_value()); } + details = details.set_cell_type(PropertyCellType::kInvalidated); + cell->set_property_details(details); cell->dependent_code()->DeoptimizeDependentCodeGroup( isolate, DependentCode::kPropertyCellChangedGroup); return new_cell; @@ -15718,9 +15720,7 @@ void PropertyCell::UpdateCell(Handle dictionary, int entry, const PropertyDetails original_details = cell->property_details(); // Data accesses could be cached in ics or optimized code. bool invalidate = - (original_details.kind() == kData && details.kind() == kAccessor) || - ((original_details.attributes() & READ_ONLY) != - (details.attributes() & READ_ONLY)); + original_details.kind() == kData && details.kind() == kAccessor; int index = original_details.dictionary_index(); PropertyCellType old_type = original_details.cell_type(); // Preserve the enumeration index unless the property was deleted or never @@ -15743,8 +15743,9 @@ void PropertyCell::UpdateCell(Handle dictionary, int entry, cell->set_value(*value); // Deopt when transitioning from a constant type. - if (!invalidate && (old_type != new_type)) { - auto isolate = dictionary->GetIsolate(); + if (!invalidate && (old_type != new_type || + original_details.IsReadOnly() != details.IsReadOnly())) { + Isolate* isolate = dictionary->GetIsolate(); cell->dependent_code()->DeoptimizeDependentCodeGroup( isolate, DependentCode::kPropertyCellChangedGroup); } @@ -15761,5 +15762,6 @@ void PropertyCell::SetValueWithInvalidation(Handle cell, isolate, DependentCode::kPropertyCellChangedGroup); } } + } // namespace internal } // namespace v8 diff --git a/src/property-details.h b/src/property-details.h index 791eb524c..5d95f05e8 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -320,6 +320,8 @@ class PropertyDetails BASE_EMBEDDED { class KindField : public BitField {}; class LocationField : public BitField {}; class AttributesField : public BitField {}; + static const int kAttributesReadOnlyMask = + (READ_ONLY << AttributesField::kShift); // Bit fields for normalized objects. class PropertyCellTypeField : public BitField {}; diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index ee607259b..ebaf006a4 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -467,7 +467,7 @@ Object* StoreGlobalViaContext(Isolate* isolate, int slot, Handle name, LookupIterator it(global_object, name, LookupIterator::HIDDEN); // Switch to fast mode only if there is a data property and it's not on // a hidden prototype. - if (LookupIterator::DATA == it.state() && !it.IsReadOnly() && + if (LookupIterator::DATA == it.state() && it.GetHolder()->IsJSGlobalObject()) { // Now update cell in the script context. Handle cell = it.GetPropertyCell(); diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 6baea7fd7..d7b7f400f 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -5091,18 +5091,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Load the PropertyCell at the specified slot. __ movp(cell_reg, ContextOperand(context_reg, slot_reg)); - // Check that value is not the_hole. - __ movp(cell_value_reg, FieldOperand(cell_reg, PropertyCell::kValueOffset)); - __ CompareRoot(cell_value_reg, Heap::kTheHoleValueRootIndex); - __ j(equal, &slow_case, FLAG_debug_code ? Label::kFar : Label::kNear); - - // Load PropertyDetails for the cell (actually only the cell_type and kind). + // Load PropertyDetails for the cell (actually only the cell_type, kind and + // READ_ONLY bit of attributes). __ SmiToInteger32(cell_details_reg, FieldOperand(cell_reg, PropertyCell::kDetailsOffset)); __ andl(cell_details_reg, Immediate(PropertyDetails::PropertyCellTypeField::kMask | - PropertyDetails::KindField::kMask)); - + PropertyDetails::KindField::kMask | + PropertyDetails::kAttributesReadOnlyMask)); // Check if PropertyCell holds mutable data. Label not_mutable_data; @@ -5125,9 +5121,14 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { // Check if PropertyCell value matches the new value (relevant for Constant, // ConstantType and Undefined cells). Label not_same_value; + __ movp(cell_value_reg, FieldOperand(cell_reg, PropertyCell::kValueOffset)); __ cmpp(cell_value_reg, value_reg); __ j(not_equal, ¬_same_value, FLAG_debug_code ? Label::kFar : Label::kNear); + // Make sure the PropertyCell is not marked READ_ONLY. + __ testl(cell_details_reg, + Immediate(PropertyDetails::kAttributesReadOnlyMask)); + __ j(not_zero, &slow_case); if (FLAG_debug_code) { Label done; // This can only be true for Constant, ConstantType and Undefined cells, @@ -5152,7 +5153,8 @@ void StoreGlobalViaContextStub::Generate(MacroAssembler* masm) { __ Ret(); __ bind(¬_same_value); - // Check if PropertyCell contains data with constant type. + // Check if PropertyCell contains data with constant type (and is not + // READ_ONLY). __ cmpl(cell_details_reg, Immediate(PropertyDetails::PropertyCellTypeField::encode( PropertyCellType::kConstantType) | diff --git a/test/mjsunit/regress/cross-script-vars.js b/test/mjsunit/regress/cross-script-vars.js index 24e53368c..fd235f997 100644 --- a/test/mjsunit/regress/cross-script-vars.js +++ b/test/mjsunit/regress/cross-script-vars.js @@ -452,6 +452,8 @@ function testSuite(opt_cfg) { assertThrows('StoreVar(113)'); assertThrows('StoreVar(113)'); assertEquals(42, LoadVar()); + assertThrows('StoreVar(42)'); + assertEquals(42, LoadVar()); assertThrows('LoadStoreLoop()'); assertEquals(42, LoadVar()); TearDown();