Refactor RootVisitor::VisitRunningCode

The contract of passing in a Code object for builtins and
InstructionStream objects for everything else was confusing. In this
CL we change it to:

 void VisitRunningCode(FullObjectSlot code_slot,
                       FullObjectSlot istream_or_smi_zero_slot)

where we *always* pass in both parts of the composite
{Code,InstructionStream} object. The istream_or_smi_zero_slot must
equal raw_instruction_stream() of the given code_slot. We pass in
both, because it is convenient at the single call site in frames.cc.

Drive-by: extract deopt literal iteration to a Code method.

Bug: v8:13654
Change-Id: I09d658fbd8d26bf483e1c778e566a53e1817f80f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4212399
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85619}
This commit is contained in:
Jakob Linke 2023-02-02 13:32:59 +01:00 committed by V8 LUCI CQ
parent 9b89942446
commit 135b63038d
7 changed files with 88 additions and 96 deletions

View File

@ -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();
}
}

View File

@ -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
}

View File

@ -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:

View File

@ -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);

View File

@ -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);

View File

@ -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

View File

@ -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: