[ext-code-space] Update loggers for handling Code-less builtins

Bug: v8:11880
Change-Id: I745caa10106870eb06526cccb8693797a36ba7bd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3825888Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82430}

src/builtins/builtins.cc

@@ -181,7 +181,7 @@ FullObjectSlot Builtins::builtin_tier0_slot(Builtin builtin) {
 
 void Builtins::set_code(Builtin builtin, CodeT code) {
   DCHECK_EQ(builtin, code.builtin_id());
-  if (V8_EXTERNAL_CODE_SPACE_BOOL) {
+  if (!V8_REMOVE_BUILTINS_CODE_OBJECTS && V8_EXTERNAL_CODE_SPACE_BOOL) {
     DCHECK_EQ(builtin, FromCodeT(code).builtin_id());
   }
   DCHECK(Internals::HasHeapObjectTag(code.ptr()));

src/codegen/compiler.cc

@@ -298,10 +298,11 @@ void Compiler::LogFunctionCompilation(Isolate* isolate,
                                       Handle<AbstractCode> abstract_code,
                                       CodeKind kind, double time_taken_ms) {
   DCHECK(!abstract_code.is_null());
-  if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-    DCHECK_NE(*abstract_code, FromCodeT(*BUILTIN_CODE(isolate, CompileLazy)));
-  } else {
   DCHECK(!abstract_code.is_identical_to(BUILTIN_CODE(isolate, CompileLazy)));
+  if (!V8_REMOVE_BUILTINS_CODE_OBJECTS && V8_EXTERNAL_CODE_SPACE_BOOL) {
+    // TODO(v8:11880): remove once AbstactCode representing an embedded builtin
+    // will contain CodeDataContainer.
+    DCHECK_NE(*abstract_code, FromCodeT(*BUILTIN_CODE(isolate, CompileLazy)));
   }
 
   // Log the code generation. If source information is available include
@@ -1157,8 +1158,8 @@ MaybeHandle<CodeT> CompileTurbofan(Isolate* isolate,
 void RecordMaglevFunctionCompilation(Isolate* isolate,
                                      Handle<JSFunction> function) {
   PtrComprCageBase cage_base(isolate);
-  Handle<AbstractCode> abstract_code(
-      AbstractCode::cast(FromCodeT(function->code(cage_base))), isolate);
+  Handle<AbstractCode> abstract_code(ToAbstractCode(function->code(cage_base)),
+                                     isolate);
   Handle<SharedFunctionInfo> shared(function->shared(cage_base), isolate);
   Handle<Script> script(Script::cast(shared->script(cage_base)), isolate);
   Handle<FeedbackVector> feedback_vector(function->feedback_vector(cage_base),

src/diagnostics/objects-debug.cc

@@ -1550,7 +1550,7 @@ void JSRegExp::JSRegExpVerify(Isolate* isolate) {
 
       bool is_compiled = latin1_code.IsCodeT();
       if (is_compiled) {
-        CHECK_EQ(FromCodeT(CodeT::cast(latin1_code)).builtin_id(),
+        CHECK_EQ(CodeT::cast(latin1_code).builtin_id(),
                  Builtin::kRegExpExperimentalTrampoline);
         CHECK_EQ(uc16_code, latin1_code);
 

src/diagnostics/objects-printer.cc

@@ -1756,12 +1756,17 @@ void Code::CodePrint(std::ostream& os) {
 
 void CodeDataContainer::CodeDataContainerPrint(std::ostream& os) {
   PrintHeader(os, "CodeDataContainer");
-  os << "\n - kind_specific_flags: " << kind_specific_flags(kRelaxedLoad);
-  if (V8_EXTERNAL_CODE_SPACE_BOOL) {
-    os << "\n - code: " << Brief(code());
+#ifdef V8_EXTERNAL_CODE_SPACE
+  os << "\n - kind: " << CodeKindToString(kind());
+  if (is_builtin()) {
+    os << "\n - builtin: " << Builtins::name(builtin_id());
+  }
+  os << "\n - is_off_heap_trampoline: " << is_off_heap_trampoline();
+  os << "\n - code: " << Brief(raw_code());
   os << "\n - code_entry_point: "
      << reinterpret_cast<void*>(code_entry_point());
-  }
+#endif  // V8_EXTERNAL_CODE_SPACE
+  os << "\n - kind_specific_flags: " << kind_specific_flags(kRelaxedLoad);
   os << "\n";
 }
 

src/heap/code-stats.cc

@@ -45,7 +45,7 @@ void CodeStatistics::RecordCodeAndMetadataStatistics(HeapObject object,
     CodeKind code_kind = abstract_code.kind(cage_base);
     isolate->code_kind_statistics()[static_cast<int>(code_kind)] +=
         abstract_code.Size(cage_base);
-    CodeStatistics::CollectCodeCommentStatistics(object, isolate);
+    CodeStatistics::CollectCodeCommentStatistics(abstract_code, isolate);
 #endif
   }
 }
@@ -197,14 +197,14 @@ void CodeStatistics::CollectCommentStatistics(Isolate* isolate,
 }
 
 // Collects code comment statistics.
-void CodeStatistics::CollectCodeCommentStatistics(HeapObject obj,
+void CodeStatistics::CollectCodeCommentStatistics(AbstractCode obj,
                                                   Isolate* isolate) {
-  // Bytecode objects do not contain RelocInfo. Only process code objects
-  // for code comment statistics.
-  if (!obj.IsCode()) {
-    DCHECK(obj.IsBytecodeArray());
-    return;
-  }
+  // Bytecode objects do not contain RelocInfo. Off-heap builtins might contain
+  // comments but they are a part of binary so it doesn't make sense to account
+  // them in the stats.
+  // Only process code objects for code comment statistics.
+  PtrComprCageBase cage_base(isolate);
+  if (!obj.IsCode(cage_base)) return;
 
   Code code = Code::cast(obj);
   CodeCommentsIterator cit(code.code_comments(), code.code_comments_size());

src/heap/code-stats.h

@@ -8,6 +8,7 @@
 namespace v8 {
 namespace internal {
 
+class AbstractCode;
 class CodeCommentsIterator;
 class HeapObject;
 class Isolate;
@@ -38,7 +39,7 @@ class CodeStatistics {
 #ifdef DEBUG
   static void CollectCommentStatistics(Isolate* isolate,
                                        CodeCommentsIterator* it);
-  static void CollectCodeCommentStatistics(HeapObject obj, Isolate* isolate);
+  static void CollectCodeCommentStatistics(AbstractCode obj, Isolate* isolate);
   static void EnterComment(Isolate* isolate, const char* comment, int delta);
   static void ResetCodeStatistics(Isolate* isolate);
 #endif

src/logging/log.cc

@@ -128,9 +128,8 @@ const char* ComputeMarker(SharedFunctionInfo shared, AbstractCode code) {
   // We record interpreter trampoline builtin copies as having the
   // "interpreted" marker.
   if (FLAG_interpreted_frames_native_stack && kind == CodeKind::BUILTIN &&
-      code.GetCode().is_interpreter_trampoline_builtin() &&
-      ToCodeT(code.GetCode()) !=
-          *BUILTIN_CODE(shared.GetIsolate(), InterpreterEntryTrampoline)) {
+      !code.is_off_heap_trampoline(cage_base)) {
+    DCHECK_EQ(code.builtin_id(cage_base), Builtin::kInterpreterEntryTrampoline);
     kind = CodeKind::INTERPRETED_FUNCTION;
   }
   if (shared.optimization_disabled() &&
@@ -449,6 +448,7 @@ ExternalLogEventListener::~ExternalLogEventListener() {
 void ExternalLogEventListener::LogExistingCode() {
   HandleScope scope(isolate_);
   ExistingCodeLogger logger(isolate_, this);
+  logger.LogBuiltins();
   logger.LogCodeObjects();
   logger.LogCompiledFunctions();
 }
@@ -1359,6 +1359,11 @@ void V8FileLogger::LogCodeDisassemble(Handle<AbstractCode> code) {
     if (code->IsCode(cage_base)) {
 #ifdef ENABLE_DISASSEMBLER
       Code::cast(*code).Disassemble(nullptr, stream, isolate_);
+#endif
+    } else if (V8_REMOVE_BUILTINS_CODE_OBJECTS &&
+               code->IsCodeDataContainer(cage_base)) {
+#ifdef ENABLE_DISASSEMBLER
+      CodeT::cast(*code).Disassemble(nullptr, stream, isolate_);
 #endif
     } else {
       BytecodeArray::cast(*code).Disassemble(stream);
@@ -2141,8 +2146,8 @@ void V8FileLogger::SetCodeEventHandler(uint32_t options,
     AddLogEventListener(jit_logger_.get());
     if (options & kJitCodeEventEnumExisting) {
       HandleScope scope(isolate_);
-      LogCodeObjects();
       LogBuiltins();
+      LogCodeObjects();
       LogCompiledFunctions();
     }
   }
@@ -2206,12 +2211,13 @@ void V8FileLogger::UpdateIsLogging(bool value) {
   isolate_->UpdateLogObjectRelocation();
 }
 
-void ExistingCodeLogger::LogCodeObject(Object object) {
+void ExistingCodeLogger::LogCodeObject(AbstractCode object) {
   HandleScope scope(isolate_);
-  Handle<AbstractCode> abstract_code(AbstractCode::cast(object), isolate_);
+  Handle<AbstractCode> abstract_code(object, isolate_);
   CodeTag tag = CodeTag::kStub;
   const char* description = "Unknown code from before profiling";
-  switch (abstract_code->kind(isolate_)) {
+  PtrComprCageBase cage_base(isolate_);
+  switch (abstract_code->kind(cage_base)) {
     case CodeKind::INTERPRETED_FUNCTION:
     case CodeKind::TURBOFAN:
     case CodeKind::BASELINE:
@@ -2227,17 +2233,18 @@ void ExistingCodeLogger::LogCodeObject(Object object) {
       break;
     case CodeKind::BYTECODE_HANDLER:
       description =
-          isolate_->builtins()->name(abstract_code->GetCode().builtin_id());
+          isolate_->builtins()->name(abstract_code->builtin_id(cage_base));
       tag = CodeTag::kBytecodeHandler;
       break;
     case CodeKind::BUILTIN:
-      if (Code::cast(object).is_interpreter_trampoline_builtin() &&
-          ToCodeT(Code::cast(object)) !=
-              *BUILTIN_CODE(isolate_, InterpreterEntryTrampoline)) {
+      if (!abstract_code->is_off_heap_trampoline(cage_base)) {
+        DCHECK_EQ(abstract_code->builtin_id(cage_base),
+                  Builtin::kInterpreterEntryTrampoline);
+        // We treat interpreter trampoline builtin copies as
+        // INTERPRETED_FUNCTION, which are logged using LogCompiledFunctions.
         return;
       }
-      description =
-          isolate_->builtins()->name(abstract_code->GetCode().builtin_id());
+      description = Builtins::name(abstract_code->builtin_id(cage_base));
       tag = CodeTag::kBuiltin;
       break;
     case CodeKind::WASM_FUNCTION:
@@ -2272,21 +2279,35 @@ void ExistingCodeLogger::LogCodeObjects() {
   Heap* heap = isolate_->heap();
   HeapObjectIterator iterator(heap);
   DisallowGarbageCollection no_gc;
+  PtrComprCageBase cage_base(isolate_);
   for (HeapObject obj = iterator.Next(); !obj.is_null();
        obj = iterator.Next()) {
-    if (obj.IsCode()) LogCodeObject(obj);
-    if (obj.IsBytecodeArray()) LogCodeObject(obj);
+    InstanceType instance_type = obj.map(cage_base).instance_type();
+    if (V8_REMOVE_BUILTINS_CODE_OBJECTS) {
+      // In this case AbstactCode is Code|CodeDataContainer|BytecodeArray but
+      // we want to log code objects only once, thus we ignore Code objects
+      // which will be logged via corresponding CodeDataContainer.
+      if (InstanceTypeChecker::IsCodeT(instance_type) ||
+          InstanceTypeChecker::IsBytecodeArray(instance_type)) {
+        LogCodeObject(AbstractCode::cast(obj));
+      }
+    } else {
+      // In this case AbstactCode is Code|BytecodeArray.
+      if (InstanceTypeChecker::IsCode(instance_type) ||
+          InstanceTypeChecker::IsBytecodeArray(instance_type)) {
+        LogCodeObject(AbstractCode::cast(obj));
+      }
+    }
   }
 }
 
 void ExistingCodeLogger::LogBuiltins() {
-  Builtins* builtins = isolate_->builtins();
-  DCHECK(builtins->is_initialized());
-  for (Builtin builtin = Builtins::kFirst; builtin <= Builtins::kLast;
-       ++builtin) {
-    Code code = FromCodeT(builtins->code(builtin));
-    LogCodeObject(code);
-  }
+  DCHECK(isolate_->builtins()->is_initialized());
+  // The main "copy" of used builtins are logged by LogCodeObjects() while
+  // iterating CodeT objects.
+  // TODO(v8:11880): Log other copies of remapped builtins once we
+  // decide to remap them multiple times into the code range (for example
+  // for arm64).
 }
 
 void ExistingCodeLogger::LogCompiledFunctions() {

src/logging/log.h

@@ -95,7 +95,7 @@ class ExistingCodeLogger {
   void LogExistingFunction(
       Handle<SharedFunctionInfo> shared, Handle<AbstractCode> code,
       LogEventListener::CodeTag tag = LogEventListener::CodeTag::kFunction);
-  void LogCodeObject(Object object);
+  void LogCodeObject(AbstractCode object);
 
  private:
   Isolate* isolate_;

src/objects/call-site-info-inl.h

@@ -35,6 +35,10 @@ DEF_GETTER(CallSiteInfo, code_object, HeapObject) {
   HeapObject value = TorqueGeneratedClass::code_object(cage_base);
   // The |code_object| field can contain many types of objects, but only CodeT
   // values have to be converted to Code.
+  if (V8_REMOVE_BUILTINS_CODE_OBJECTS) {
+    // In this mode the callers are fine with CodeT result.
+    return value;
+  }
   if (V8_EXTERNAL_CODE_SPACE_BOOL && value.IsCodeT()) {
     return FromCodeT(CodeT::cast(value));
   }
@@ -44,7 +48,7 @@ DEF_GETTER(CallSiteInfo, code_object, HeapObject) {
 void CallSiteInfo::set_code_object(HeapObject code, WriteBarrierMode mode) {
   // The |code_object| field can contain many types of objects, but only Code
   // values have to be converted to CodeT.
-  if (V8_EXTERNAL_CODE_SPACE_BOOL && code.IsCode()) {
+  if (V8_EXTERNAL_CODE_SPACE_BOOL && IsCodeSpaceObject(code)) {
     TorqueGeneratedClass::set_code_object(ToCodeT(Code::cast(code)), mode);
   } else {
     TorqueGeneratedClass::set_code_object(code, mode);

src/objects/code-inl.h

@@ -181,14 +181,13 @@ Builtin AbstractCode::builtin_id(PtrComprCageBase cage_base) {
   }
 }
 
-bool AbstractCode::is_interpreter_trampoline_builtin(
-    PtrComprCageBase cage_base) {
+bool AbstractCode::is_off_heap_trampoline(PtrComprCageBase cage_base) {
   InstanceType instance_type = map(cage_base).instance_type();
   if (InstanceTypeChecker::IsCode(instance_type)) {
-    return GetCode().is_interpreter_trampoline_builtin();
+    return GetCode().is_off_heap_trampoline();
   } else if (V8_REMOVE_BUILTINS_CODE_OBJECTS &&
              InstanceTypeChecker::IsCodeDataContainer(instance_type)) {
-    return GetCodeT().is_interpreter_trampoline_builtin();
+    return GetCodeT().is_off_heap_trampoline();
   } else {
     DCHECK(InstanceTypeChecker::IsBytecodeArray(instance_type));
     return false;
@@ -232,6 +231,20 @@ BytecodeArray AbstractCode::GetBytecodeArray() {
   return BytecodeArray::cast(*this);
 }
 
+Code AbstractCode::ToCode(PtrComprCageBase cage_base) {
+  InstanceType instance_type = map(cage_base).instance_type();
+  if (InstanceTypeChecker::IsCode(instance_type)) {
+    return GetCode();
+  } else if (V8_REMOVE_BUILTINS_CODE_OBJECTS &&
+             InstanceTypeChecker::IsCodeDataContainer(instance_type)) {
+    CodeT codet = GetCodeT();
+    DCHECK(!codet.is_off_heap_trampoline());
+    return FromCodeT(codet);
+  } else {
+    UNREACHABLE();
+  }
+}
+
 CodeT AbstractCode::ToCodeT(PtrComprCageBase cage_base) {
   InstanceType instance_type = map(cage_base).instance_type();
   if (InstanceTypeChecker::IsCode(instance_type)) {

src/objects/code.h

@@ -1053,7 +1053,7 @@ class AbstractCode : public HeapObject {
 
   inline Builtin builtin_id(PtrComprCageBase cage_base);
 
-  inline bool is_interpreter_trampoline_builtin(PtrComprCageBase cage_base);
+  inline bool is_off_heap_trampoline(PtrComprCageBase cage_base);
 
   inline HandlerTable::CatchPrediction GetBuiltinCatchPrediction(
       PtrComprCageBase cage_base);
@@ -1068,6 +1068,7 @@ class AbstractCode : public HeapObject {
   inline bool IsCodeT(PtrComprCageBase cage_base) const;
   inline bool IsBytecodeArray(PtrComprCageBase cage_base) const;
 
+  inline Code ToCode(PtrComprCageBase cage_base);
   inline CodeT ToCodeT(PtrComprCageBase cage_base);
 
   inline Code GetCode();

src/objects/instance-type-inl.h

@@ -79,6 +79,12 @@ V8_INLINE constexpr bool IsThinString(InstanceType instance_type) {
   return (instance_type & kStringRepresentationMask) == kThinStringTag;
 }
 
+V8_INLINE constexpr bool IsAbstractCode(InstanceType instance_type) {
+  return IsBytecodeArray(instance_type) || IsCode(instance_type) ||
+         (V8_REMOVE_BUILTINS_CODE_OBJECTS &&
+          IsCodeDataContainer(instance_type));
+}
+
 V8_INLINE constexpr bool IsFreeSpaceOrFiller(InstanceType instance_type) {
   return instance_type == FREE_SPACE_TYPE || instance_type == FILLER_TYPE;
 }

src/objects/instance-type.h

@@ -273,6 +273,7 @@ V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
   TORQUE_INSTANCE_CHECKERS_RANGE_ONLY_DECLARED(V)
 
 #define INSTANCE_TYPE_CHECKERS_CUSTOM(V) \
+  V(AbstractCode)                        \
   V(FreeSpaceOrFiller)                   \
   V(ExternalString)                      \
   V(InternalizedString)

src/objects/objects-inl.h

@@ -348,17 +348,6 @@ DEF_GETTER(HeapObject, IsOSROptimizedCodeCache, bool) {
   return IsWeakFixedArray(cage_base);
 }
 
-bool HeapObject::IsAbstractCode() const {
-  // TODO(v8:11880): Either make AbstractCode be ByteArray|CodeT or
-  // ensure this version is not called for hot code.
-  PtrComprCageBase cage_base = GetPtrComprCageBaseSlow(*this);
-  return HeapObject::IsAbstractCode(cage_base);
-}
-bool HeapObject::IsAbstractCode(PtrComprCageBase cage_base) const {
-  return IsBytecodeArray(cage_base) || IsCode(cage_base) ||
-         (V8_REMOVE_BUILTINS_CODE_OBJECTS && IsCodeDataContainer(cage_base));
-}
-
 DEF_GETTER(HeapObject, IsStringWrapper, bool) {
   return IsJSPrimitiveWrapper(cage_base) &&
          JSPrimitiveWrapper::cast(*this).value().IsString(cage_base);

src/objects/shared-function-info-inl.h

@@ -564,6 +564,7 @@ DEF_GETTER(SharedFunctionInfo, HasBytecodeArray, bool) {
 
 template <typename IsolateT>
 BytecodeArray SharedFunctionInfo::GetBytecodeArray(IsolateT* isolate) const {
+  // TODO(ishell): access shared_function_info_access() via IsolateT.
   SharedMutexGuardIfOffThread<IsolateT, base::kShared> mutex_guard(
       GetIsolate()->shared_function_info_access(), isolate);
 

src/profiler/profiler-listener.cc

@@ -136,7 +136,8 @@ void ProfilerListener::CodeCreateEvent(CodeTag tag,
       Handle<BytecodeArray> bytecodes(shared->GetBytecodeArray(isolate_),
                                       isolate_);
       Handle<ByteArray> bytecode_offsets(
-          abstract_code->GetCode().bytecode_offset_table(), isolate_);
+          abstract_code->ToCode(cage_base).bytecode_offset_table(cage_base),
+          isolate_);
       baseline_iterator = std::make_unique<baseline::BytecodeOffsetIterator>(
           bytecode_offsets, bytecodes);
     }

test/cctest/test-unwinder-code-pages.cc

@@ -296,13 +296,12 @@ TEST(Unwind_CodeObjectPCInMiddle_Success_CodePagesAPI) {
       Handle<JSFunction>::cast(v8::Utils::OpenHandle(*local_foo));
 
   // Put the current PC inside of the created code object.
-  AbstractCode abstract_code = foo->abstract_code(i_isolate);
-  PtrComprCageBase cage_base(i_isolate);
-  // We don't produce optimized code when run with --no-turbofan.
-  if (!abstract_code.IsCode(cage_base) && !FLAG_turbofan) return;
-  CHECK(abstract_code.IsCode(cage_base));
+  CodeT codet = foo->code();
+  // We don't produce optimized code when run with --no-turbofan and
+  // --no-maglev.
+  if (!codet.is_optimized_code()) return;
 
-  Code code = abstract_code.GetCode();
+  Code code = FromCodeT(codet);
   // We don't want the offset too early or it could be the `push rbp`
   // instruction (which is not at the start of generated code, because the lazy
   // deopt check happens before frame setup).

test/unittests/codegen/code-pages-unittest.cc

@@ -149,12 +149,11 @@ TEST_F(CodePagesTest, OptimizedCodeWithCodeRange) {
   Handle<JSFunction> foo =
       Handle<JSFunction>::cast(v8::Utils::OpenHandle(*local_foo));
 
-  AbstractCode abstract_code = foo->abstract_code(i_isolate());
-  PtrComprCageBase cage_base(i_isolate());
-  // We don't produce optimized code when run with --no-turbofan.
-  if (!abstract_code.IsCode(cage_base) && !FLAG_turbofan) return;
-  EXPECT_TRUE(abstract_code.IsCode(cage_base));
-  Code foo_code = abstract_code.GetCode();
+  CodeT codet = foo->code();
+  // We don't produce optimized code when run with --no-turbofan and
+  // --no-maglev.
+  if (!codet.is_optimized_code()) return;
+  Code foo_code = FromCodeT(codet);
 
   EXPECT_TRUE(i_isolate()->heap()->InSpace(foo_code, CODE_SPACE));
 
@@ -200,12 +199,11 @@ TEST_F(CodePagesTest, OptimizedCodeWithCodePages) {
         EXPECT_TRUE(FLAG_always_sparkplug);
         return;
       }
-      AbstractCode abstract_code = foo->abstract_code(i_isolate());
-      PtrComprCageBase cage_base(i_isolate());
-      // We don't produce optimized code when run with --no-turbofan.
-      if (!abstract_code.IsCode(cage_base) && !FLAG_turbofan) return;
-      EXPECT_TRUE(abstract_code.IsCode(cage_base));
-      Code foo_code = abstract_code.GetCode();
+      CodeT codet = foo->code();
+      // We don't produce optimized code when run with --no-turbofan and
+      // --no-maglev.
+      if (!codet.is_optimized_code()) return;
+      Code foo_code = FromCodeT(codet);
 
       EXPECT_TRUE(i_isolate()->heap()->InSpace(foo_code, CODE_SPACE));