[strings] Fix dictionary forwarded string hash lookup
Strings forwarded to external resources have their real hashes stored in the forwarding table. Dictionary mode lookups currently do not correctly load the hash for these tables, causing misses for properties that are in fact in the object. Bug: v8:12007 Change-Id: I60ca4c084db7ddf6d2b7f7be8f63519c9cf3bc73 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3935218 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Shu-yu Guo <syg@chromium.org> Reviewed-by: Patrick Thier <pthier@chromium.org> Cr-Commit-Position: refs/heads/main@{#83577}
This commit is contained in:
parent
1e124ff8e6
commit
ed8953b695
@ -8659,9 +8659,12 @@ void CodeStubAssembler::NameDictionaryLookup(
|
||||
Comment("NameDictionaryLookup");
|
||||
CSA_DCHECK(this, IsUniqueName(unique_name));
|
||||
|
||||
Label if_not_computed(this, Label::kDeferred);
|
||||
|
||||
TNode<IntPtrT> capacity = SmiUntag(GetCapacity<Dictionary>(dictionary));
|
||||
TNode<IntPtrT> mask = IntPtrSub(capacity, IntPtrConstant(1));
|
||||
TNode<UintPtrT> hash = ChangeUint32ToWord(LoadNameHash(unique_name));
|
||||
TNode<UintPtrT> hash =
|
||||
ChangeUint32ToWord(LoadNameHash(unique_name, &if_not_computed));
|
||||
|
||||
// See Dictionary::FirstProbe().
|
||||
TNode<IntPtrT> count = IntPtrConstant(0);
|
||||
@ -8708,6 +8711,52 @@ void CodeStubAssembler::NameDictionaryLookup(
|
||||
var_entry = entry;
|
||||
Goto(&loop);
|
||||
}
|
||||
|
||||
BIND(&if_not_computed);
|
||||
{
|
||||
// Strings will only have the forwarding index with experimental shared
|
||||
// memory features turned on. To minimize affecting the fast path, the
|
||||
// forwarding index branch defers both fetching the actual hash value and
|
||||
// the dictionary lookup to the runtime.
|
||||
ExternalReference func_ref;
|
||||
if constexpr (std::is_same<Dictionary, NameDictionary>::value) {
|
||||
func_ref =
|
||||
mode == kFindExisting
|
||||
? ExternalReference::name_dictionary_lookup_forwarded_string()
|
||||
: ExternalReference::
|
||||
name_dictionary_find_insertion_entry_forwarded_string();
|
||||
} else if constexpr (std::is_same<Dictionary, GlobalDictionary>::value) {
|
||||
func_ref =
|
||||
mode == kFindExisting
|
||||
? ExternalReference::global_dictionary_lookup_forwarded_string()
|
||||
: ExternalReference::
|
||||
global_dictionary_find_insertion_entry_forwarded_string();
|
||||
} else {
|
||||
func_ref =
|
||||
mode == kFindExisting
|
||||
? ExternalReference::
|
||||
name_to_index_hashtable_lookup_forwarded_string()
|
||||
: ExternalReference::
|
||||
name_to_index_hashtable_find_insertion_entry_forwarded_string();
|
||||
}
|
||||
const TNode<ExternalReference> function = ExternalConstant(func_ref);
|
||||
const TNode<ExternalReference> isolate_ptr =
|
||||
ExternalConstant(ExternalReference::isolate_address(isolate()));
|
||||
TNode<IntPtrT> entry = UncheckedCast<IntPtrT>(CallCFunction(
|
||||
function, MachineType::IntPtr(),
|
||||
std::make_pair(MachineType::Pointer(), isolate_ptr),
|
||||
std::make_pair(MachineType::TaggedPointer(), dictionary),
|
||||
std::make_pair(MachineType::TaggedPointer(), unique_name)));
|
||||
if (var_name_index) *var_name_index = EntryToIndex<Dictionary>(entry);
|
||||
if (mode == kFindExisting) {
|
||||
GotoIf(IntPtrEqual(entry,
|
||||
IntPtrConstant(InternalIndex::NotFound().raw_value())),
|
||||
if_not_found);
|
||||
Goto(if_found);
|
||||
} else {
|
||||
Goto(if_not_found);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Instantiate template methods to workaround GCC compilation issue.
|
||||
|
@ -1018,6 +1018,50 @@ static uint32_t ComputeSeededIntegerHash(Isolate* isolate, int32_t key) {
|
||||
}
|
||||
|
||||
FUNCTION_REFERENCE(compute_integer_hash, ComputeSeededIntegerHash)
|
||||
|
||||
enum LookupMode { kFindExisting, kFindInsertionEntry };
|
||||
template <typename Dictionary, LookupMode mode>
|
||||
static size_t NameDictionaryLookupForwardedString(Isolate* isolate,
|
||||
Address raw_dict,
|
||||
Address raw_key) {
|
||||
// This function cannot allocate, but there is a HandleScope because it needs
|
||||
// to pass Handle<Name> to the dictionary methods.
|
||||
DisallowGarbageCollection no_gc;
|
||||
HandleScope handle_scope(isolate);
|
||||
|
||||
Handle<String> key(String::cast(Object(raw_key)), isolate);
|
||||
// This function should only be used as the slow path for forwarded strings.
|
||||
DCHECK(Name::IsForwardingIndex(key->raw_hash_field()));
|
||||
|
||||
Dictionary dict = Dictionary::cast(Object(raw_dict));
|
||||
ReadOnlyRoots roots(isolate);
|
||||
uint32_t hash = key->hash();
|
||||
InternalIndex entry = mode == kFindExisting
|
||||
? dict.FindEntry(isolate, roots, key, hash)
|
||||
: dict.FindInsertionEntry(isolate, roots, hash);
|
||||
return entry.raw_value();
|
||||
}
|
||||
|
||||
FUNCTION_REFERENCE(
|
||||
name_dictionary_lookup_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<NameDictionary, kFindExisting>))
|
||||
FUNCTION_REFERENCE(
|
||||
name_dictionary_find_insertion_entry_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<NameDictionary, kFindInsertionEntry>))
|
||||
FUNCTION_REFERENCE(
|
||||
global_dictionary_lookup_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<GlobalDictionary, kFindExisting>))
|
||||
FUNCTION_REFERENCE(global_dictionary_find_insertion_entry_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<GlobalDictionary,
|
||||
kFindInsertionEntry>))
|
||||
FUNCTION_REFERENCE(
|
||||
name_to_index_hashtable_lookup_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<NameToIndexHashTable, kFindExisting>))
|
||||
FUNCTION_REFERENCE(
|
||||
name_to_index_hashtable_find_insertion_entry_forwarded_string,
|
||||
(NameDictionaryLookupForwardedString<NameToIndexHashTable,
|
||||
kFindInsertionEntry>))
|
||||
|
||||
FUNCTION_REFERENCE(copy_fast_number_jsarray_elements_to_typed_array,
|
||||
CopyFastNumberJSArrayElementsToTypedArray)
|
||||
FUNCTION_REFERENCE(copy_typed_array_elements_to_typed_array,
|
||||
|
@ -211,6 +211,18 @@ class StatsCounter;
|
||||
"try_string_to_index_or_lookup_existing") \
|
||||
V(string_from_forward_table, "string_from_forward_table") \
|
||||
V(raw_hash_from_forward_table, "raw_hash_from_forward_table") \
|
||||
V(name_dictionary_lookup_forwarded_string, \
|
||||
"name_dictionary_lookup_forwarded_string") \
|
||||
V(name_dictionary_find_insertion_entry_forwarded_string, \
|
||||
"name_dictionary_find_insertion_entry_forwarded_string") \
|
||||
V(global_dictionary_lookup_forwarded_string, \
|
||||
"global_dictionary_lookup_forwarded_string") \
|
||||
V(global_dictionary_find_insertion_entry_forwarded_string, \
|
||||
"global_dictionary_find_insertion_entry_forwarded_string") \
|
||||
V(name_to_index_hashtable_lookup_forwarded_string, \
|
||||
"name_to_index_hashtable_lookup_forwarded_string") \
|
||||
V(name_to_index_hashtable_find_insertion_entry_forwarded_string, \
|
||||
"name_to_index_hashtable_find_insertion_entry_forwarded_string") \
|
||||
IF_WASM(V, wasm_call_trap_callback_for_testing, \
|
||||
"wasm::call_trap_callback_for_testing") \
|
||||
IF_WASM(V, wasm_f32_ceil, "wasm::f32_ceil_wrapper") \
|
||||
|
@ -172,6 +172,10 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) BaseNameDictionary
|
||||
Handle<Object> value, PropertyDetails details,
|
||||
InternalIndex* entry_out = nullptr);
|
||||
|
||||
// Exposed for NameDictionaryLookupForwardedString slow path for forwarded
|
||||
// strings.
|
||||
using Dictionary<Derived, Shape>::FindInsertionEntry;
|
||||
|
||||
OBJECT_CONSTRUCTORS(BaseNameDictionary, Dictionary<Derived, Shape>);
|
||||
};
|
||||
|
||||
|
@ -470,6 +470,10 @@ class V8_EXPORT_PRIVATE NameToIndexHashTable
|
||||
Handle<NameToIndexHashTable> table,
|
||||
Handle<Name> key, int32_t value);
|
||||
|
||||
// Exposed for NameDictionaryLookupForwardedString slow path for forwarded
|
||||
// strings.
|
||||
using HashTable<NameToIndexHashTable, NameToIndexShape>::FindInsertionEntry;
|
||||
|
||||
DECL_CAST(NameToIndexHashTable)
|
||||
DECL_PRINTER(NameToIndexHashTable)
|
||||
|
||||
|
@ -0,0 +1,38 @@
|
||||
// Copyright 2022 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: --expose-externalize-string --shared-string-table
|
||||
|
||||
const long_key = 'key1234567890abcdefg';
|
||||
const substr_key = long_key.substring(3,17);
|
||||
const consstr_key = 'key' + 1234567890 + 'abcdefg';
|
||||
const integer_index = long_key.substring(3,8);
|
||||
|
||||
{
|
||||
let obj = [];
|
||||
for (let i = 0; i < 100; ++i) {
|
||||
obj[i] = i;
|
||||
obj['XXX' + i] = 'XXX' + i;
|
||||
}
|
||||
|
||||
obj['key1234567890abcdefg'] = 'long_key_value';
|
||||
obj['1234567890abcd'] = 'substr_value';
|
||||
obj[12345] = 'integer_index';
|
||||
|
||||
try {
|
||||
externalizeString(long_key);
|
||||
externalizeString(substr_key);
|
||||
externalizeString(consstr_key);
|
||||
externalizeString(integer_index);
|
||||
} catch {}
|
||||
|
||||
(function exerciseICs() {
|
||||
for (let i = 0; i < 10; i++) {
|
||||
assertEquals('long_key_value', obj[long_key]);
|
||||
assertEquals('substr_value', obj[substr_key]);
|
||||
assertEquals('long_key_value', obj[consstr_key]);
|
||||
assertEquals('integer_index', obj[integer_index]);
|
||||
}
|
||||
})();
|
||||
}
|
Loading…
Reference in New Issue
Block a user