diff --git a/src/execution/frames.cc b/src/execution/frames.cc index 99d35408ef..49dc88f874 100644 --- a/src/execution/frames.cc +++ b/src/execution/frames.cc @@ -582,37 +582,39 @@ Code StackFrame::LookupCode() const { void StackFrame::IteratePc(RootVisitor* v, Address* pc_address, Address* constant_pool_address, GcSafeCode holder) const { - if (!holder.has_instruction_stream()) { - // The embedded builtins are immovable so there's no need to update PCs on - // the stack. Just visit the Code object. - Object code = holder.UnsafeCastToCode(); - v->VisitRunningCode(FullObjectSlot(&code)); + const Address old_pc = ReadPC(pc_address); + DCHECK_GE(old_pc, holder.InstructionStart(isolate(), old_pc)); + DCHECK_LT(old_pc, holder.InstructionEnd(isolate(), old_pc)); + + // Keep the old pc offset before visiting the code since we need it to + // calculate the new pc after a potential InstructionStream move. + // It's okay to use raw_instruction_start() here since `pc_offset_from_start` + // will only be used if `holder` is not a builtin. + const uintptr_t pc_offset_from_start = + old_pc - holder.raw_instruction_start(); + + // Visit. + GcSafeCode visited_holder = holder; + const Object old_istream = holder.raw_instruction_stream(); + Object visited_istream = old_istream; + v->VisitRunningCode(FullObjectSlot{&visited_holder}, + FullObjectSlot{&visited_istream}); + if (visited_istream == old_istream) { + // Note this covers two important cases: + // 1. the associated InstructionStream object did not move, and + // 2. `holder` is an embedded builtin and has no InstructionStream. return; } - InstructionStream unsafe_istream = InstructionStream::unchecked_cast( - holder.UnsafeCastToCode().raw_instruction_stream()); + DCHECK(visited_holder.has_instruction_stream()); - // Keep the old pc (offset) before visiting (and potentially moving) the - // InstructionStream. - const Address old_pc = ReadPC(pc_address); - DCHECK(isolate_->heap()->GcSafeInstructionStreamContains(unsafe_istream, - old_pc)); - const uintptr_t pc_offset = old_pc - unsafe_istream.instruction_start(); - - // Visit the InstructionStream. - Object visited_unsafe_istream = unsafe_istream; - v->VisitRunningCode(FullObjectSlot(&visited_unsafe_istream)); - if (visited_unsafe_istream == unsafe_istream) return; - - // Take care when accessing unsafe_istream from here on. It may have been - // moved. - unsafe_istream = InstructionStream::unchecked_cast(visited_unsafe_istream); - const Address pc = unsafe_istream.instruction_start() + pc_offset; + InstructionStream istream = + InstructionStream::unchecked_cast(visited_istream); + const Address new_pc = istream.instruction_start() + pc_offset_from_start; // TODO(v8:10026): avoid replacing a signed pointer. - PointerAuthentication::ReplacePC(pc_address, pc, kSystemPointerSize); + PointerAuthentication::ReplacePC(pc_address, new_pc, kSystemPointerSize); if (V8_EMBEDDED_CONSTANT_POOL_BOOL && constant_pool_address != nullptr) { - *constant_pool_address = unsafe_istream.constant_pool(); + *constant_pool_address = istream.constant_pool(); } } diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 1021b19f0b..22352548eb 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -4775,10 +4775,13 @@ class ClientRootVisitor : public RootVisitor { actual_visitor_->VisitRootPointers(root, description, start, end); } - void VisitRunningCode(FullObjectSlot slot) final { + void VisitRunningCode(FullObjectSlot code_slot, + FullObjectSlot maybe_istream_slot) final { #if DEBUG - HeapObject object = HeapObject::cast(*slot); - DCHECK(!object.InSharedWritableHeap()); + DCHECK(!HeapObject::cast(*code_slot).InSharedWritableHeap()); + Object maybe_istream = *maybe_istream_slot; + DCHECK(maybe_istream == Smi::zero() || + !HeapObject::cast(maybe_istream).InSharedWritableHeap()); #endif } diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 528f3d3210..28071ac334 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -1093,39 +1093,24 @@ class MarkCompactCollector::RootMarkingVisitor final : public RootVisitor { } } - void VisitRunningCode(FullObjectSlot p) final { - // If Code is currently executing, then we must not remove its - // deoptimization literals, which it might need in order to successfully - // deoptimize. - // - // Must match behavior in RootsReferencesExtractor::VisitRunningCode, so - // that heap snapshots accurately describe the roots. - HeapObject value = HeapObject::cast(*p); - if (!IsCodeSpaceObject(value)) { - // The slot might contain a Code object representing an - // embedded builtin, which doesn't require additional processing. - DCHECK(!Code::cast(value).has_instruction_stream()); - } else { - InstructionStream code = InstructionStream::cast(value); - if (code.kind() != CodeKind::BASELINE) { - DeoptimizationData deopt_data = - DeoptimizationData::cast(code.deoptimization_data()); - if (deopt_data.length() > 0) { - DeoptimizationLiteralArray literals = deopt_data.LiteralArray(); - int literals_length = literals.length(); - for (int i = 0; i < literals_length; ++i) { - MaybeObject maybe_literal = literals.Get(i); - HeapObject heap_literal; - if (maybe_literal.GetHeapObject(&heap_literal)) { - MarkObjectByPointer(Root::kStackRoots, - FullObjectSlot(&heap_literal)); - } - } - } - } + // Keep this synced with RootsReferencesExtractor::VisitRunningCode. + void VisitRunningCode(FullObjectSlot code_slot, + FullObjectSlot istream_or_smi_zero_slot) final { + Object istream_or_smi_zero = *istream_or_smi_zero_slot; + DCHECK(istream_or_smi_zero == Smi::zero() || + istream_or_smi_zero.IsInstructionStream()); + DCHECK_EQ(Code::cast(*code_slot).raw_instruction_stream(), + istream_or_smi_zero); + + if (istream_or_smi_zero != Smi::zero()) { + InstructionStream istream = InstructionStream::cast(istream_or_smi_zero); + // We must not remove deoptimization literals which may be needed in + // order to successfully deoptimize. + istream.IterateDeoptimizationLiterals(this); + VisitRootPointer(Root::kStackRoots, nullptr, istream_or_smi_zero_slot); } - // And then mark the InstructionStream itself. - VisitRootPointer(Root::kStackRoots, nullptr, p); + + VisitRootPointer(Root::kStackRoots, nullptr, code_slot); } private: diff --git a/src/objects/code-inl.h b/src/objects/code-inl.h index c571c0d035..20cd3ba7ec 100644 --- a/src/objects/code-inl.h +++ b/src/objects/code-inl.h @@ -1103,6 +1103,24 @@ bool InstructionStream::IsWeakObjectInDeoptimizationLiteralArray( HeapObject::cast(object)); } +void InstructionStream::IterateDeoptimizationLiterals(RootVisitor* v) { + if (kind() == CodeKind::BASELINE) return; + + auto deopt_data = DeoptimizationData::cast(deoptimization_data()); + if (deopt_data.length() == 0) return; + + DeoptimizationLiteralArray literals = deopt_data.LiteralArray(); + const int literals_length = literals.length(); + for (int i = 0; i < literals_length; ++i) { + MaybeObject maybe_literal = literals.Get(i); + HeapObject heap_literal; + if (maybe_literal.GetHeapObject(&heap_literal)) { + v->VisitRootPointer(Root::kStackRoots, "deoptimization literal", + FullObjectSlot(&heap_literal)); + } + } +} + // This field has to have relaxed atomic accessors because it is accessed in the // concurrent marker. static_assert(FIELD_SIZE(Code::kKindSpecificFlagsOffset) == kInt32Size); diff --git a/src/objects/code.h b/src/objects/code.h index 671f2ecf84..769d399489 100644 --- a/src/objects/code.h +++ b/src/objects/code.h @@ -715,6 +715,8 @@ class InstructionStream : public HeapObject { static inline bool IsWeakObjectInDeoptimizationLiteralArray(Object object); + inline void IterateDeoptimizationLiterals(RootVisitor* v); + // Returns true if the function is inlined in the code. bool Inlines(SharedFunctionInfo sfi); diff --git a/src/objects/visitors.h b/src/objects/visitors.h index d28e9d6e81..4ae9ad2b26 100644 --- a/src/objects/visitors.h +++ b/src/objects/visitors.h @@ -91,12 +91,14 @@ class RootVisitor { UNREACHABLE(); } - // Visits a single pointer which is InstructionStream from the execution - // stack. - virtual void VisitRunningCode(FullObjectSlot p) { - // For most visitors, currently running InstructionStream is no different - // than any other on-stack pointer. - VisitRootPointer(Root::kStackRoots, nullptr, p); + // Visits a running Code object and potentially its associated + // InstructionStream from the execution stack. + virtual void VisitRunningCode(FullObjectSlot code_slot, + FullObjectSlot istream_or_smi_zero_slot) { + // For most visitors, currently running code is no different than any other + // on-stack pointer. + VisitRootPointer(Root::kStackRoots, nullptr, istream_or_smi_zero_slot); + VisitRootPointer(Root::kStackRoots, nullptr, code_slot); } // Intended for serialization/deserialization checking: insert, or diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc index 5bac4fff08..e82e8b4170 100644 --- a/src/profiler/heap-snapshot-generator.cc +++ b/src/profiler/heap-snapshot-generator.cc @@ -2064,37 +2064,17 @@ class RootsReferencesExtractor : public RootVisitor { } } - void VisitRunningCode(FullObjectSlot p) override { - // Must match behavior in - // MarkCompactCollector::RootMarkingVisitor::VisitRunningCode, which treats - // deoptimization literals in running code as stack roots. - HeapObject value = HeapObject::cast(*p); - if (!IsCodeSpaceObject(value)) { - // When external code space is enabled, the slot might contain a - // Code object representing an embedded builtin, which - // doesn't require additional processing. - DCHECK(!Code::cast(value).has_instruction_stream()); - } else { - InstructionStream code = InstructionStream::cast(value); - if (code.kind() != CodeKind::BASELINE) { - DeoptimizationData deopt_data = - DeoptimizationData::cast(code.deoptimization_data()); - if (deopt_data.length() > 0) { - DeoptimizationLiteralArray literals = deopt_data.LiteralArray(); - int literals_length = literals.length(); - for (int i = 0; i < literals_length; ++i) { - MaybeObject maybe_literal = literals.Get(i); - HeapObject heap_literal; - if (maybe_literal.GetHeapObject(&heap_literal)) { - VisitRootPointer(Root::kStackRoots, nullptr, - FullObjectSlot(&heap_literal)); - } - } - } - } + // Keep this synced with + // MarkCompactCollector::RootMarkingVisitor::VisitRunningCode. + void VisitRunningCode(FullObjectSlot code_slot, + FullObjectSlot istream_or_smi_zero_slot) final { + Object istream_or_smi_zero = *istream_or_smi_zero_slot; + if (istream_or_smi_zero != Smi::zero()) { + InstructionStream istream = InstructionStream::cast(istream_or_smi_zero); + istream.IterateDeoptimizationLiterals(this); + VisitRootPointer(Root::kStackRoots, nullptr, istream_or_smi_zero_slot); } - // Finally visit the InstructionStream itself. - VisitRootPointer(Root::kStackRoots, nullptr, p); + VisitRootPointer(Root::kStackRoots, nullptr, code_slot); } private: