[heap] Don't internalize external uncached Strings

In order to avoid internal external uncached Strings, we can copy the
String at the moment of internalizing if it is an external & uncached
String.

Bug: v8:7790
Change-Id: Ie7ed287c105a127b8b4c867aab1a808265a922b7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2613029
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71977}

src/heap/factory.cc

@@ -797,10 +797,6 @@ MaybeHandle<Map> GetInternalizedStringMap(Factory* f, Handle<String> string) {
       return f->external_internalized_string_map();
     case EXTERNAL_ONE_BYTE_STRING_TYPE:
       return f->external_one_byte_internalized_string_map();
-    case UNCACHED_EXTERNAL_STRING_TYPE:
-      return f->uncached_external_internalized_string_map();
-    case UNCACHED_EXTERNAL_ONE_BYTE_STRING_TYPE:
-      return f->uncached_external_one_byte_internalized_string_map();
     default:
       return MaybeHandle<Map>();  // No match found.
   }

src/heap/factory.h

@@ -993,7 +993,6 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   void InitializeJSObjectBody(Handle<JSObject> obj, Handle<Map> map,
                               int start_offset);
 
- private:
   Handle<WeakArrayList> NewUninitializedWeakArrayList(
       int capacity, AllocationType allocation = AllocationType::kYoung);
 };

src/logging/log.cc

@@ -2227,8 +2227,9 @@ void ExistingCodeLogger::LogExistingFunction(
     }
   } else if (shared->IsApiFunction()) {
     // API function.
-    FunctionTemplateInfo fun_data = shared->get_api_func_data();
-    Object raw_call_data = fun_data.call_code(kAcquireLoad);
+    Handle<FunctionTemplateInfo> fun_data =
+        handle(shared->get_api_func_data(), isolate_);
+    Object raw_call_data = fun_data->call_code(kAcquireLoad);
     if (!raw_call_data.IsUndefined(isolate_)) {
       CallHandlerInfo call_data = CallHandlerInfo::cast(raw_call_data);
       Object callback_obj = call_data.callback();
@@ -2240,7 +2241,7 @@ void ExistingCodeLogger::LogExistingFunction(
       CALL_CODE_EVENT_HANDLER(CallbackEvent(fun_name, entry_point))
 
       // Fast API function.
-      Address c_function = v8::ToCData<Address>(fun_data.GetCFunction());
+      Address c_function = v8::ToCData<Address>(fun_data->GetCFunction());
       if (c_function != kNullAddress) {
         CALL_CODE_EVENT_HANDLER(CallbackEvent(fun_name, c_function))
       }

src/objects/string-inl.h

@@ -154,6 +154,10 @@ bool StringShape::IsSequential() {
   return (type_ & kStringRepresentationMask) == kSeqStringTag;
 }
 
+bool StringShape::IsUncachedExternal() {
+  return (type_ & kUncachedExternalStringMask) == kUncachedExternalStringTag;
+}
+
 StringRepresentationTag StringShape::representation_tag() {
   uint32_t tag = (type_ & kStringRepresentationMask);
   return static_cast<StringRepresentationTag>(tag);

src/objects/string-table.cc

@@ -372,11 +372,12 @@ class InternalizedStringKey final : public StringTableKey {
     }
     if (FLAG_thin_strings) {
       // External strings get special treatment, to avoid copying their
-      // contents.
-      if (string_->IsExternalOneByteString()) {
+      // contents as long as they are not uncached.
+      StringShape shape(*string_);
+      if (shape.IsExternalOneByte() && !shape.IsUncachedExternal()) {
         return isolate->factory()
             ->InternalizeExternalString<ExternalOneByteString>(string_);
-      } else if (string_->IsExternalTwoByteString()) {
+      } else if (shape.IsExternalTwoByte() && !shape.IsUncachedExternal()) {
         return isolate->factory()
             ->InternalizeExternalString<ExternalTwoByteString>(string_);
       }

src/objects/string.h

@@ -50,6 +50,7 @@ class StringShape {
   inline bool IsSliced();
   inline bool IsThin();
   inline bool IsIndirect();
+  inline bool IsUncachedExternal();
   inline bool IsExternalOneByte();
   inline bool IsExternalTwoByte();
   inline bool IsSequentialOneByte();

test/cctest/test-strings.cc

@@ -2011,6 +2011,141 @@ TEST(MakeExternalCreationFailureTwoByte) {
   }
 }
 
+// Show that it is possible to internalize an external string without a copy, as
+// long as it is not uncached.
+TEST(InternalizeExternalString) {
+  CcTest::InitializeVM();
+  Factory* factory = CcTest::i_isolate()->factory();
+  v8::HandleScope scope(CcTest::isolate());
+
+  // Create the string.
+  const char* raw_string = "external";
+  OneByteResource* resource =
+      new OneByteResource(i::StrDup(raw_string), strlen(raw_string));
+  Handle<String> string =
+      factory->NewExternalStringFromOneByte(resource).ToHandleChecked();
+  CHECK(string->IsExternalString());
+
+  // Check it is not uncached.
+  Handle<ExternalString> external = Handle<ExternalString>::cast(string);
+  CHECK(!external->is_uncached());
+
+  // Internalize succesfully, without a copy.
+  Handle<String> internal = factory->InternalizeString(external);
+  CHECK(string->IsInternalizedString());
+  CHECK(string.equals(internal));
+}
+
+// Show that it is possible to internalize an external string without a copy, as
+// long as it is not uncached. Two byte version.
+TEST(InternalizeExternalStringTwoByte) {
+  CcTest::InitializeVM();
+  Factory* factory = CcTest::i_isolate()->factory();
+  v8::HandleScope scope(CcTest::isolate());
+
+  // Create the string.
+  const char* raw_string = "external";
+  Resource* resource =
+      new Resource(AsciiToTwoByteString(raw_string), strlen(raw_string));
+  Handle<String> string =
+      factory->NewExternalStringFromTwoByte(resource).ToHandleChecked();
+  CHECK(string->IsExternalString());
+
+  // Check it is not uncached.
+  Handle<ExternalString> external = Handle<ExternalString>::cast(string);
+  CHECK(!external->is_uncached());
+
+  // Internalize succesfully, without a copy.
+  Handle<String> internal = factory->InternalizeString(external);
+  CHECK(string->IsInternalizedString());
+  CHECK(string.equals(internal));
+}
+
+class UncachedExternalOneByteResource
+    : public v8::String::ExternalOneByteStringResource {
+ public:
+  explicit UncachedExternalOneByteResource(const char* data)
+      : data_(data), length_(strlen(data)) {}
+
+  ~UncachedExternalOneByteResource() override { i::DeleteArray(data_); }
+
+  const char* data() const override { return data_; }
+  size_t length() const override { return length_; }
+  bool IsCacheable() const override { return false; }
+
+ private:
+  const char* data_;
+  size_t length_;
+};
+
+// Show that we can internalize an external uncached string, by creating a copy.
+TEST(InternalizeExternalStringUncachedWithCopy) {
+  CcTest::InitializeVM();
+  Factory* factory = CcTest::i_isolate()->factory();
+  v8::HandleScope scope(CcTest::isolate());
+
+  // Create the string.
+  const char* raw_string = "external";
+  UncachedExternalOneByteResource* resource =
+      new UncachedExternalOneByteResource(i::StrDup(raw_string));
+  Handle<String> string =
+      factory->NewExternalStringFromOneByte(resource).ToHandleChecked();
+  CHECK(string->IsExternalString());
+
+  // Check it is uncached.
+  Handle<ExternalString> external = Handle<ExternalString>::cast(string);
+  CHECK(external->is_uncached());
+
+  // Internalize succesfully, with a copy.
+  Handle<String> internal = factory->InternalizeString(external);
+  CHECK(!external->IsInternalizedString());
+  CHECK(internal->IsInternalizedString());
+}
+
+class UncachedExternalResource : public v8::String::ExternalStringResource {
+ public:
+  explicit UncachedExternalResource(const uint16_t* data)
+      : data_(data), length_(0) {
+    while (data[length_]) ++length_;
+  }
+
+  ~UncachedExternalResource() override { i::DeleteArray(data_); }
+
+  const uint16_t* data() const override { return data_; }
+  size_t length() const override { return length_; }
+  bool IsCacheable() const override { return false; }
+
+ private:
+  const uint16_t* data_;
+  size_t length_;
+};
+
+// Show that we can internalize an external uncached string, by creating a copy.
+// Two byte version.
+TEST(InternalizeExternalStringUncachedWithCopyTwoByte) {
+  CcTest::InitializeVM();
+  Factory* factory = CcTest::i_isolate()->factory();
+  v8::HandleScope scope(CcTest::isolate());
+
+  // Create the string.
+  const char* raw_string = "external";
+  UncachedExternalResource* resource =
+      new UncachedExternalResource(AsciiToTwoByteString(raw_string));
+  Handle<String> string =
+      factory->NewExternalStringFromTwoByte(resource).ToHandleChecked();
+  CHECK(string->IsExternalString());
+
+  // Check it is uncached.
+  Handle<ExternalString> external = Handle<ExternalString>::cast(string);
+  CHECK(external->is_uncached());
+
+  // Internalize succesfully, with a copy.
+  CHECK(!external->IsInternalizedString());
+  Handle<String> internal = factory->InternalizeString(external);
+  CHECK(!external->IsInternalizedString());
+  CHECK(internal->IsInternalizedString());
+}
+
 }  // namespace test_strings
 }  // namespace internal
 }  // namespace v8