LibJS: Skip caching get_by_id() if object's shape is changed by a getter
Some checks are pending
CI / Lagom (arm64, Sanitizer_CI, false, macos-15, macOS, Clang) (push) Waiting to run
CI / Lagom (x86_64, Fuzzers_CI, false, ubuntu-24.04, Linux, Clang) (push) Waiting to run
CI / Lagom (x86_64, Sanitizer_CI, false, ubuntu-24.04, Linux, GNU) (push) Waiting to run
CI / Lagom (x86_64, Sanitizer_CI, true, ubuntu-24.04, Linux, Clang) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (arm64, macos-15, macOS, macOS-universal2) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (x86_64, ubuntu-24.04, Linux, Linux-x86_64) (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

Fixes a bug that reproduces with the following steps:
1. Create an object with a getter for property "a" in its prototype,
   where the getter adds an "a" property to the object itself.
2. Call the "a" getter in a loop for the first time. This triggers
   caching of metadata indicating that the "a" property is located in
   the prototype chain.
3. Call the "a" getter in a loop for the second time. Oops, the cache
   says the getter is in the prototype chain, but the object now
   also has its own "a" property that was added by the first getter
   call.
This commit is contained in:
Aliaksandr Kalenik 2025-05-20 23:42:17 +03:00 committed by Alexander Kalenik
parent 7af188dc52
commit 95e1ec4abc
Notes: github-actions[bot] 2025-05-20 23:11:54 +00:00
2 changed files with 56 additions and 17 deletions

View file

@ -1022,23 +1022,28 @@ inline ThrowCompletionOr<Value> get_by_id(VM& vm, Optional<IdentifierTableIndex>
CacheablePropertyMetadata cacheable_metadata;
auto value = TRY(base_obj->internal_get(executable.get_identifier(property), this_value, &cacheable_metadata));
auto get_cache_slot = [&] -> PropertyLookupCache::Entry& {
for (size_t i = cache.entries.size() - 1; i >= 1; --i) {
cache.entries[i] = cache.entries[i - 1];
// If internal_get() caused object's shape change, we can no longer be sure
// that collected metadata is valid, e.g. if getter in prototype chain added
// property with the same name into the object itself.
if (&shape == &base_obj->shape()) {
auto get_cache_slot = [&] -> PropertyLookupCache::Entry& {
for (size_t i = cache.entries.size() - 1; i >= 1; --i) {
cache.entries[i] = cache.entries[i - 1];
}
cache.entries[0] = {};
return cache.entries[0];
};
if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) {
auto& entry = get_cache_slot();
entry.shape = shape;
entry.property_offset = cacheable_metadata.property_offset.value();
} else if (cacheable_metadata.type == CacheablePropertyMetadata::Type::InPrototypeChain) {
auto& entry = get_cache_slot();
entry.shape = &base_obj->shape();
entry.property_offset = cacheable_metadata.property_offset.value();
entry.prototype = *cacheable_metadata.prototype;
entry.prototype_chain_validity = *cacheable_metadata.prototype->shape().prototype_chain_validity();
}
cache.entries[0] = {};
return cache.entries[0];
};
if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) {
auto& entry = get_cache_slot();
entry.shape = shape;
entry.property_offset = cacheable_metadata.property_offset.value();
} else if (cacheable_metadata.type == CacheablePropertyMetadata::Type::InPrototypeChain) {
auto& entry = get_cache_slot();
entry.shape = &base_obj->shape();
entry.property_offset = cacheable_metadata.property_offset.value();
entry.prototype = *cacheable_metadata.prototype;
entry.prototype_chain_validity = *cacheable_metadata.prototype->shape().prototype_chain_validity();
}
return value;
@ -1227,6 +1232,7 @@ inline ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value thi
break;
}
case Op::PropertyKind::KeyValue: {
auto& shape = object->shape();
if (caches) {
for (auto& cache : caches->entries) {
if (cache.prototype) {
@ -1262,7 +1268,10 @@ inline ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value thi
CacheablePropertyMetadata cacheable_metadata;
bool succeeded = TRY(object->internal_set(name, value, this_value, &cacheable_metadata));
if (succeeded && caches) {
// If internal_set() caused object's shape change, we can no longer be sure
// that collected metadata is valid, e.g. if setter in prototype chain added
// property with the same name into the object itself.
if (succeeded && caches && &shape == &object->shape()) {
auto get_cache_slot = [&] -> PropertyLookupCache::Entry& {
for (size_t i = caches->entries.size() - 1; i >= 1; --i) {
caches->entries[i] = caches->entries[i - 1];