From 76c6fd5e78137d21d268b185adfd0b8fb4004cc3 Mon Sep 17 00:00:00 2001 From: Bill Budge Date: Wed, 31 Mar 2021 16:50:18 +0000 Subject: [PATCH] Revert "[string] Fix non-SeqStrings in IsEqualTo" This reverts commit e70cbb83da136f6c35f9835c200ed627664d512c. Reason for revert: Breaks compile on gcc. https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20gcc/11148 Original change's description: > [string] Fix non-SeqStrings in IsEqualTo > > Bug: chromium:1193903 > Change-Id: I80704dd3cba5754779432356b20bd3ea99630291 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2794426 > Commit-Queue: Leszek Swirski > Commit-Queue: Igor Sheludko > Reviewed-by: Igor Sheludko > Auto-Submit: Leszek Swirski > Cr-Commit-Position: refs/heads/master@{#73746} Bug: chromium:1193903 Change-Id: If700cdc7cf8b50a9430d17489485769cb524efd5 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2797539 Auto-Submit: Bill Budge Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/master@{#73749} --- src/objects/string-inl.h | 140 ++++++++++-------------- src/objects/string.cc | 6 +- src/objects/string.h | 13 +-- test/mjsunit/regress/regress-1193903.js | 12 -- 4 files changed, 61 insertions(+), 110 deletions(-) delete mode 100644 test/mjsunit/regress/regress-1193903.js diff --git a/src/objects/string-inl.h b/src/objects/string-inl.h index 9747102f51..aa8c584636 100644 --- a/src/objects/string-inl.h +++ b/src/objects/string-inl.h @@ -9,7 +9,6 @@ #include "src/common/external-pointer-inl.h" #include "src/common/external-pointer.h" #include "src/common/globals.h" -#include "src/execution/isolate-utils.h" #include "src/handles/handles-inl.h" #include "src/heap/factory.h" #include "src/numbers/conversions-inl.h" @@ -404,8 +403,7 @@ class SeqSubStringKey final : public StringTableKey { DCHECK(!SharedStringAccessGuardIfNeeded::IsNeeded(*string_)); DisallowGarbageCollection no_gc; return string.IsEqualTo( - Vector(string_->GetChars(no_gc) + from_, length()), - isolate); + Vector(string_->GetChars(no_gc) + from_, length())); } Handle AsHandle(Isolate* isolate) { @@ -456,26 +454,19 @@ bool String::Equals(Isolate* isolate, Handle one, Handle two) { template bool String::IsEqualTo(Vector str, Isolate* isolate) const { DCHECK(!SharedStringAccessGuardIfNeeded::IsNeeded(*this)); - return IsEqualToImpl(str, isolate, - SharedStringAccessGuardIfNeeded::NotNeeded()); -} - -template -bool String::IsEqualTo(Vector str) const { - DCHECK(!SharedStringAccessGuardIfNeeded::IsNeeded(*this)); - return IsEqualToImpl(str, GetIsolateForPtrCompr(*this), + return IsEqualToImpl(str, SharedStringAccessGuardIfNeeded::NotNeeded()); } template bool String::IsEqualTo(Vector str, LocalIsolate* isolate) const { SharedStringAccessGuardIfNeeded access_guard(isolate); - return IsEqualToImpl(str, isolate, access_guard); + return IsEqualToImpl(str, access_guard); } template bool String::IsEqualToImpl( - Vector str, IsolateRoot isolate, + Vector str, const SharedStringAccessGuardIfNeeded& access_guard) const { size_t len = str.size(); switch (kEqType) { @@ -492,77 +483,60 @@ bool String::IsEqualToImpl( DisallowGarbageCollection no_gc; - int slice_offset = 0; - String string = *this; - const Char* data = str.data(); - while (true) { - int32_t type = string.map(isolate).instance_type(); - switch (type & (kStringRepresentationMask | kStringEncodingMask)) { - case kSeqStringTag | kOneByteStringTag: - return CompareCharsEqual( - SeqOneByteString::cast(string).GetChars(no_gc, access_guard) + - slice_offset, - data, len); - case kSeqStringTag | kTwoByteStringTag: - return CompareCharsEqual( - SeqTwoByteString::cast(string).GetChars(no_gc, access_guard) + - slice_offset, - data, len); - case kExternalStringTag | kOneByteStringTag: - return CompareCharsEqual( - ExternalOneByteString::cast(string).GetChars() + slice_offset, data, - len); - case kExternalStringTag | kTwoByteStringTag: - return CompareCharsEqual( - ExternalTwoByteString::cast(string).GetChars() + slice_offset, data, - len); - - case kSlicedStringTag | kOneByteStringTag: - case kSlicedStringTag | kTwoByteStringTag: { - SlicedString slicedString = SlicedString::cast(string); - slice_offset += slicedString.offset(); - string = slicedString.parent(isolate); - continue; - } - - case kConsStringTag | kOneByteStringTag: - case kConsStringTag | kTwoByteStringTag: { - ConsStringIterator iter(ConsString::cast(string), slice_offset); - const Char* p = data; - size_t remaining_len = len; - for (String segment = iter.Next(&slice_offset); !segment.is_null(); - segment = iter.Next(&slice_offset)) { - int str_len = segment.length(); - if (kEqType == EqualityType::kPrefix) { - str_len = std::min(str_len, static_cast(remaining_len)); - remaining_len -= str_len; - } - DCHECK_LE(p + str_len, data + len); - if (!segment.IsEqualToImpl( - VectorOf(p, str_len), isolate, access_guard)) { - return false; - } - p += str_len; - if (kEqType == EqualityType::kPrefix && remaining_len == 0) { - break; - } - } - DCHECK_EQ(p, data + len); - if (kEqType == EqualityType::kPrefix) { - DCHECK_EQ(remaining_len, 0); - } - return true; - } - - case kThinStringTag | kOneByteStringTag: - case kThinStringTag | kTwoByteStringTag: - string = ThinString::cast(string).actual(isolate); - continue; - - default: - UNREACHABLE(); + class IsEqualToDispatcher : public AllStatic { + public: + static inline bool HandleSeqOneByteString( + SeqOneByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareCharsEqual(str.GetChars(no_gc, access_guard), data, len); } - } + static inline bool HandleSeqTwoByteString( + SeqTwoByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareCharsEqual(str.GetChars(no_gc, access_guard), data, len); + } + static inline bool HandleExternalOneByteString( + ExternalOneByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareCharsEqual(str.GetChars(), data, len); + } + static inline bool HandleExternalTwoByteString( + ExternalTwoByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareCharsEqual(str.GetChars(), data, len); + } + static inline bool HandleConsString( + ConsString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleSlicedString( + SlicedString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleThinString( + ThinString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleInvalidString( + String str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + }; + + return StringShape(*this).DispatchToSpecificType( + *this, str.data(), len, no_gc, access_guard); } bool String::IsOneByteEqualTo(Vector str) { return IsEqualTo(str); } diff --git a/src/objects/string.cc b/src/objects/string.cc index a65cfc9bcf..4d137d096d 100644 --- a/src/objects/string.cc +++ b/src/objects/string.cc @@ -6,7 +6,6 @@ #include "src/common/assert-scope.h" #include "src/common/globals.h" -#include "src/execution/isolate-utils.h" #include "src/execution/thread-id.h" #include "src/handles/handles-inl.h" #include "src/heap/heap-inl.h" @@ -1287,10 +1286,7 @@ Object String::LastIndexOf(Isolate* isolate, Handle receiver, } bool String::HasOneBytePrefix(Vector str) { - DCHECK(!SharedStringAccessGuardIfNeeded::IsNeeded(*this)); - return IsEqualToImpl( - str, GetIsolateForPtrCompr(*this), - SharedStringAccessGuardIfNeeded::NotNeeded()); + return IsEqualTo(str); } namespace { diff --git a/src/objects/string.h b/src/objects/string.h index bd363f3df1..006dafdd51 100644 --- a/src/objects/string.h +++ b/src/objects/string.h @@ -326,15 +326,8 @@ class String : public TorqueGeneratedString { // The Isolate is passed as "evidence" that this call is on the main thread, // and to distiguish from the LocalIsolate overload. template - inline bool IsEqualTo(Vector str, Isolate* isolate) const; - - // Check if this string matches the given vector of characters, either as a - // whole string or just a prefix. - // - // This is main-thread only, like the Isolate* overload, but additionally - // computes the IsolateRoot for IsEqualToImpl. - template - inline bool IsEqualTo(Vector str) const; + inline bool IsEqualTo(Vector str, + Isolate* isolate = nullptr) const; // Check if this string matches the given vector of characters, either as a // whole string or just a prefix. @@ -546,7 +539,7 @@ class String : public TorqueGeneratedString { // Implementation of the IsEqualTo() public methods. Do not use directly. template V8_INLINE bool IsEqualToImpl( - Vector str, IsolateRoot isolate, + Vector str, const SharedStringAccessGuardIfNeeded& access_guard) const; V8_EXPORT_PRIVATE static Handle SlowFlatten( diff --git a/test/mjsunit/regress/regress-1193903.js b/test/mjsunit/regress/regress-1193903.js deleted file mode 100644 index 491ba1150d..0000000000 --- a/test/mjsunit/regress/regress-1193903.js +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2014 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. -// -// Flags: --allow-natives-syntax - -var no_sync_uninternalized = "no " + "sync"; -%InternalizeString(no_sync_uninternalized); - -// Make sure %GetOptimizationStatus works with a non-internalized string -// parameter. -%GetOptimizationStatus(function() {}, no_sync_uninternalized)