[log] Use handles for LogRecordedBuffer

LogWriteDebugInfo can allocate when calculating line ends for source
positions, so make its called, LogRecordedBuffer, take Handles rather
than raw Objects. This also improves its API, as we can change the
maybe-null SharedFunctionInfo argument into a MaybeHandle.

Bug: chromium:1037872
Change-Id: Ifa3e2d9be7aa7de3b05e5c1e107406004b8963c7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1985995
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65603}

src/diagnostics/perf-jit.cc

@@ -33,6 +33,7 @@
 #include "src/codegen/source-position-table.h"
 #include "src/diagnostics/eh-frame.h"
 #include "src/objects/objects-inl.h"
+#include "src/objects/shared-function-info.h"
 #include "src/snapshot/embedded/embedded-data.h"
 #include "src/utils/ostreams.h"
 #include "src/wasm/wasm-code-manager.h"
@@ -197,12 +198,13 @@ uint64_t PerfJitLogger::GetTimestamp() {
   return (ts.tv_sec * kNsecPerSec) + ts.tv_nsec;
 }
 
-void PerfJitLogger::LogRecordedBuffer(AbstractCode abstract_code,
-                                      SharedFunctionInfo shared,
-                                      const char* name, int length) {
+void PerfJitLogger::LogRecordedBuffer(
+    Handle<AbstractCode> abstract_code,
+    MaybeHandle<SharedFunctionInfo> maybe_shared, const char* name,
+    int length) {
   if (FLAG_perf_basic_prof_only_functions &&
-      (abstract_code.kind() != AbstractCode::INTERPRETED_FUNCTION &&
-       abstract_code.kind() != AbstractCode::OPTIMIZED_FUNCTION)) {
+      (abstract_code->kind() != AbstractCode::INTERPRETED_FUNCTION &&
+       abstract_code->kind() != AbstractCode::OPTIMIZED_FUNCTION)) {
     return;
   }
 
@@ -211,28 +213,29 @@ void PerfJitLogger::LogRecordedBuffer(AbstractCode abstract_code,
   if (perf_output_handle_ == nullptr) return;
 
   // We only support non-interpreted functions.
-  if (!abstract_code.IsCode()) return;
-  Code code = abstract_code.GetCode();
-  DCHECK(code.raw_instruction_start() == code.address() + Code::kHeaderSize);
+  if (!abstract_code->IsCode()) return;
+  Handle<Code> code = Handle<Code>::cast(abstract_code);
+  DCHECK(code->raw_instruction_start() == code->address() + Code::kHeaderSize);
 
   // Debug info has to be emitted first.
-  if (FLAG_perf_prof && !shared.is_null()) {
+  Handle<SharedFunctionInfo> shared;
+  if (FLAG_perf_prof && !maybe_shared.ToHandle(&shared)) {
     // TODO(herhut): This currently breaks for js2wasm/wasm2js functions.
-    if (code.kind() != Code::JS_TO_WASM_FUNCTION &&
-        code.kind() != Code::WASM_TO_JS_FUNCTION) {
+    if (code->kind() != Code::JS_TO_WASM_FUNCTION &&
+        code->kind() != Code::WASM_TO_JS_FUNCTION) {
       LogWriteDebugInfo(code, shared);
     }
   }
 
   const char* code_name = name;
-  uint8_t* code_pointer = reinterpret_cast<uint8_t*>(code.InstructionStart());
+  uint8_t* code_pointer = reinterpret_cast<uint8_t*>(code->InstructionStart());
 
   // Code generated by Turbofan will have the safepoint table directly after
   // instructions. There is no need to record the safepoint table itself.
-  uint32_t code_size = code.ExecutableInstructionSize();
+  uint32_t code_size = code->ExecutableInstructionSize();
 
   // Unwinding info comes right after debug info.
-  if (FLAG_perf_prof_unwinding_info) LogWriteUnwindingInfo(code);
+  if (FLAG_perf_prof_unwinding_info) LogWriteUnwindingInfo(*code);
 
   WriteJitCodeLoadEntry(code_pointer, code_size, code_name, length);
 }
@@ -328,24 +331,24 @@ SourcePositionInfo GetSourcePositionInfo(Handle<Code> code,
 
 }  // namespace
 
-void PerfJitLogger::LogWriteDebugInfo(Code code, SharedFunctionInfo shared) {
+void PerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
+                                      Handle<SharedFunctionInfo> shared) {
   // Compute the entry count and get the name of the script.
   uint32_t entry_count = 0;
-  for (SourcePositionTableIterator iterator(code.SourcePositionTable());
+  for (SourcePositionTableIterator iterator(code->SourcePositionTable());
        !iterator.done(); iterator.Advance()) {
     entry_count++;
   }
   if (entry_count == 0) return;
   // The WasmToJS wrapper stubs have source position entries.
-  if (!shared.HasSourceCode()) return;
-  Isolate* isolate = shared.GetIsolate();
-  Handle<Script> script(Script::cast(shared.script()), isolate);
+  if (!shared->HasSourceCode()) return;
+  Handle<Script> script(Script::cast(shared->script()), isolate_);
 
   PerfJitCodeDebugInfo debug_info;
 
   debug_info.event_ = PerfJitCodeLoad::kDebugInfo;
   debug_info.time_stamp_ = GetTimestamp();
-  debug_info.address_ = code.InstructionStart();
+  debug_info.address_ = code->InstructionStart();
   debug_info.entry_count_ = entry_count;
 
   uint32_t size = sizeof(debug_info);
@@ -353,12 +356,10 @@ void PerfJitLogger::LogWriteDebugInfo(Code code, SharedFunctionInfo shared) {
   size += entry_count * sizeof(PerfJitDebugEntry);
   // Add the size of the name after each entry.
 
-  Handle<Code> code_handle(code, isolate);
-  Handle<SharedFunctionInfo> function_handle(shared, isolate);
-  for (SourcePositionTableIterator iterator(code.SourcePositionTable());
+  for (SourcePositionTableIterator iterator(code->SourcePositionTable());
        !iterator.done(); iterator.Advance()) {
-    SourcePositionInfo info(GetSourcePositionInfo(code_handle, function_handle,
-                                                  iterator.source_position()));
+    SourcePositionInfo info(
+        GetSourcePositionInfo(code, shared, iterator.source_position()));
     size += GetScriptNameLength(info) + 1;
   }
 
@@ -366,12 +367,12 @@ void PerfJitLogger::LogWriteDebugInfo(Code code, SharedFunctionInfo shared) {
   debug_info.size_ = size + padding;
   LogWriteBytes(reinterpret_cast<const char*>(&debug_info), sizeof(debug_info));
 
-  Address code_start = code.InstructionStart();
+  Address code_start = code->InstructionStart();
 
-  for (SourcePositionTableIterator iterator(code.SourcePositionTable());
+  for (SourcePositionTableIterator iterator(code->SourcePositionTable());
        !iterator.done(); iterator.Advance()) {
-    SourcePositionInfo info(GetSourcePositionInfo(code_handle, function_handle,
-                                                  iterator.source_position()));
+    SourcePositionInfo info(
+        GetSourcePositionInfo(code, shared, iterator.source_position()));
     PerfJitDebugEntry entry;
     // The entry point of the function will be placed straight after the ELF
     // header when processed by "perf inject". Adjust the position addresses

src/diagnostics/perf-jit.h

@@ -52,7 +52,8 @@ class PerfJitLogger : public CodeEventLogger {
   void CloseMarkerFile(void* marker_address);
 
   uint64_t GetTimestamp();
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo> maybe_shared,
                          const char* name, int length) override;
   void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                          int length) override;
@@ -70,7 +71,7 @@ class PerfJitLogger : public CodeEventLogger {
 
   void LogWriteBytes(const char* bytes, int size);
   void LogWriteHeader();
-  void LogWriteDebugInfo(Code code, SharedFunctionInfo shared);
+  void LogWriteDebugInfo(Handle<Code> code, Handle<SharedFunctionInfo> shared);
   void LogWriteDebugInfo(const wasm::WasmCode* code);
   void LogWriteUnwindingInfo(Code code);
 
@@ -136,7 +137,8 @@ class PerfJitLogger : public CodeEventLogger {
     UNIMPLEMENTED();
   }
 
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo> maybe_shared,
                          const char* name, int length) override {
     UNIMPLEMENTED();
   }

src/logging/log.cc

@@ -189,8 +189,8 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       const char* comment) {
   name_buffer_->Init(tag);
   name_buffer_->AppendBytes(comment);
-  LogRecordedBuffer(*code, SharedFunctionInfo(), name_buffer_->get(),
-                    name_buffer_->size());
+  LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
+                    name_buffer_->get(), name_buffer_->size());
 }
 
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
@@ -198,8 +198,8 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       Handle<Name> name) {
   name_buffer_->Init(tag);
   name_buffer_->AppendName(*name);
-  LogRecordedBuffer(*code, SharedFunctionInfo(), name_buffer_->get(),
-                    name_buffer_->size());
+  LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
+                    name_buffer_->get(), name_buffer_->size());
 }
 
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
@@ -210,7 +210,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
   name_buffer_->AppendBytes(ComputeMarker(*shared, *code));
   name_buffer_->AppendByte(' ');
   name_buffer_->AppendName(*script_name);
-  LogRecordedBuffer(*code, *shared, name_buffer_->get(), name_buffer_->size());
+  LogRecordedBuffer(code, shared, name_buffer_->get(), name_buffer_->size());
 }
 
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
@@ -231,7 +231,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
   }
   name_buffer_->AppendByte(':');
   name_buffer_->AppendInt(line);
-  LogRecordedBuffer(*code, *shared, name_buffer_->get(), name_buffer_->size());
+  LogRecordedBuffer(code, shared, name_buffer_->get(), name_buffer_->size());
 }
 
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
@@ -258,8 +258,8 @@ void CodeEventLogger::RegExpCodeCreateEvent(Handle<AbstractCode> code,
                                             Handle<String> source) {
   name_buffer_->Init(CodeEventListener::REG_EXP_TAG);
   name_buffer_->AppendString(*source);
-  LogRecordedBuffer(*code, SharedFunctionInfo(), name_buffer_->get(),
-                    name_buffer_->size());
+  LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
+                    name_buffer_->get(), name_buffer_->size());
 }
 
 // Linux perf tool logging support
@@ -273,7 +273,8 @@ class PerfBasicLogger : public CodeEventLogger {
                            Handle<SharedFunctionInfo> shared) override {}
 
  private:
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo> maybe_shared,
                          const char* name, int length) override;
   void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                          int length) override;
@@ -323,17 +324,18 @@ void PerfBasicLogger::WriteLogRecordedBuffer(uintptr_t address, int size,
                    size, name_length, name);
 }
 
-void PerfBasicLogger::LogRecordedBuffer(AbstractCode code, SharedFunctionInfo,
+void PerfBasicLogger::LogRecordedBuffer(Handle<AbstractCode> code,
+                                        MaybeHandle<SharedFunctionInfo>,
                                         const char* name, int length) {
   if (FLAG_perf_basic_prof_only_functions &&
-      (code.kind() != AbstractCode::INTERPRETED_FUNCTION &&
-       code.kind() != AbstractCode::BUILTIN &&
-       code.kind() != AbstractCode::OPTIMIZED_FUNCTION)) {
+      (code->kind() != AbstractCode::INTERPRETED_FUNCTION &&
+       code->kind() != AbstractCode::BUILTIN &&
+       code->kind() != AbstractCode::OPTIMIZED_FUNCTION)) {
     return;
   }
 
-  WriteLogRecordedBuffer(static_cast<uintptr_t>(code.InstructionStart()),
-                         code.InstructionSize(), name, length);
+  WriteLogRecordedBuffer(static_cast<uintptr_t>(code->InstructionStart()),
+                         code->InstructionSize(), name, length);
 }
 
 void PerfBasicLogger::LogRecordedBuffer(const wasm::WasmCode* code,
@@ -513,7 +515,8 @@ class LowLevelLogger : public CodeEventLogger {
   void CodeMovingGCEvent() override;
 
  private:
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo> maybe_shared,
                          const char* name, int length) override;
   void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                          int length) override;
@@ -594,16 +597,17 @@ void LowLevelLogger::LogCodeInfo() {
   LogWriteBytes(arch, sizeof(arch));
 }
 
-void LowLevelLogger::LogRecordedBuffer(AbstractCode code, SharedFunctionInfo,
+void LowLevelLogger::LogRecordedBuffer(Handle<AbstractCode> code,
+                                       MaybeHandle<SharedFunctionInfo>,
                                        const char* name, int length) {
   CodeCreateStruct event;
   event.name_size = length;
-  event.code_address = code.InstructionStart();
-  event.code_size = code.InstructionSize();
+  event.code_address = code->InstructionStart();
+  event.code_size = code->InstructionSize();
   LogWriteStruct(event);
   LogWriteBytes(name, length);
-  LogWriteBytes(reinterpret_cast<const char*>(code.InstructionStart()),
-                code.InstructionSize());
+  LogWriteBytes(reinterpret_cast<const char*>(code->InstructionStart()),
+                code->InstructionSize());
 }
 
 void LowLevelLogger::LogRecordedBuffer(const wasm::WasmCode* code,
@@ -652,7 +656,8 @@ class JitLogger : public CodeEventLogger {
   void EndCodePosInfoEvent(Address start_address, void* jit_handler_data);
 
  private:
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo> maybe_shared,
                          const char* name, int length) override;
   void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                          int length) override;
@@ -664,21 +669,22 @@ class JitLogger : public CodeEventLogger {
 JitLogger::JitLogger(Isolate* isolate, JitCodeEventHandler code_event_handler)
     : CodeEventLogger(isolate), code_event_handler_(code_event_handler) {}
 
-void JitLogger::LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+void JitLogger::LogRecordedBuffer(Handle<AbstractCode> code,
+                                  MaybeHandle<SharedFunctionInfo> maybe_shared,
                                   const char* name, int length) {
   JitCodeEvent event;
   memset(static_cast<void*>(&event), 0, sizeof(event));
   event.type = JitCodeEvent::CODE_ADDED;
-  event.code_start = reinterpret_cast<void*>(code.InstructionStart());
+  event.code_start = reinterpret_cast<void*>(code->InstructionStart());
   event.code_type =
-      code.IsCode() ? JitCodeEvent::JIT_CODE : JitCodeEvent::BYTE_CODE;
-  event.code_len = code.InstructionSize();
-  Handle<SharedFunctionInfo> shared_function_handle;
-  if (!shared.is_null() && shared.script().IsScript()) {
-    shared_function_handle =
-        Handle<SharedFunctionInfo>(shared, shared.GetIsolate());
-  }
-  event.script = ToApiHandle<v8::UnboundScript>(shared_function_handle);
+      code->IsCode() ? JitCodeEvent::JIT_CODE : JitCodeEvent::BYTE_CODE;
+  event.code_len = code->InstructionSize();
+  Handle<SharedFunctionInfo> shared;
+  if (maybe_shared.ToHandle(&shared) && shared->script().IsScript()) {
+    event.script = ToApiHandle<v8::UnboundScript>(shared);
+  } else {
+    event.script = Local<v8::UnboundScript>();
+  }
   event.name.str = name;
   event.name.len = length;
   event.isolate = reinterpret_cast<v8::Isolate*>(isolate_);
@@ -855,9 +861,9 @@ class Profiler : public base::Thread {
   // Cyclic buffer for communicating profiling samples
   // between the signal handler and the worker thread.
   static const int kBufferSize = 128;
-  TickSample buffer_[kBufferSize];      // Buffer storage.
-  int head_;                            // Index to the buffer head.
-  base::Atomic32 tail_;                 // Index to the buffer tail.
+  TickSample buffer_[kBufferSize];  // Buffer storage.
+  int head_;                        // Index to the buffer head.
+  base::Atomic32 tail_;             // Index to the buffer tail.
   bool overflow_;  // Tell whether a buffer overflow has occurred.
   // Semaphore used for buffer synchronization.
   base::Semaphore buffer_semaphore_;

src/logging/log.h

@@ -404,7 +404,8 @@ class V8_EXPORT_PRIVATE CodeEventLogger : public CodeEventListener {
  private:
   class NameBuffer;
 
-  virtual void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo shared,
+  virtual void LogRecordedBuffer(Handle<AbstractCode> code,
+                                 MaybeHandle<SharedFunctionInfo> maybe_shared,
                                  const char* name, int length) = 0;
   virtual void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                                  int length) = 0;

src/snapshot/serializer.h

@@ -114,9 +114,10 @@ class CodeAddressMap : public CodeEventLogger {
     DISALLOW_COPY_AND_ASSIGN(NameMap);
   };
 
-  void LogRecordedBuffer(AbstractCode code, SharedFunctionInfo,
-                         const char* name, int length) override {
-    address_to_name_map_.Insert(code.address(), name, length);
+  void LogRecordedBuffer(Handle<AbstractCode> code,
+                         MaybeHandle<SharedFunctionInfo>, const char* name,
+                         int length) override {
+    address_to_name_map_.Insert(code->address(), name, length);
   }
 
   void LogRecordedBuffer(const wasm::WasmCode* code, const char* name,

test/cctest/test-log.cc

@@ -465,7 +465,8 @@ UNINITIALIZED_TEST(Issue539892) {
     }
 
    private:
-    void LogRecordedBuffer(i::AbstractCode code, i::SharedFunctionInfo shared,
+    void LogRecordedBuffer(i::Handle<i::AbstractCode> code,
+                           i::MaybeHandle<i::SharedFunctionInfo> maybe_shared,
                            const char* name, int length) override {}
     void LogRecordedBuffer(const i::wasm::WasmCode* code, const char* name,
                            int length) override {}