[string] Fix casting around external internalized string thinning

When an external string matches and internalized external string, we
want to deduplicate them. As part of that de-duplication, we check if
either a) the internalized string's resource is null (it's freshly
created because the original string was in new space) or b) they point
to the same data (in which case we don't want to dispose of the data).

However, when doing these checks we cast both the initial and the
internalized external string to the same type (one or two byte). So,
if a two-byte string finds a one-byte internalized string with
equivalent data, this cast will fail.

Since we only care about the external string resource being null or
equal to another during the above deduplication, the solution is
casting first to the more general ExternalString type, comparing
resources by address, and only casting to the more specific type
when needed (and we know that the types have to match by other
construction).

Change-Id: Id34a02eb1900d8aa492c030488afaffd0d035454
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2315987
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69024}

src/objects/string.cc

@@ -72,19 +72,19 @@ Handle<String> String::SlowFlatten(Isolate* isolate, Handle<ConsString> cons,
 namespace {
 
 template <class StringClass>
-void MigrateExternalStringResource(Isolate* isolate, String from, String to) {
+void MigrateExternalStringResource(Isolate* isolate, ExternalString from,
+                                   StringClass to) {
+  Address to_resource_address = to.resource_as_address();
+  if (to_resource_address == kNullAddress) {
     StringClass cast_from = StringClass::cast(from);
-  StringClass cast_to = StringClass::cast(to);
-  const typename StringClass::Resource* to_resource = cast_to.resource();
-  if (to_resource == nullptr) {
     // |to| is a just-created internalized copy of |from|. Migrate the resource.
-    cast_to.SetResource(isolate, cast_from.resource());
+    to.SetResource(isolate, cast_from.resource());
     // Zap |from|'s resource pointer to reflect the fact that |from| has
     // relinquished ownership of its resource.
     isolate->heap()->UpdateExternalString(
         from, ExternalString::cast(from).ExternalPayloadSize(), 0);
     cast_from.SetResource(isolate, nullptr);
-  } else if (to_resource != cast_from.resource()) {
+  } else if (to_resource_address != from.resource_as_address()) {
     // |to| already existed and has its own resource. Finalize |from|.
     isolate->heap()->FinalizeExternalString(from);
   }
@@ -99,11 +99,11 @@ void String::MakeThin(Isolate* isolate, String internalized) {
 
   if (this->IsExternalString()) {
     if (internalized.IsExternalOneByteString()) {
-      MigrateExternalStringResource<ExternalOneByteString>(isolate, *this,
-                                                           internalized);
+      MigrateExternalStringResource(isolate, ExternalString::cast(*this),
+                                    ExternalOneByteString::cast(internalized));
     } else if (internalized.IsExternalTwoByteString()) {
-      MigrateExternalStringResource<ExternalTwoByteString>(isolate, *this,
-                                                           internalized);
+      MigrateExternalStringResource(isolate, ExternalString::cast(*this),
+                                    ExternalTwoByteString::cast(internalized));
     } else {
       // If the external string is duped into an existing non-external
       // internalized string, free its resource (it's about to be rewritten