[string] SLOW_DCHECK string hash during lifetime of String::FlatContent

With String contents being accessible off-main-thread or from multiple
main threads, add a SLOW_DCHECK that the hash of the string contents
inside a String::FlatContent doesn't change during its lifetime.

Bug: v8:12007
Change-Id: Iaf6bb785e44c97c13ac2fe9c5c20099bf1e0d2fd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3451355Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79080}

src/objects/string-inl.h

@@ -728,6 +728,51 @@ String::FlatContent String::GetFlatContent(
   return GetFlatContent(no_gc, SharedStringAccessGuardIfNeeded::NotNeeded());
 }
 
+String::FlatContent::FlatContent(const uint8_t* start, int length,
+                                 const DisallowGarbageCollection& no_gc)
+    : onebyte_start(start), length_(length), state_(ONE_BYTE), no_gc_(no_gc) {
+#ifdef ENABLE_SLOW_DCHECKS
+  checksum_ = ComputeChecksum();
+#endif
+}
+
+String::FlatContent::FlatContent(const base::uc16* start, int length,
+                                 const DisallowGarbageCollection& no_gc)
+    : twobyte_start(start), length_(length), state_(TWO_BYTE), no_gc_(no_gc) {
+#ifdef ENABLE_SLOW_DCHECKS
+  checksum_ = ComputeChecksum();
+#endif
+}
+
+String::FlatContent::~FlatContent() {
+  // When ENABLE_SLOW_DCHECKS, check the string contents did not change during
+  // the lifetime of the FlatContent. To avoid extra memory use, only the hash
+  // is checked instead of snapshotting the full character data.
+  //
+  // If you crashed here, it means something changed the character data of this
+  // FlatContent during its lifetime (e.g. GC relocated the string). This is
+  // almost always a bug. If you are certain it is not a bug, you can disable
+  // the checksum verification in the caller by calling
+  // UnsafeDisableChecksumVerification().
+  SLOW_DCHECK(checksum_ == kChecksumVerificationDisabled ||
+              checksum_ == ComputeChecksum());
+}
+
+#ifdef ENABLE_SLOW_DCHECKS
+uint32_t String::FlatContent::ComputeChecksum() const {
+  constexpr uint64_t hashseed = 1;
+  uint32_t hash;
+  if (state_ == ONE_BYTE) {
+    hash = StringHasher::HashSequentialString(onebyte_start, length_, hashseed);
+  } else {
+    DCHECK_EQ(TWO_BYTE, state_);
+    hash = StringHasher::HashSequentialString(twobyte_start, length_, hashseed);
+  }
+  DCHECK_NE(kChecksumVerificationDisabled, hash);
+  return hash;
+}
+#endif
+
 String::FlatContent String::GetFlatContent(
     const DisallowGarbageCollection& no_gc,
     const SharedStringAccessGuardIfNeeded& access_guard) {

src/objects/string.h

@@ -116,6 +116,8 @@ class String : public TorqueGeneratedString<String, Name> {
   // FlatStringReader is relocatable.
   class FlatContent {
    public:
+    inline ~FlatContent();
+
     // Returns true if the string is flat and this structure contains content.
     bool IsFlat() const { return state_ != NON_FLAT; }
     // Returns true if the structure contains one-byte content.
@@ -147,24 +149,27 @@ class String : public TorqueGeneratedString<String, Name> {
       return onebyte_start == other.onebyte_start;
     }
 
+    // It is almost always a bug if the contents of a FlatContent changes during
+    // its lifetime, which can happen due to GC or bugs in concurrent string
+    // access. Rarely, callers need the ability to GC and have ensured safety in
+    // other ways, such as in IrregexpInterpreter. Those callers can disable the
+    // checksum verification with this call.
+    void UnsafeDisableChecksumVerification() {
+#ifdef ENABLE_SLOW_DCHECKS
+      checksum_ = kChecksumVerificationDisabled;
+#endif
+    }
+
     int length() const { return length_; }
 
    private:
     enum State { NON_FLAT, ONE_BYTE, TWO_BYTE };
 
     // Constructors only used by String::GetFlatContent().
-    FlatContent(const uint8_t* start, int length,
-                const DisallowGarbageCollection& no_gc)
-        : onebyte_start(start),
-          length_(length),
-          state_(ONE_BYTE),
-          no_gc_(no_gc) {}
-    FlatContent(const base::uc16* start, int length,
-                const DisallowGarbageCollection& no_gc)
-        : twobyte_start(start),
-          length_(length),
-          state_(TWO_BYTE),
-          no_gc_(no_gc) {}
+    inline FlatContent(const uint8_t* start, int length,
+                       const DisallowGarbageCollection& no_gc);
+    inline FlatContent(const base::uc16* start, int length,
+                       const DisallowGarbageCollection& no_gc);
     explicit FlatContent(const DisallowGarbageCollection& no_gc)
         : onebyte_start(nullptr), length_(0), state_(NON_FLAT), no_gc_(no_gc) {}
 
@@ -176,6 +181,14 @@ class String : public TorqueGeneratedString<String, Name> {
     State state_;
     const DisallowGarbageCollection& no_gc_;
 
+    static constexpr uint32_t kChecksumVerificationDisabled = 0;
+
+#ifdef ENABLE_SLOW_DCHECKS
+    inline uint32_t ComputeChecksum() const;
+
+    uint32_t checksum_;
+#endif
+
     friend class String;
     friend class IterableSubString;
   };

src/regexp/regexp-interpreter.cc

@@ -1088,6 +1088,10 @@ IrregexpInterpreter::Result IrregexpInterpreter::MatchInternal(
 
   base::uc16 previous_char = '\n';
   String::FlatContent subject_content = subject_string.GetFlatContent(no_gc);
+  // Because interrupts can result in GC and string content relocation, the
+  // checksum verification in FlatContent may fail even though this code is
+  // safe. See (2) above.
+  subject_content.UnsafeDisableChecksumVerification();
   if (subject_content.IsOneByte()) {
     base::Vector<const uint8_t> subject_vector =
         subject_content.ToOneByteVector();

src/strings/uri.cc

@@ -285,6 +285,7 @@ MaybeHandle<String> Uri::Encode(Isolate* isolate, Handle<String> uri,
   std::vector<uint8_t> buffer;
   buffer.reserve(uri_length);
 
+  bool throw_error = false;
   {
     DisallowGarbageCollection no_gc;
     String::FlatContent uri_content = uri->GetFlatContent(no_gc);
@@ -310,11 +311,15 @@ MaybeHandle<String> Uri::Encode(Isolate* isolate, Handle<String> uri,
         continue;
       }
 
-      AllowGarbageCollection allocate_error_and_return;
-      THROW_NEW_ERROR(isolate, NewURIError(), String);
+      // String::FlatContent DCHECKs its contents did not change during its
+      // lifetime. Throwing the error inside the loop may cause GC and move the
+      // string contents.
+      throw_error = true;
+      break;
     }
   }
 
+  if (throw_error) THROW_NEW_ERROR(isolate, NewURIError(), String);
   return isolate->factory()->NewStringFromOneByte(base::VectorOf(buffer));
 }