Fix Object.freeze for objects with mixed accessors and data properties

src/objects.cc

 // Copyright 2013 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
@@ skipping 6726 matching lines @@
 
   DescriptorArray* descriptors;
   MaybeObject* maybe_descriptors = Allocate(size);
   if (!maybe_descriptors->To(&descriptors)) return maybe_descriptors;
   DescriptorArray::WhitenessWitness witness(descriptors);
 
   if (attributes != NONE) {
     for (int i = 0; i < size; ++i) {
       Object* value = GetValue(i);
       PropertyDetails details = GetDetails(i);
+      int mask = DONT_DELETE | DONT_ENUM;

[Toon Verwaest, 2013/05/25 10:52:59]

This is very confusing to me. Don't we just want to add DONT_DELETE and
DONT_ENUM to all attributes of a frozen object (so add it to the client of
CopyUpToAddAttributes), and still mask out READ_ONLY from all callbacks; cause
it's never valid?

In this version it doesn't look like a generic CopyUpToAddAttributes at all...

       // READ_ONLY is an invalid attribute for JS setters/getters.
-      if (details.type() == CALLBACKS && value->IsAccessorPair()) {
-        attributes = static_cast<PropertyAttributes>(attributes & ~READ_ONLY);
+      if (details.type() != CALLBACKS || !value->IsAccessorPair()) {
+        mask |= READ_ONLY;
       }
-      Descriptor desc(GetKey(i), value, details.CopyAddAttributes(attributes));
+      details = details.CopyAddAttributes(
+          static_cast<PropertyAttributes>(attributes & mask));
+      Descriptor desc(GetKey(i), value, details);
       descriptors->Set(i, &desc, witness);
     }
   } else {
     for (int i = 0; i < size; ++i) {
       descriptors->CopyFrom(i, this, i, witness);
     }
   }
 
   if (number_of_descriptors() != enumeration_index) descriptors->Sort();
 
@@ skipping 8871 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal