[string] Don't skip GetMethod on Smis in String builtins

Previously, for the various customisation points of String builtins
(like String.prototype.replace), we skipped the customisation symbol
lookup (like for Symbol.replace) for Smis.

But, we do need to do the lookup for Smis in case Number.prototype or
Object.prototype have the Symbol. This missing lookup was creating an
observable difference between Smis and HeapNumbers.

Bug: chromium:1092896
Change-Id: I8928d237fa74abeaa2aa81318b8903087c507f0d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2238030
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68285}

src/builtins/builtins-string-gen.cc

@@ -1164,10 +1164,11 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
     DescriptorIndexNameValue additional_property_to_check,
     const NodeFunction0& regexp_call, const NodeFunction1& generic_call) {
   Label out(this);
+  Label get_property_lookup(this);
 
-  // Smis definitely don't have an attached symbol.
-  GotoIf(TaggedIsSmi(object), &out);
-  TNode<HeapObject> heap_object = CAST(object);
+  // Smis have to go through the GetProperty lookup in case Number.prototype or
+  // Object.prototype was modified.
+  GotoIf(TaggedIsSmi(object), &get_property_lookup);
 
   // Take the fast path for RegExps.
   // There's two conditions: {object} needs to be a fast regexp, and
@@ -1176,6 +1177,8 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
   {
     Label stub_call(this), slow_lookup(this);
 
+    TNode<HeapObject> heap_object = CAST(object);
+
     GotoIf(TaggedIsSmi(maybe_string), &slow_lookup);
     GotoIfNot(IsString(CAST(maybe_string)), &slow_lookup);
 
@@ -1196,10 +1199,10 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
     regexp_call();
 
     BIND(&slow_lookup);
+    // Special case null and undefined to skip the property lookup.
+    Branch(IsNullOrUndefined(heap_object), &out, &get_property_lookup);
   }
 
-  GotoIf(IsNullOrUndefined(heap_object), &out);
-
   // Fall back to a slow lookup of {heap_object[symbol]}.
   //
   // The spec uses GetMethod({heap_object}, {symbol}), which has a few quirks:
@@ -1208,7 +1211,8 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
   // We handle the former by jumping to {out} for null values as well, while
   // the latter is already handled by the Call({maybe_func}) operation.
 
-  const TNode<Object> maybe_func = GetProperty(context, heap_object, symbol);
+  BIND(&get_property_lookup);
+  const TNode<Object> maybe_func = GetProperty(context, object, symbol);
   GotoIf(IsUndefined(maybe_func), &out);
   GotoIf(IsNull(maybe_func), &out);
 

test/mjsunit/regress/regress-1092896.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let count = 0
+Object.prototype[Symbol.replace] = function () {
+  count++
+}
+
+"".replace(0);
+assertEquals(count, 1);
+
+"".replace(0.1);
+assertEquals(count, 2);