[object] Ensure FlatContent and FlatStringReader are main thread only

String::FlatContent and FlatStringReader are ways of reading a flat
String. Both of them provide a speedy interface to read a String since
they bypass the dynamic dispatch of String::Get. For this very same
reason, they are not safe to be used from background threads.

Bug: v8:7790
Change-Id: I919ae5e3862df81ff3ebb5131144ff5961197b35
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2509596
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70945}

src/objects/string-inl.h

@@ -239,7 +239,7 @@ uc32 FlatStringReader::Get(int index) {
 template <typename Char>
 Char FlatStringReader::Get(int index) {
   DCHECK_EQ(is_one_byte_, sizeof(Char) == 1);
-  DCHECK(0 <= index && index <= length_);
+  DCHECK(0 <= index && index < length_);
   if (sizeof(Char) == 1) {
     return static_cast<Char>(static_cast<const uint8_t*>(start_)[index]);
   } else {

src/objects/string.cc

@@ -6,6 +6,7 @@
 
 #include "src/common/assert-scope.h"
 #include "src/common/globals.h"
+#include "src/execution/thread-id.h"
 #include "src/handles/handles-inl.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/memory-chunk.h"
@@ -520,6 +521,15 @@ Handle<Object> String::ToNumber(Isolate* isolate, Handle<String> subject) {
 
 String::FlatContent String::GetFlatContent(
     const DisallowHeapAllocation& no_gc) {
+#if DEBUG
+  // Check that this method is called only from the main thread.
+  {
+    Isolate* isolate;
+    // We don't have to check read only strings as those won't move.
+    DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
+                   ThreadId::Current() == isolate->thread_id());
+  }
+#endif
   USE(no_gc);
   int length = this->length();
   StringShape shape(*this);
@@ -528,7 +538,7 @@ String::FlatContent String::GetFlatContent(
   if (shape.representation_tag() == kConsStringTag) {
     ConsString cons = ConsString::cast(string);
     if (cons.second().length() != 0) {
-      return FlatContent();
+      return FlatContent(no_gc);
     }
     string = cons.first();
     shape = StringShape(string);
@@ -554,7 +564,7 @@ String::FlatContent String::GetFlatContent(
     } else {
       start = ExternalOneByteString::cast(string).GetChars();
     }
-    return FlatContent(start + offset, length);
+    return FlatContent(start + offset, length, no_gc);
   } else {
     DCHECK_EQ(shape.encoding_tag(), kTwoByteStringTag);
     const uc16* start;
@@ -563,7 +573,7 @@ String::FlatContent String::GetFlatContent(
     } else {
       start = ExternalTwoByteString::cast(string).GetChars();
     }
-    return FlatContent(start + offset, length);
+    return FlatContent(start + offset, length, no_gc);
   }
 }
 
@@ -1512,6 +1522,10 @@ int ExternalString::ExternalPayloadSize() const {
 
 FlatStringReader::FlatStringReader(Isolate* isolate, Handle<String> str)
     : Relocatable(isolate), str_(str), length_(str->length()) {
+#if DEBUG
+  // Check that this constructor is called only from the main thread.
+  DCHECK_EQ(ThreadId::Current(), isolate->thread_id());
+#endif
   PostGarbageCollection();
 }
 

src/objects/string.h

@@ -100,6 +100,10 @@ class String : public TorqueGeneratedString<String, Name> {
   // A flat string has content that's encoded as a sequence of either
   // one-byte chars or two-byte UC16.
   // Returned by String::GetFlatContent().
+  // Not safe to use from concurrent background threads.
+  // TODO(solanes): Move FlatContent into FlatStringReader, and make it private.
+  // This would de-duplicate code, as well as taking advantage of the fact that
+  // FlatStringReader is relocatable.
   class FlatContent {
    public:
     // Returns true if the string is flat and this structure contains content.
@@ -137,11 +141,20 @@ class String : public TorqueGeneratedString<String, Name> {
     enum State { NON_FLAT, ONE_BYTE, TWO_BYTE };
 
     // Constructors only used by String::GetFlatContent().
-    explicit FlatContent(const uint8_t* start, int length)
-        : onebyte_start(start), length_(length), state_(ONE_BYTE) {}
-    explicit FlatContent(const uc16* start, int length)
-        : twobyte_start(start), length_(length), state_(TWO_BYTE) {}
-    FlatContent() : onebyte_start(nullptr), length_(0), state_(NON_FLAT) {}
+    FlatContent(const uint8_t* start, int length,
+                const DisallowHeapAllocation& no_gc)
+        : onebyte_start(start),
+          length_(length),
+          state_(ONE_BYTE),
+          no_gc_(no_gc) {}
+    FlatContent(const uc16* start, int length,
+                const DisallowHeapAllocation& no_gc)
+        : twobyte_start(start),
+          length_(length),
+          state_(TWO_BYTE),
+          no_gc_(no_gc) {}
+    explicit FlatContent(const DisallowHeapAllocation& no_gc)
+        : onebyte_start(nullptr), length_(0), state_(NON_FLAT), no_gc_(no_gc) {}
 
     union {
       const uint8_t* onebyte_start;
@@ -149,6 +162,7 @@ class String : public TorqueGeneratedString<String, Name> {
     };
     int length_;
     State state_;
+    const DisallowHeapAllocation& no_gc_;
 
     friend class String;
     friend class IterableSubString;
@@ -834,8 +848,9 @@ class ExternalTwoByteString : public ExternalString {
 };
 
 // A flat string reader provides random access to the contents of a
-// string independent of the character width of the string.  The handle
+// string independent of the character width of the string. The handle
 // must be valid as long as the reader is being used.
+// Not safe to use from concurrent background threads.
 class V8_EXPORT_PRIVATE FlatStringReader : public Relocatable {
  public:
   FlatStringReader(Isolate* isolate, Handle<String> str);

src/regexp/regexp-parser.cc

@@ -1829,15 +1829,6 @@ bool RegExpParser::ParseRegExp(Isolate* isolate, Zone* zone,
   return success;
 }
 
-bool RegExpParser::VerifyRegExpSyntax(Isolate* isolate, Zone* zone,
-                                      FlatStringReader* input,
-                                      JSRegExp::Flags flags,
-                                      RegExpCompileData* result,
-                                      const DisallowHeapAllocation& no_gc) {
-  RegExpParser parser(input, flags, isolate, zone);
-  return parser.Parse(result, no_gc);
-}
-
 RegExpBuilder::RegExpBuilder(Zone* zone, JSRegExp::Flags flags)
     : zone_(zone),
       pending_empty_(false),

src/regexp/regexp-parser.h

@@ -159,10 +159,6 @@ class V8_EXPORT_PRIVATE RegExpParser {
 
   static bool ParseRegExp(Isolate* isolate, Zone* zone, FlatStringReader* input,
                           JSRegExp::Flags flags, RegExpCompileData* result);
-  static bool VerifyRegExpSyntax(Isolate* isolate, Zone* zone,
-                                 FlatStringReader* input, JSRegExp::Flags flags,
-                                 RegExpCompileData* result,
-                                 const DisallowHeapAllocation& no_gc);
 
  private:
   bool Parse(RegExpCompileData* result, const DisallowHeapAllocation&);