Fix ObjectToString builtin to work for subclasses

When we call ObjectToString on JSValues we only looked at string tags
on the prototype map of the JSValue's value type and do not check the
tags on the JSValue's prototype chain which may be non-trivial if
subclassing is involved. For ex: if we have a class Test extend Number,
we only looked for tags on the Number prototype and not on the Test
prototype. This cl fixes the builtin to also check for these cases.


Bug: v8:7706
Change-Id: I9f0e3bb6499646bf27b92bf4fb4e9014f6efa56b
Reviewed-on: https://chromium-review.googlesource.com/c/1378176Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58338}

src/builtins/builtins-object-gen.cc

@@ -1056,17 +1056,57 @@ TF_BUILTIN(ObjectToString, ObjectBuiltinsAssembler) {
 
   BIND(&if_value);
   {
+    Label if_value_is_number(this, Label::kDeferred),
+        if_value_is_boolean(this, Label::kDeferred),
+        if_value_is_symbol(this, Label::kDeferred),
+        if_value_is_bigint(this, Label::kDeferred),
+        if_value_is_string(this, Label::kDeferred);
+
     Node* receiver_value = LoadJSValueValue(receiver);
-    GotoIf(TaggedIsSmi(receiver_value), &if_number);
+    // We need to start with the object to see if the value was a subclass
+    // which might have interesting properties.
+    var_holder.Bind(receiver);
+    GotoIf(TaggedIsSmi(receiver_value), &if_value_is_number);
     Node* receiver_value_map = LoadMap(receiver_value);
-    GotoIf(IsHeapNumberMap(receiver_value_map), &if_number);
-    GotoIf(IsBooleanMap(receiver_value_map), &if_boolean);
-    GotoIf(IsSymbolMap(receiver_value_map), &if_symbol);
+    GotoIf(IsHeapNumberMap(receiver_value_map), &if_value_is_number);
+    GotoIf(IsBooleanMap(receiver_value_map), &if_value_is_boolean);
+    GotoIf(IsSymbolMap(receiver_value_map), &if_value_is_symbol);
     Node* receiver_value_instance_type =
         LoadMapInstanceType(receiver_value_map);
-    GotoIf(IsBigIntInstanceType(receiver_value_instance_type), &if_bigint);
+    GotoIf(IsBigIntInstanceType(receiver_value_instance_type),
+           &if_value_is_bigint);
     CSA_ASSERT(this, IsStringInstanceType(receiver_value_instance_type));
-    Goto(&if_string);
+    Goto(&if_value_is_string);
+
+    BIND(&if_value_is_number);
+    {
+      var_default.Bind(LoadRoot(RootIndex::knumber_to_string));
+      Goto(&checkstringtag);
+    }
+
+    BIND(&if_value_is_boolean);
+    {
+      var_default.Bind(LoadRoot(RootIndex::kboolean_to_string));
+      Goto(&checkstringtag);
+    }
+
+    BIND(&if_value_is_string);
+    {
+      var_default.Bind(LoadRoot(RootIndex::kstring_to_string));
+      Goto(&checkstringtag);
+    }
+
+    BIND(&if_value_is_bigint);
+    {
+      var_default.Bind(LoadRoot(RootIndex::kobject_to_string));
+      Goto(&checkstringtag);
+    }
+
+    BIND(&if_value_is_symbol);
+    {
+      var_default.Bind(LoadRoot(RootIndex::kobject_to_string));
+      Goto(&checkstringtag);
+    }
   }
 
   BIND(&checkstringtag);

src/objects.cc

@@ -6693,11 +6693,10 @@ void JSObject::MigrateSlowToFast(Handle<JSObject> object,
 
   // Allocate new map.
   Handle<Map> new_map = Map::CopyDropDescriptors(isolate, old_map);
-  if (new_map->has_named_interceptor() || new_map->is_access_check_needed()) {
-    // Force certain slow paths when API interceptors are used, or if an access
-    // check is required.
-    new_map->set_may_have_interesting_symbols(true);
-  }
+  // We should not only set this bit if we need to. We should not retain the
+  // old bit because turning a map into dictionary always sets this bit.
+  new_map->set_may_have_interesting_symbols(new_map->has_named_interceptor() ||
+                                            new_map->is_access_check_needed());
   new_map->set_is_dictionary_map(false);
 
   NotifyMapChange(old_map, new_map, isolate);

test/mjsunit/es6/regress/regress-7706.js

@@ -4,13 +4,37 @@
 
 // Flags: --allow-natives-syntax
 
-class Test extends Number {}
-Test.prototype[Symbol.toStringTag] = "Test";
-
 function toString(o) {
   %ToFastProperties(o.__proto__);
   return Object.prototype.toString.call(o);
 }
 
-assertEquals("[object Test]", toString(new Test), "Try #1");
-assertEquals("[object Test]", toString(new Test), "Try #2");
+class TestNumber extends Number {}
+TestNumber.prototype[Symbol.toStringTag] = "TestNumber";
+assertEquals("[object TestNumber]", toString(new TestNumber), "Try #1");
+assertEquals("[object TestNumber]", toString(new TestNumber), "Try #2");
+
+class TestBoolean extends Boolean {}
+TestBoolean.prototype[Symbol.toStringTag] = "TestBoolean";
+assertEquals("[object TestBoolean]", toString(new TestBoolean), "Try #1");
+assertEquals("[object TestBoolean]", toString(new TestBoolean), "Try #2");
+
+class TestString extends String {}
+TestString.prototype[Symbol.toStringTag] = "TestString";
+assertEquals("[object TestString]", toString(new TestString), "Try #1");
+assertEquals("[object TestString]", toString(new TestString), "Try #2");
+
+class base {}
+class TestBigInt extends base {}
+TestBigInt.prototype[Symbol.toStringTag] = 'TestBigInt';
+var b = new TestBigInt();
+b.__proto__.__proto__ = BigInt.prototype;
+assertEquals("[object TestBigInt]", toString(b), "Try #1");
+assertEquals("[object TestBigInt]", toString(b), "Try #2");
+
+class TestSymbol extends base {}
+TestSymbol.prototype[Symbol.toStringTag] = 'TestSymbol';
+var sym = new TestSymbol();
+sym.__proto__.__proto__ = Symbol.prototype;
+assertEquals("[object TestSymbol]", toString(sym), "Try #1");
+assertEquals("[object TestSymbol]", toString(sym), "Try #2");