Commit 6e9f5de2 authored by Seth Brenith's avatar Seth Brenith Committed by V8 LUCI CQ

Fix crashes introduced by "Fix leaks due to deoptimization literals"

My previous change https://crrev.com/c/3160299 introduced a runtime
CHECK that crashes the process if V8 attempts to read a deoptimization
literal which has been cleared. That CHECK is indeed crashing the
process.

It appears that the trouble arises in cases where the deoptimization
data indicates that an object should be materialized as needed. In those
cases, one of the deoptimization literals is the Map to use when
materializing the object. It is possible to reach a part of the code
that requires the materialized object, and therefore the Map, without
there being any other owner of that Map. This is in contrast to most
other deoptimization literals, which are logically equivalent to omitted
values from the stack frame and therefore can't be reached without a
real owner somewhere to keep them alive.

To fix, I propose referring to Maps strongly from the deoptimization
literals. The cases I investigated in v8:4578 didn't involve Maps, so I
believe that the observed memory leaks are still fixed with this change.

Bug: chromium:1268681, chromium:1268683, chromium:1268825, v8:12300
Change-Id: Ifd32a7f9cc29e0384650013ab16e05646bf57895
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3272880
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77857}
parent f7bc3333
......@@ -856,6 +856,15 @@ bool Code::IsWeakObjectInOptimizedCode(HeapObject object) {
InstanceTypeChecker::IsContext(instance_type);
}
bool Code::IsWeakObjectInDeoptimizationLiteralArray(Object object) {
// Maps must be strong because they can be used as part of the description for
// how to materialize an object upon deoptimization, in which case it is
// possible to reach the code that requires the Map without anything else
// holding a strong pointer to that Map.
return object.IsHeapObject() && !object.IsMap() &&
Code::IsWeakObjectInOptimizedCode(HeapObject::cast(object));
}
bool Code::IsExecutable() {
return !Builtins::IsBuiltinId(builtin_id()) || !is_off_heap_trampoline() ||
Builtins::CodeObjectIsExecutable(builtin_id());
......@@ -1214,8 +1223,7 @@ inline Object DeoptimizationLiteralArray::get(PtrComprCageBase cage_base,
inline void DeoptimizationLiteralArray::set(int index, Object value) {
MaybeObject maybe = MaybeObject::FromObject(value);
if (value.IsHeapObject() &&
Code::IsWeakObjectInOptimizedCode(HeapObject::cast(value))) {
if (Code::IsWeakObjectInDeoptimizationLiteralArray(value)) {
maybe = MaybeObject::MakeWeak(maybe);
}
Set(index, maybe);
......
......@@ -541,6 +541,8 @@ class Code : public HeapObject {
static inline bool IsWeakObjectInOptimizedCode(HeapObject object);
static inline bool IsWeakObjectInDeoptimizationLiteralArray(Object object);
// Returns false if this is an embedded builtin Code object that's in
// read_only_space and hence doesn't have execute permissions.
inline bool IsExecutable();
......
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