Commit cc6d54c3 authored by Mike Stanton's avatar Mike Stanton Committed by V8 LUCI CQ

[compiler] Fix invalid MakeRef use in JSArrayRef::length_unsafe()

Since we are reading an Object field, it could be that the gc
predicate fails. Therefore, this CL changes to TryMakeRef, and
makes the return value of length_unsafe() optional.

Bug: v8:7790, v8:12282
Change-Id: I86a8bcc6649d5e8121e52f8947b8331fcf242887
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3200078Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77209}
parent 75c130a8
......@@ -2456,13 +2456,14 @@ ObjectRef JSArrayRef::GetBoilerplateLength() const {
// Safe to read concurrently because:
// - boilerplates are immutable after initialization.
// - boilerplates are published into the feedback vector.
return length_unsafe();
// These facts also mean we can expect a valid value.
return length_unsafe().value();
}
ObjectRef JSArrayRef::length_unsafe() const {
base::Optional<ObjectRef> JSArrayRef::length_unsafe() const {
if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
return MakeRef(broker(),
object()->length(broker()->isolate(), kRelaxedLoad));
return TryMakeRef(broker(),
object()->length(broker()->isolate(), kRelaxedLoad));
} else {
return ObjectRef{broker(), data()->AsJSArray()->length()};
}
......@@ -2491,14 +2492,16 @@ base::Optional<ObjectRef> JSArrayRef::GetOwnCowElement(
// `elements`. We rely on the invariant that any `length` change will
// also result in an `elements` change to make this safe. The `elements`
// consistency check in the caller thus also guards the value of `length`.
ObjectRef length_ref = length_unsafe();
base::Optional<ObjectRef> length_ref = length_unsafe();
if (!length_ref.has_value()) return {};
// Likewise we only deal with smi lengths.
if (!length_ref.IsSmi()) return {};
if (!length_ref->IsSmi()) return {};
base::Optional<Object> result = ConcurrentLookupIterator::TryGetOwnCowElement(
broker()->isolate(), *elements_ref.AsFixedArray().object(), elements_kind,
length_ref.AsSmi(), index);
length_ref->AsSmi(), index);
if (!result.has_value()) return {};
return TryMakeRef(broker(), result.value());
......
......@@ -866,8 +866,9 @@ class JSArrayRef : public JSObjectRef {
// The `JSArray::length` property; not safe to use in general, but can be
// used in some special cases that guarantee a valid `length` value despite
// concurrent reads.
ObjectRef length_unsafe() const;
// concurrent reads. The result needs to be optional in case the
// return value was created too recently to pass the gc predicate.
base::Optional<ObjectRef> length_unsafe() const;
};
class ScopeInfoRef : public HeapObjectRef {
......
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