Commit aeddf8c4 authored by Patrick Thier's avatar Patrick Thier Committed by V8 LUCI CQ

[string] Add CHECKs to String::ComputeAndSetHash

In addition change DCHECKs to CHECKs in StringForwardingTable.
The added CHECKs hopefully make it easier to reason about crashes on
canary.

Bug: chromium:1336516
Change-Id: I30bbabbc2a9186eaeac42c2963e7ae8dbb9fb527
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3707103Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81194}
parent c5efd19b
......@@ -24,7 +24,7 @@ uint32_t StringTableKey::hash() const {
return Name::HashBits::decode(raw_hash_field_);
}
int StringForwardingTable::Size() { return next_free_index_; }
int StringForwardingTable::Size() const { return next_free_index_; }
// static
uint32_t StringForwardingTable::BlockForIndex(int index,
......
......@@ -823,18 +823,18 @@ class StringForwardingTable::Block {
void operator delete(void* data);
void Set(int index, String string, String forward_to) {
DCHECK_LT(index, capacity());
CHECK_LT(index, capacity());
Set(IndexOfOriginalString(index), string);
Set(IndexOfForwardString(index), forward_to);
}
String GetOriginalString(Isolate* isolate, int index) const {
DCHECK_LT(index, capacity());
CHECK_LT(index, capacity());
return String::cast(Get(isolate, IndexOfOriginalString(index)));
}
String GetForwardString(Isolate* isolate, int index) const {
DCHECK_LT(index, capacity());
CHECK_LT(index, capacity());
return String::cast(Get(isolate, IndexOfForwardString(index)));
}
......@@ -944,17 +944,17 @@ class StringForwardingTable::BlockVector {
size_t capacity() const { return capacity_; }
Block* LoadBlock(size_t index, AcquireLoadTag) {
DCHECK_LT(index, size());
CHECK_LT(index, size());
return base::AsAtomicPointer::Acquire_Load(&begin_[index]);
}
Block* LoadBlock(size_t index) {
DCHECK_LT(index, size());
CHECK_LT(index, size());
return begin_[index];
}
void AddBlock(std::unique_ptr<Block> block) {
DCHECK_LT(size(), capacity());
CHECK_LT(size(), capacity());
base::AsAtomicPointer::Release_Store(&begin_[size_], block.release());
size_++;
}
......@@ -1061,6 +1061,8 @@ int StringForwardingTable::Add(Isolate* isolate, String string,
String StringForwardingTable::GetForwardString(Isolate* isolate,
int index) const {
// TODO(chromium:1336516): Change to DCHECK.
CHECK_LT(index, Size());
uint32_t index_in_block;
const uint32_t block = BlockForIndex(index, &index_in_block);
Block* data =
......
......@@ -116,7 +116,7 @@ class StringForwardingTable {
explicit StringForwardingTable(Isolate* isolate);
~StringForwardingTable();
inline int Size();
inline int Size() const;
// Returns the index of the added string pair.
int Add(Isolate* isolate, String string, String forward_to);
String GetForwardString(Isolate* isolate, int index) const;
......
......@@ -1508,10 +1508,14 @@ uint32_t String::ComputeAndSetHash(
// ComputeAndSetHash in parallel. Since only flat strings are in-place
// internalizable and their contents do not change, the result hash is the
// same. The raw hash field is stored with relaxed ordering.
DCHECK_IMPLIES(!FLAG_shared_string_table, !HasHashCode());
// TODO(chromium:1336516): Change to DCHECK.
CHECK_IMPLIES(!FLAG_shared_string_table, !HasHashCode());
uint32_t field = raw_hash_field(kAcquireLoad);
if (Name::IsForwardingIndex(field)) {
// TODO(chromium:1336516): Temporary CHECK to catch potential crashes
// earlier.
CHECK(FLAG_shared_string_table || FLAG_always_use_string_forwarding_table);
// Get the real hash from the forwarded string.
Isolate* isolate = GetIsolateFromWritableObject(*this);
const int forward_index = Name::HashBits::decode(field);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment