[compiler] Fix a wrong access to a string in the background

Before https://crrev.com/c/3829541, ReduceStringPrototypeStartsWith
would not be called if the String's content wasn't safe to access in
the background, because StringRef::length would fail in that case. Now
that StringRef::length always succeeds, an additional check is
required before calling ReduceStringPrototypeStartsWith.

Note that none of the other callers of StringRef::length access the
String's content later, so we shouldn't have any more bugs caused by
the aforementioned CL.

Bug: chromium:1354439
Change-Id: I4a590ccdb7cc4c8a85e4e6beaf6f3c3ab2d7d479
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3840938
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82592}

src/compiler/heap-refs.cc

@@ -1276,8 +1276,12 @@ bool StringRef::SupportedStringKind() const {
   return IsInternalizedString() || object()->IsThinString();
 }
 
+bool StringRef::IsContentAccessible() const {
+  return data_->kind() != kNeverSerializedHeapObject || SupportedStringKind();
+}
+
 base::Optional<Handle<String>> StringRef::ObjectIfContentAccessible() {
-  if (data_->kind() == kNeverSerializedHeapObject && !SupportedStringKind()) {
+  if (!IsContentAccessible()) {
     TRACE_BROKER_MISSING(
         broker(),
         "content for kNeverSerialized unsupported string kind " << *this);
@@ -1292,7 +1296,7 @@ int StringRef::length() const { return object()->length(kAcquireLoad); }
 base::Optional<uint16_t> StringRef::GetFirstChar() const { return GetChar(0); }
 
 base::Optional<uint16_t> StringRef::GetChar(int index) const {
-  if (data_->kind() == kNeverSerializedHeapObject && !SupportedStringKind()) {
+  if (!IsContentAccessible()) {
     TRACE_BROKER_MISSING(
         broker(),
         "get char for kNeverSerialized unsupported string kind " << *this);
@@ -1309,7 +1313,7 @@ base::Optional<uint16_t> StringRef::GetChar(int index) const {
 }
 
 base::Optional<double> StringRef::ToNumber() {
-  if (data_->kind() == kNeverSerializedHeapObject && !SupportedStringKind()) {
+  if (!IsContentAccessible()) {
     TRACE_BROKER_MISSING(
         broker(),
         "number for kNeverSerialized unsupported string kind " << *this);

src/compiler/heap-refs.h

@@ -930,6 +930,8 @@ class StringRef : public NameRef {
   bool IsSeqString() const;
   bool IsExternalString() const;
 
+  bool IsContentAccessible() const;
+
  private:
   // With concurrent inlining on, we currently support reading directly
   // internalized strings, and thin strings (which are pointers to internalized

src/compiler/js-call-reducer.cc

@@ -1194,6 +1194,7 @@ TNode<String> JSCallReducerAssembler::ReduceStringPrototypeSubstring() {
 
 TNode<Boolean> JSCallReducerAssembler::ReduceStringPrototypeStartsWith(
     const StringRef& search_element_string) {
+  DCHECK(search_element_string.IsContentAccessible());
   TNode<Object> receiver = ReceiverInput();
   TNode<Object> start = ArgumentOrZero(1);
 
@@ -6660,6 +6661,7 @@ Reduction JSCallReducer::ReduceStringPrototypeStartsWith(Node* node) {
     ObjectRef target_ref = search_element_matcher.Ref(broker());
     if (!target_ref.IsString()) return NoChange();
     StringRef search_element_string = target_ref.AsString();
+    if (!search_element_string.IsContentAccessible()) return NoChange();
     int length = search_element_string.length();
     // If search_element's length is less or equal than
     // kMaxInlineMatchSequence, we inline the entire