[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 de4aeb9..e201134 100644 (file)
--- 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, &not_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(&not_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 b62d374..d8b1532 100644 (file)
--- 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, &not_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(&not_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 a596848..944206b 100644 (file)
--- 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, &not_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(&not_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 ae4f3de..cb6d67f 100644 (file)
--- 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(&not_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(&not_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 20cae95..fdbae1f 100644 (file)
--- 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(&not_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(&not_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 8095de9..c6d613b 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -15647,6 +15647,8 @@ Handle<PropertyCell> 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<GlobalDictionary> 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<GlobalDictionary> 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<PropertyCell> cell,
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
+
 }  // namespace internal
 }  // namespace v8

diff --git a/src/property-details.h b/src/property-details.h
index 791eb52..5d95f05 100644 (file)
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -320,6 +320,8 @@ class PropertyDetails BASE_EMBEDDED {
   class KindField : public BitField<PropertyKind, 0, 1> {};
   class LocationField : public BitField<PropertyLocation, 1, 1> {};
   class AttributesField : public BitField<PropertyAttributes, 2, 3> {};
+  static const int kAttributesReadOnlyMask =
+      (READ_ONLY << AttributesField::kShift);
 
   // Bit fields for normalized objects.
   class PropertyCellTypeField : public BitField<PropertyCellType, 5, 2> {};

diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc
index ee60725..ebaf006 100644 (file)
--- a/src/runtime/runtime-object.cc
+++ b/src/runtime/runtime-object.cc
@@ -467,7 +467,7 @@ Object* StoreGlobalViaContext(Isolate* isolate, int slot, Handle<Name> 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<Object>()->IsJSGlobalObject()) {
     // Now update cell in the script context.
     Handle<PropertyCell> cell = it.GetPropertyCell();

diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index 6baea7f..d7b7f40 100644 (file)
--- 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, &not_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(&not_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 24e5336..fd235f9 100644 (file)
--- 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();