From 1691b1f6295165b2de6a10c8c729a8684baed39a Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Wed, 10 Mar 2021 20:28:23 +0100 Subject: [PATCH] [cleanup] Make InstructionStream::TryLookupCode() return builtin ID ... instead of Code. This is useful because usually the callers are interested in having just a builtin ID but not the Code object. This CL also makes Builtins::kNoBuiltinId a part of the Builtins::Name enum. Bug: v8:11527 Change-Id: I501e3e52dccc73cc7800f271939e0bf9fd00a975 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2749635 Reviewed-by: Jakob Gruber Reviewed-by: Ulan Degenbaev Commit-Queue: Igor Sheludko Cr-Commit-Position: refs/heads/master@{#73331} --- src/builtins/builtins.cc | 4 ++-- src/builtins/builtins.h | 7 ++++--- src/codegen/reloc-info.cc | 3 ++- src/deoptimizer/deoptimizer.cc | 7 +++---- src/heap/heap.cc | 14 +++++++++++--- src/snapshot/embedded/embedded-data.cc | 9 +++++---- src/snapshot/embedded/embedded-data.h | 5 +++-- src/snapshot/serializer.cc | 7 ++++--- 8 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/builtins/builtins.cc b/src/builtins/builtins.cc index 1e94dac811..4e9a770a97 100644 --- a/src/builtins/builtins.cc +++ b/src/builtins/builtins.cc @@ -106,8 +106,8 @@ void Builtins::TearDown() { initialized_ = false; } const char* Builtins::Lookup(Address pc) { // Off-heap pc's can be looked up through binary search. - Code maybe_builtin = InstructionStream::TryLookupCode(isolate_, pc); - if (!maybe_builtin.is_null()) return name(maybe_builtin.builtin_index()); + Builtins::Name builtin = InstructionStream::TryLookupCode(isolate_, pc); + if (Builtins::IsBuiltinId(builtin)) return name(builtin); // May be called during initialization (disassembler). if (initialized_) { diff --git a/src/builtins/builtins.h b/src/builtins/builtins.h index 7bb957bb66..95f7728a3f 100644 --- a/src/builtins/builtins.h +++ b/src/builtins/builtins.h @@ -49,6 +49,7 @@ class Builtins { const char* Lookup(Address pc); enum Name : int32_t { + kNoBuiltinId = -1, #define DEF_ENUM(Name, ...) k##Name, BUILTIN_LIST(DEF_ENUM, DEF_ENUM, DEF_ENUM, DEF_ENUM, DEF_ENUM, DEF_ENUM, DEF_ENUM) @@ -62,8 +63,6 @@ class Builtins { #undef EXTRACT_NAME }; - static const int32_t kNoBuiltinId = -1; - static constexpr int kFirstWideBytecodeHandler = kFirstBytecodeHandler + kNumberOfBytecodeHandlers; static constexpr int kFirstExtraWideBytecodeHandler = @@ -73,7 +72,9 @@ class Builtins { STATIC_ASSERT(kLastBytecodeHandlerPlusOne == builtin_count); static constexpr bool IsBuiltinId(int maybe_id) { - return 0 <= maybe_id && maybe_id < builtin_count; + STATIC_ASSERT(kNoBuiltinId == -1); + return static_cast(maybe_id) < + static_cast(builtin_count); } // The different builtin kinds are documented in builtins-definitions.h. diff --git a/src/codegen/reloc-info.cc b/src/codegen/reloc-info.cc index 25b3daef8e..5ccf82e67e 100644 --- a/src/codegen/reloc-info.cc +++ b/src/codegen/reloc-info.cc @@ -526,7 +526,8 @@ void RelocInfo::Verify(Isolate* isolate) { case OFF_HEAP_TARGET: { Address addr = target_off_heap_target(); CHECK_NE(addr, kNullAddress); - CHECK(!InstructionStream::TryLookupCode(isolate, addr).is_null()); + CHECK(Builtins::IsBuiltinId( + InstructionStream::TryLookupCode(isolate, addr))); break; } case RUNTIME_ENTRY: diff --git a/src/deoptimizer/deoptimizer.cc b/src/deoptimizer/deoptimizer.cc index be647a1f01..8d25695ef4 100644 --- a/src/deoptimizer/deoptimizer.cc +++ b/src/deoptimizer/deoptimizer.cc @@ -662,11 +662,10 @@ Builtins::Name Deoptimizer::GetDeoptimizationEntry(DeoptimizeKind kind) { bool Deoptimizer::IsDeoptimizationEntry(Isolate* isolate, Address addr, DeoptimizeKind* type_out) { - Code maybe_code = InstructionStream::TryLookupCode(isolate, addr); - if (maybe_code.is_null()) return false; + Builtins::Name builtin = InstructionStream::TryLookupCode(isolate, addr); + if (!Builtins::IsBuiltinId(builtin)) return false; - Code code = maybe_code; - switch (code.builtin_index()) { + switch (builtin) { case Builtins::kDeoptimizationEntry_Eager: *type_out = DeoptimizeKind::kEager; return true; diff --git a/src/heap/heap.cc b/src/heap/heap.cc index d5e9e93479..2c1b6788f3 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -6495,15 +6495,23 @@ Code Heap::GcSafeCastToCode(HeapObject object, Address inner_pointer) { bool Heap::GcSafeCodeContains(Code code, Address addr) { Map map = GcSafeMapOfCodeSpaceObject(code); DCHECK(map == ReadOnlyRoots(this).code_map()); - if (InstructionStream::TryLookupCode(isolate(), addr) == code) return true; + Builtins::Name maybe_builtin = + InstructionStream::TryLookupCode(isolate(), addr); + if (Builtins::IsBuiltinId(maybe_builtin) && + code.builtin_index() == maybe_builtin) { + return true; + } Address start = code.address(); Address end = code.address() + code.SizeFromMap(map); return start <= addr && addr < end; } Code Heap::GcSafeFindCodeForInnerPointer(Address inner_pointer) { - Code code = InstructionStream::TryLookupCode(isolate(), inner_pointer); - if (!code.is_null()) return code; + Builtins::Name maybe_builtin = + InstructionStream::TryLookupCode(isolate(), inner_pointer); + if (Builtins::IsBuiltinId(maybe_builtin)) { + return builtin(maybe_builtin); + } if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) { Address start = tp_heap_->GetObjectFromInnerPointer(inner_pointer); diff --git a/src/snapshot/embedded/embedded-data.cc b/src/snapshot/embedded/embedded-data.cc index 03702bf331..c6a36472a4 100644 --- a/src/snapshot/embedded/embedded-data.cc +++ b/src/snapshot/embedded/embedded-data.cc @@ -21,11 +21,12 @@ bool InstructionStream::PcIsOffHeap(Isolate* isolate, Address pc) { } // static -Code InstructionStream::TryLookupCode(Isolate* isolate, Address address) { - if (!PcIsOffHeap(isolate, address)) return Code(); +Builtins::Name InstructionStream::TryLookupCode(Isolate* isolate, + Address address) { + if (!PcIsOffHeap(isolate, address)) return Builtins::kNoBuiltinId; EmbeddedData d = EmbeddedData::FromBlob(); - if (address < d.InstructionStartOfBuiltin(0)) return Code(); + if (address < d.InstructionStartOfBuiltin(0)) return Builtins::kNoBuiltinId; // Note: Addresses within the padding section between builtins (i.e. within // start + size <= address < start + padded_size) are interpreted as belonging @@ -42,7 +43,7 @@ Code InstructionStream::TryLookupCode(Isolate* isolate, Address address) { } else if (address >= end) { l = mid + 1; } else { - return isolate->builtins()->builtin(mid); + return static_cast(mid); } } diff --git a/src/snapshot/embedded/embedded-data.h b/src/snapshot/embedded/embedded-data.h index d8d2dd822d..821707dc8d 100644 --- a/src/snapshot/embedded/embedded-data.h +++ b/src/snapshot/embedded/embedded-data.h @@ -23,8 +23,9 @@ class InstructionStream final : public AllStatic { // Returns true, iff the given pc points into an off-heap instruction stream. static bool PcIsOffHeap(Isolate* isolate, Address pc); - // Returns the corresponding Code object if it exists, and nullptr otherwise. - static Code TryLookupCode(Isolate* isolate, Address address); + // Returns the corresponding builtin ID if lookup succeeds, and kNoBuiltinId + // otherwise. + static Builtins::Name TryLookupCode(Isolate* isolate, Address address); // During snapshot creation, we first create an executable off-heap area // containing all off-heap code. The area is guaranteed to be contiguous. diff --git a/src/snapshot/serializer.cc b/src/snapshot/serializer.cc index 720ffbe741..89c5485d62 100644 --- a/src/snapshot/serializer.cc +++ b/src/snapshot/serializer.cc @@ -1003,11 +1003,12 @@ void Serializer::ObjectSerializer::VisitOffHeapTarget(Code host, Address addr = rinfo->target_off_heap_target(); CHECK_NE(kNullAddress, addr); - Code target = InstructionStream::TryLookupCode(isolate(), addr); - CHECK(Builtins::IsIsolateIndependentBuiltin(target)); + Builtins::Name builtin = InstructionStream::TryLookupCode(isolate(), addr); + CHECK(Builtins::IsBuiltinId(builtin)); + CHECK(Builtins::IsIsolateIndependent(builtin)); sink_->Put(kOffHeapTarget, "OffHeapTarget"); - sink_->PutInt(target.builtin_index(), "builtin index"); + sink_->PutInt(builtin, "builtin index"); } void Serializer::ObjectSerializer::VisitCodeTarget(Code host,