Fix redefining of attributes on aliased arguments.

This allows elements of the non-strict arguments object to be redefined
with custom attributes and still maintain an alias into the context.
Such a slow alias is maintained by placing a special marker into the
dictionary backing store of the arguments object.

R=rossberg@chromium.org
BUG=v8:1772
TEST=test262,mjsunit/object-define-property

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10827 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8.h

@@ -3850,7 +3850,7 @@ class Internals {
   static const int kFullStringRepresentationMask = 0x07;
   static const int kExternalTwoByteRepresentationTag = 0x02;
 
-  static const int kJSObjectType = 0xa8;
+  static const int kJSObjectType = 0xa9;
   static const int kFirstNonstringType = 0x80;
   static const int kForeignType = 0x85;
 

src/elements.cc

@@ -705,10 +705,20 @@ class NonStrictArgumentsElementsAccessor
     } else {
       // Object is not mapped, defer to the arguments.
       FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
-      return ElementsAccessor::ForArray(arguments)->Get(arguments,
-                                                        key,
-                                                        obj,
-                                                        receiver);
+      MaybeObject* maybe_result = ElementsAccessor::ForArray(arguments)->Get(
+          arguments, key, obj, receiver);
+      Object* result;
+      if (!maybe_result->ToObject(&result)) return maybe_result;
+      // Elements of the arguments object in slow mode might be slow aliases.
+      if (result->IsAliasedArgumentsEntry()) {
+        AliasedArgumentsEntry* entry = AliasedArgumentsEntry::cast(result);
+        Context* context = Context::cast(parameter_map->get(0));
+        int context_index = entry->aliased_context_slot();
+        ASSERT(!context->get(context_index)->IsTheHole());
+        return context->get(context_index);
+      } else {
+        return result;
+      }
     }
   }
 

src/heap.cc

@@ -1951,6 +1951,16 @@ MaybeObject* Heap::AllocateTypeFeedbackInfo() {
 }
 
 
+MaybeObject* Heap::AllocateAliasedArgumentsEntry(int aliased_context_slot) {
+  AliasedArgumentsEntry* entry;
+  { MaybeObject* maybe_result = AllocateStruct(ALIASED_ARGUMENTS_ENTRY_TYPE);
+    if (!maybe_result->To(&entry)) return maybe_result;
+  }
+  entry->set_aliased_context_slot(aliased_context_slot);
+  return entry;
+}
+
+
 const Heap::StringTypeTable Heap::string_type_table[] = {
 #define STRING_TYPE_ELEMENT(type, size, name, camel_name)                      \
   {type, size, k##camel_name##MapRootIndex},

src/heap.h

@@ -641,6 +641,9 @@ class Heap {
   // Allocates an empty TypeFeedbackInfo.
   MUST_USE_RESULT MaybeObject* AllocateTypeFeedbackInfo();
 
+  // Allocates an AliasedArgumentsEntry.
+  MUST_USE_RESULT MaybeObject* AllocateAliasedArgumentsEntry(int slot);
+
   // Clear the Instanceof cache (used when a prototype changes).
   inline void ClearInstanceofCache();
 

src/objects-debug.cc

@@ -333,6 +333,11 @@ void TypeFeedbackInfo::TypeFeedbackInfoVerify() {
 }
 
 
+void AliasedArgumentsEntry::AliasedArgumentsEntryVerify() {
+  VerifySmiField(kAliasedContextSlot);
+}
+
+
 void FixedArray::FixedArrayVerify() {
   for (int i = 0; i < length(); i++) {
     Object* e = get(i);

src/objects-inl.h

@@ -4806,6 +4806,9 @@ ACCESSORS(TypeFeedbackInfo, type_feedback_cells, TypeFeedbackCells,
           kTypeFeedbackCellsOffset)
 
 
+SMI_ACCESSORS(AliasedArgumentsEntry, aliased_context_slot, kAliasedContextSlot)
+
+
 Relocatable::Relocatable(Isolate* isolate) {
   ASSERT(isolate == Isolate::Current());
   isolate_ = isolate;

src/objects-printer.cc

@@ -563,6 +563,12 @@ void TypeFeedbackInfo::TypeFeedbackInfoPrint(FILE* out) {
 }
 
 
+void AliasedArgumentsEntry::AliasedArgumentsEntryPrint(FILE* out) {
+  HeapObject::PrintHeader(out, "AliasedArgumentsEntry");
+  PrintF(out, "\n - aliased_context_slot: %d", aliased_context_slot());
+}
+
+
 void FixedArray::FixedArrayPrint(FILE* out) {
   HeapObject::PrintHeader(out, "FixedArray");
   PrintF(out, " - length: %d", length());

src/objects.cc

@@ -9559,19 +9559,31 @@ MaybeObject* JSObject::SetDictionaryElement(uint32_t index,
       // value is being defined we skip attribute checks completely.
       if (set_mode == DEFINE_PROPERTY) {
         details = PropertyDetails(attributes, NORMAL, details.index());
-        dictionary->ValueAtPut(entry, value);
         dictionary->DetailsAtPut(entry, details);
-      } else if (!details.IsReadOnly() || element->IsTheHole()) {
-        dictionary->ValueAtPut(entry, value);
-      } else if (strict_mode == kStrictMode) {
-        Handle<Object> holder(this);
-        Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
-        Handle<Object> args[2] = { number, holder };
-        Handle<Object> error =
-            isolate->factory()->NewTypeError("strict_read_only_property",
-                                             HandleVector(args, 2));
-        return isolate->Throw(*error);
+      } else if (details.IsReadOnly() && !element->IsTheHole()) {
+        if (strict_mode == kNonStrictMode) {
+          return isolate->heap()->undefined_value();
+        } else {
+          Handle<Object> holder(this);
+          Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
+          Handle<Object> args[2] = { number, holder };
+          Handle<Object> error =
+              isolate->factory()->NewTypeError("strict_read_only_property",
+                                               HandleVector(args, 2));
+          return isolate->Throw(*error);
+        }
       }
+      // Elements of the arguments object in slow mode might be slow aliases.
+      if (is_arguments && element->IsAliasedArgumentsEntry()) {
+        AliasedArgumentsEntry* entry = AliasedArgumentsEntry::cast(element);
+        Context* context = Context::cast(elements->get(0));
+        int context_index = entry->aliased_context_slot();
+        ASSERT(!context->get(context_index)->IsTheHole());
+        context->set(context_index, value);
+        // For elements that are still writable we keep slow aliasing.
+        if (!details.IsReadOnly()) value = element;
+      }
+      dictionary->ValueAtPut(entry, value);
     }
   } else {
     // Index not already used. Look for an accessor in the prototype chain.
@@ -9927,17 +9939,23 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
         int context_index = Smi::cast(probe)->value();
         ASSERT(!context->get(context_index)->IsTheHole());
         context->set(context_index, value);
-        return value;
-      } else {
-        // Object is not mapped, defer to the arguments.
-        FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
-        if (arguments->IsDictionary()) {
-          return SetDictionaryElement(index, value, attr, strict_mode,
-                                      check_prototype, set_mode);
-        } else {
-          return SetFastElement(index, value, strict_mode, check_prototype);
+        // Redefining attributes of an aliased element destroys fast aliasing.
+        if (set_mode == SET_PROPERTY || attr == NONE) return value;
+        parameter_map->set_the_hole(index + 2);
+        // For elements that are still writable we re-establish slow aliasing.
+        if ((attr & READ_ONLY) == 0) {
+          MaybeObject* maybe_entry =
+              isolate->heap()->AllocateAliasedArgumentsEntry(context_index);
+          if (!maybe_entry->ToObject(&value)) return maybe_entry;
         }
       }
+      FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
+      if (arguments->IsDictionary()) {
+        return SetDictionaryElement(index, value, attr, strict_mode,
+                                    check_prototype, set_mode);
+      } else {
+        return SetFastElement(index, value, strict_mode, check_prototype);
+      }
     }
   }
   // All possible cases have been handled above. Add a return to avoid the

src/objects.h

@@ -440,7 +440,8 @@ const int kVariableSizeSentinel = 0;
   V(SCRIPT, Script, script)                                                    \
   V(CODE_CACHE, CodeCache, code_cache)                                         \
   V(POLYMORPHIC_CODE_CACHE, PolymorphicCodeCache, polymorphic_code_cache)      \
-  V(TYPE_FEEDBACK_INFO, TypeFeedbackInfo, type_feedback_info)
+  V(TYPE_FEEDBACK_INFO, TypeFeedbackInfo, type_feedback_info)                  \
+  V(ALIASED_ARGUMENTS_ENTRY, AliasedArgumentsEntry, aliased_arguments_entry)
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
 #define STRUCT_LIST_DEBUGGER(V)                                                \
@@ -596,6 +597,7 @@ enum InstanceType {
   CODE_CACHE_TYPE,
   POLYMORPHIC_CODE_CACHE_TYPE,
   TYPE_FEEDBACK_INFO_TYPE,
+  ALIASED_ARGUMENTS_ENTRY_TYPE,
   // The following two instance types are only used when ENABLE_DEBUGGER_SUPPORT
   // is defined. However as include/v8.h contain some of the instance type
   // constants always having them avoids them getting different numbers
@@ -6435,6 +6437,39 @@ class TypeFeedbackInfo: public Struct {
 };
 
 
+// Representation of a slow alias as part of a non-strict arguments objects.
+// For fast aliases (if HasNonStrictArgumentsElements()):
+// - the parameter map contains an index into the context
+// - all attributes of the element have default values
+// For slow aliases (if HasDictionaryArgumentsElements()):
+// - the parameter map contains no fast alias mapping (i.e. the hole)
+// - this struct (in the slow backing store) contains an index into the context
+// - all attributes are available as part if the property details
+class AliasedArgumentsEntry: public Struct {
+ public:
+  inline int aliased_context_slot();
+  inline void set_aliased_context_slot(int count);
+
+  static inline AliasedArgumentsEntry* cast(Object* obj);
+
+#ifdef OBJECT_PRINT
+  inline void AliasedArgumentsEntryPrint() {
+    AliasedArgumentsEntryPrint(stdout);
+  }
+  void AliasedArgumentsEntryPrint(FILE* out);
+#endif
+#ifdef DEBUG
+  void AliasedArgumentsEntryVerify();
+#endif
+
+  static const int kAliasedContextSlot = HeapObject::kHeaderSize;
+  static const int kSize = kAliasedContextSlot + kPointerSize;
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(AliasedArgumentsEntry);
+};
+
+
 enum AllowNullsFlag {ALLOW_NULLS, DISALLOW_NULLS};
 enum RobustnessFlag {ROBUST_STRING_TRAVERSAL, FAST_STRING_TRAVERSAL};
 

test/mjsunit/object-define-property.js

@@ -1053,4 +1053,25 @@ for (var i = 0; i < 1000; i++) {
   // Non-enumerable property forces dictionary mode.
   Object.defineProperty(o, i, {value: i, enumerable: false});
 }
-assertEquals(999, o[999]);
+assertEquals(999, o[999]);
+
+
+// Regression test: Bizzare behavior on non-strict arguments object.
+(function test(arg0) {
+  // Here arguments[0] is a fast alias on arg0.
+  Object.defineProperty(arguments, "0", {
+    value:1,
+    enumerable:false
+  });
+  // Here arguments[0] is a slow alias on arg0.
+  Object.defineProperty(arguments, "0", {
+    value:2,
+    writable:false
+  });
+  // Here arguments[0] is no alias at all.
+  Object.defineProperty(arguments, "0", {
+    value:3
+  });
+  assertEquals(2, arg0);
+  assertEquals(3, arguments[0]);
+})(0);

test/test262/test262.status

@@ -42,19 +42,6 @@ S10.4.2.1_A1: FAIL
 15.2.3.6-4-415: FAIL
 15.2.3.6-4-420: FAIL
 
-# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1772
-15.2.3.6-4-292-1: FAIL
-15.2.3.6-4-293-2: FAIL
-15.2.3.6-4-293-3: FAIL
-15.2.3.6-4-294-1: FAIL
-15.2.3.6-4-295-1: FAIL
-15.2.3.6-4-296-1: FAIL
-15.2.3.7-6-a-281: FAIL
-15.2.3.7-6-a-282: FAIL
-15.2.3.7-6-a-283: FAIL
-15.2.3.7-6-a-284: FAIL
-15.2.3.7-6-a-285: FAIL
-
 ##################### DELIBERATE INCOMPATIBILITIES #####################
 
 # We deliberately treat arguments to parseInt() with a leading zero as