Maintain order of keys for object.assign as spec

According to spec https://tc39.github.io/ecma262/#sec-object.assign,
https://tc39.github.io/ecma262/#sec-ordinaryownpropertykeys, object.assign should copy symbols last. The current implementation ignores that order.
The idea of the fix here is to do iteration twice, one to skip symbol first then one to skip string.

Bug: v8:6705
Change-Id: I27a353e0c44a8f7adcf55d7143dd3ce26bea2724
Reviewed-on: https://chromium-review.googlesource.com/c/1432597
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59258}

src/builtins/builtins-object-gen.cc

@@ -554,14 +554,14 @@ void ObjectBuiltinsAssembler::ObjectAssignFast(TNode<Context> context,
   GotoIfNot(IsJSObjectInstanceType(from_instance_type), slow);
   GotoIfNot(IsEmptyFixedArray(LoadElements(CAST(from))), slow);
 
-  ForEachEnumerableOwnProperty(context, from_map, CAST(from),
-                               [=](TNode<Name> key, TNode<Object> value) {
-                                 KeyedStoreGenericGenerator::SetProperty(
-                                     state(), context, to,
-                                     to_is_simple_receiver, key, value,
-                                     LanguageMode::kStrict);
-                               },
-                               slow);
+  ForEachEnumerableOwnProperty(
+      context, from_map, CAST(from), kEnumerationOrder,
+      [=](TNode<Name> key, TNode<Object> value) {
+        KeyedStoreGenericGenerator::SetProperty(state(), context, to,
+                                                to_is_simple_receiver, key,
+                                                value, LanguageMode::kStrict);
+      },
+      slow);
 
   Goto(&done);
   BIND(&done);

src/code-stub-assembler.cc

@@ -8874,7 +8874,8 @@ void CodeStubAssembler::DescriptorArrayForEach(
 
 void CodeStubAssembler::ForEachEnumerableOwnProperty(
     TNode<Context> context, TNode<Map> map, TNode<JSObject> object,
-    const ForEachKeyValueFunction& body, Label* bailout) {
+    ForEachEnumerationMode mode, const ForEachKeyValueFunction& body,
+    Label* bailout) {
   TNode<Int32T> type = LoadMapInstanceType(map);
   TNode<Uint32T> bit_field3 = EnsureOnlyHasSimpleProperties(map, type, bailout);
 
@@ -8883,17 +8884,48 @@ void CodeStubAssembler::ForEachEnumerableOwnProperty(
       DecodeWord32<Map::NumberOfOwnDescriptorsBits>(bit_field3);
 
   TVARIABLE(BoolT, var_stable, Int32TrueConstant());
-  VariableList list({&var_stable}, zone());
+
+  TVARIABLE(BoolT, var_has_symbol, Int32FalseConstant());
+  // false - iterate only string properties, true - iterate only symbol
+  // properties
+  TVARIABLE(BoolT, var_name_filter, Int32FalseConstant());
+  VariableList list({&var_stable, &var_has_symbol, &var_name_filter}, zone());
+  Label descriptor_array_loop(this,
+                              {&var_stable, &var_has_symbol, &var_name_filter});
+
+  Goto(&descriptor_array_loop);
+  BIND(&descriptor_array_loop);
 
   DescriptorArrayForEach(
       list, Unsigned(Int32Constant(0)), nof_descriptors,
-      [=, &var_stable](TNode<IntPtrT> descriptor_key_index) {
+      [=, &var_stable, &var_has_symbol,
+       &var_name_filter](TNode<IntPtrT> descriptor_key_index) {
         TNode<Name> next_key =
             LoadKeyByKeyIndex(descriptors, descriptor_key_index);
 
         TVARIABLE(Object, var_value, SmiConstant(0));
         Label callback(this), next_iteration(this);
 
+        if (mode == kEnumerationOrder) {
+          // |next_key| is either a string or a symbol
+          // Skip strings or symbols depending on var_name_filter value.
+          Label if_string(this), if_symbol(this), if_name_ok(this);
+
+          Branch(IsSymbol(next_key), &if_symbol, &if_string);
+          BIND(&if_symbol);
+          {
+            var_has_symbol = Int32TrueConstant();
+            // Process symbol property when |var_name_filer| is true.
+            Branch(var_name_filter.value(), &if_name_ok, &next_iteration);
+          }
+          BIND(&if_string);
+          {
+            CSA_ASSERT(this, IsString(next_key));
+            // Process string property when |var_name_filer| is false.
+            Branch(var_name_filter.value(), &next_iteration, &if_name_ok);
+          }
+          BIND(&if_name_ok);
+        }
         {
           TVARIABLE(Map, var_map);
           TVARIABLE(HeapObject, var_meta_storage);
@@ -8986,9 +9018,19 @@ void CodeStubAssembler::ForEachEnumerableOwnProperty(
             Goto(&next_iteration);
           }
         }
-
         BIND(&next_iteration);
       });
+
+  if (mode == kEnumerationOrder) {
+    Label done(this);
+    GotoIf(var_name_filter.value(), &done);
+    GotoIfNot(var_has_symbol.value(), &done);
+    // All string properties are processed, now process symbol properties.
+    var_name_filter = Int32TrueConstant();
+    Goto(&descriptor_array_loop);
+
+    BIND(&done);
+  }
 }
 
 void CodeStubAssembler::DescriptorLookup(

src/code-stub-assembler.h

@@ -3254,11 +3254,20 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   typedef std::function<void(TNode<Name> key, TNode<Object> value)>
       ForEachKeyValueFunction;
 
+  enum ForEachEnumerationMode {
+    // String and then Symbol properties according to the spec
+    // ES#sec-object.assign
+    kEnumerationOrder,
+    // Order of property addition
+    kPropertyAdditionOrder,
+  };
+
   // For each JSObject property (in DescriptorArray order), check if the key is
   // enumerable, and if so, load the value from the receiver and evaluate the
   // closure.
   void ForEachEnumerableOwnProperty(TNode<Context> context, TNode<Map> map,
                                     TNode<JSObject> object,
+                                    ForEachEnumerationMode mode,
                                     const ForEachKeyValueFunction& body,
                                     Label* bailout);
 

src/ic/accessor-assembler.cc

@@ -3507,12 +3507,12 @@ void AccessorAssembler::GenerateCloneObjectIC_Slow() {
 
   GotoIfNot(IsEmptyFixedArray(LoadElements(CAST(source))), &call_runtime);
 
-  ForEachEnumerableOwnProperty(context, map, CAST(source),
-                               [=](TNode<Name> key, TNode<Object> value) {
-                                 SetPropertyInLiteral(context, result, key,
-                                                      value);
-                               },
-                               &call_runtime);
+  ForEachEnumerableOwnProperty(
+      context, map, CAST(source), kPropertyAdditionOrder,
+      [=](TNode<Name> key, TNode<Object> value) {
+        SetPropertyInLiteral(context, result, key, value);
+      },
+      &call_runtime);
   Goto(&done);
 
   BIND(&call_runtime);

test/test262/test262.status

@@ -546,9 +546,6 @@
   'intl402/Locale/getters-grandfathered': [FAIL],
   'intl402/Locale/likely-subtags-grandfathered': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=6705
-  'built-ins/Object/assign/strings-and-symbol-order': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=7831
   'language/statements/generators/generator-created-after-decl-inst': [FAIL],
   'language/expressions/generators/generator-created-after-decl-inst': [FAIL],