[Profiler] Ensure ticks in frameless bytecode handlers are interpreted frames.

On Arm/64 the last return address is stored in a link register instead of
being pushed to the top-of-stack like on x64/ia32. Extend the support in the
tick sampler to check for samples in a frameless bytecode handler with support
for checking the link register if it exists instead of top-of-stack. In addition,
make the x64/ia32 check more robust by ensuring we only apply the change if the
pc is a bytecode handler and the top frame isn't a bytecode handler (stub) frame.

BUG=v8:9162

Change-Id: I89d2e80ea8a0b84ff6a265d0e0e73f9fdd1daca8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578464
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60976}

include/v8.h

@@ -1919,10 +1919,11 @@ enum StateTag {
 // A RegisterState represents the current state of registers used
 // by the sampling profiler API.
 struct RegisterState {
-  RegisterState() : pc(nullptr), sp(nullptr), fp(nullptr) {}
+  RegisterState() : pc(nullptr), sp(nullptr), fp(nullptr), lr(nullptr) {}
   void* pc;  // Instruction pointer.
   void* sp;  // Stack pointer.
   void* fp;  // Frame pointer.
+  void* lr;  // Link register (or nullptr on platforms without a link register).
 };
 
 // The output structure filled up by GetStackSample API function.

src/frames.cc

@@ -107,16 +107,14 @@ void StackFrameIterator::Reset(ThreadLocalTop* top) {
   frame_ = SingletonFor(type, &state);
 }
 
-
 StackFrame* StackFrameIteratorBase::SingletonFor(StackFrame::Type type,
-                                             StackFrame::State* state) {
+                                                 StackFrame::State* state) {
   StackFrame* result = SingletonFor(type);
   DCHECK((!result) == (type == StackFrame::NONE));
   if (result) result->state_ = *state;
   return result;
 }
 
-
 StackFrame* StackFrameIteratorBase::SingletonFor(StackFrame::Type type) {
 #define FRAME_TYPE_CASE(type, field) \
   case StackFrame::type:             \
@@ -210,20 +208,48 @@ bool IsInterpreterFramePc(Isolate* isolate, Address pc,
   }
 }
 
-DISABLE_ASAN Address ReadMemoryAt(Address address) {
-  return Memory<Address>(address);
-}
-
 }  // namespace
 
-SafeStackFrameIterator::SafeStackFrameIterator(
-    Isolate* isolate,
-    Address fp, Address sp, Address js_entry_sp)
+bool SafeStackFrameIterator::IsNoFrameBytecodeHandlerPc(Isolate* isolate,
+                                                        Address pc,
+                                                        Address fp) const {
+  // Return false for builds with non-embedded bytecode handlers.
+  if (Isolate::CurrentEmbeddedBlob() == nullptr) return false;
+
+  EmbeddedData d = EmbeddedData::FromBlob();
+  if (pc < d.InstructionStartOfBytecodeHandlers() ||
+      pc >= d.InstructionEndOfBytecodeHandlers()) {
+    // Not a bytecode handler pc address.
+    return false;
+  }
+
+  if (!IsValidStackAddress(fp +
+                           CommonFrameConstants::kContextOrFrameTypeOffset)) {
+    return false;
+  }
+
+  // Check if top stack frame is a bytecode handler stub frame.
+  MSAN_MEMORY_IS_INITIALIZED(
+      fp + CommonFrameConstants::kContextOrFrameTypeOffset, kSystemPointerSize);
+  intptr_t marker =
+      Memory<intptr_t>(fp + CommonFrameConstants::kContextOrFrameTypeOffset);
+  if (StackFrame::IsTypeMarker(marker) &&
+      StackFrame::MarkerToType(marker) == StackFrame::STUB) {
+    // Bytecode handler built a frame.
+    return false;
+  }
+  return true;
+}
+
+SafeStackFrameIterator::SafeStackFrameIterator(Isolate* isolate, Address pc,
+                                               Address fp, Address sp,
+                                               Address lr, Address js_entry_sp)
     : StackFrameIteratorBase(isolate, false),
       low_bound_(sp),
       high_bound_(js_entry_sp),
       top_frame_type_(StackFrame::NONE),
-      external_callback_scope_(isolate->external_callback_scope()) {
+      external_callback_scope_(isolate->external_callback_scope()),
+      top_link_register_(lr) {
   StackFrame::State state;
   StackFrame::Type type;
   ThreadLocalTop* top = isolate->thread_local_top();
@@ -255,14 +281,22 @@ SafeStackFrameIterator::SafeStackFrameIterator(
     state.pc_address = StackFrame::ResolveReturnAddressLocation(
         reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp)));
 
-    // If the top of stack is a return address to the interpreter trampoline,
-    // then we are likely in a bytecode handler with elided frame. In that
-    // case, set the PC properly and make sure we do not drop the frame.
-    if (IsValidStackAddress(sp)) {
-      MSAN_MEMORY_IS_INITIALIZED(sp, kSystemPointerSize);
-      Address tos = ReadMemoryAt(sp);
-      if (IsInterpreterFramePc(isolate, tos, &state)) {
-        state.pc_address = reinterpret_cast<Address*>(sp);
+    // If the current PC is in a bytecode handler, the top stack frame isn't
+    // the bytecode handler's frame and the top of stack or link register is a
+    // return address into the interpreter entry trampoline, then we are likely
+    // in a bytecode handler with elided frame. In that case, set the PC
+    // properly and make sure we do not drop the frame.
+    if (IsNoFrameBytecodeHandlerPc(isolate, pc, fp)) {
+      Address* tos_location = nullptr;
+      if (top_link_register_) {
+        tos_location = &top_link_register_;
+      } else if (IsValidStackAddress(sp)) {
+        MSAN_MEMORY_IS_INITIALIZED(sp, kSystemPointerSize);
+        tos_location = reinterpret_cast<Address*>(sp);
+      }
+
+      if (IsInterpreterFramePc(isolate, *tos_location, &state)) {
+        state.pc_address = tos_location;
         advance_frame = false;
       }
     }
@@ -300,7 +334,6 @@ SafeStackFrameIterator::SafeStackFrameIterator(
   if (advance_frame && frame_) Advance();
 }
 
-
 bool SafeStackFrameIterator::IsValidTop(ThreadLocalTop* top) const {
   Address c_entry_fp = Isolate::c_entry_fp(top);
   if (!IsValidExitFrame(c_entry_fp)) return false;

src/frames.h

@@ -1274,9 +1274,8 @@ class V8_EXPORT_PRIVATE StackTraceFrameIterator {
 
 class SafeStackFrameIterator: public StackFrameIteratorBase {
  public:
-  SafeStackFrameIterator(Isolate* isolate,
-                         Address fp, Address sp,
-                         Address js_entry_sp);
+  SafeStackFrameIterator(Isolate* isolate, Address pc, Address fp, Address sp,
+                         Address lr, Address js_entry_sp);
 
   inline StackFrame* frame() const;
   void Advance();
@@ -1294,10 +1293,19 @@ class SafeStackFrameIterator: public StackFrameIteratorBase {
   bool IsValidExitFrame(Address fp) const;
   bool IsValidTop(ThreadLocalTop* top) const;
 
+  // Returns true if the pc points to a bytecode handler and the frame pointer
+  // doesn't seem to be a bytecode handler's frame, which implies that the
+  // bytecode handler has an elided frame. This is not precise and might give
+  // false negatives since it relies on checks to the frame's type marker,
+  // which might be uninitialized.
+  bool IsNoFrameBytecodeHandlerPc(Isolate* isolate, Address pc,
+                                  Address fp) const;
+
   const Address low_bound_;
   const Address high_bound_;
   StackFrame::Type top_frame_type_;
   ExternalCallbackScope* external_callback_scope_;
+  Address top_link_register_;
 };
 }  // namespace internal
 }  // namespace v8

src/libsampler/sampler.cc

@@ -391,16 +391,20 @@ void SignalHandler::FillRegisterState(void* context, RegisterState* state) {
   state->pc = reinterpret_cast<void*>(mcontext.gregs[R15]);
   state->sp = reinterpret_cast<void*>(mcontext.gregs[R13]);
   state->fp = reinterpret_cast<void*>(mcontext.gregs[R11]);
+  state->lr = reinterpret_cast<void*>(mcontext.gregs[R14]);
 #else
   state->pc = reinterpret_cast<void*>(mcontext.arm_pc);
   state->sp = reinterpret_cast<void*>(mcontext.arm_sp);
   state->fp = reinterpret_cast<void*>(mcontext.arm_fp);
+  state->lr = reinterpret_cast<void*>(mcontext.arm_lr);
 #endif  // V8_LIBC_GLIBC && !V8_GLIBC_PREREQ(2, 4)
 #elif V8_HOST_ARCH_ARM64
   state->pc = reinterpret_cast<void*>(mcontext.pc);
   state->sp = reinterpret_cast<void*>(mcontext.sp);
   // FP is an alias for x29.
   state->fp = reinterpret_cast<void*>(mcontext.regs[29]);
+  // LR is an alias for x30.
+  state->lr = reinterpret_cast<void*>(mcontext.regs[30]);
 #elif V8_HOST_ARCH_MIPS
   state->pc = reinterpret_cast<void*>(mcontext.pc);
   state->sp = reinterpret_cast<void*>(mcontext.gregs[29]);

src/profiler/tick-sample.cc

@@ -100,10 +100,12 @@ bool SimulatorHelper::FillRegisters(Isolate* isolate,
   }
   state->sp = reinterpret_cast<void*>(simulator->get_register(Simulator::sp));
   state->fp = reinterpret_cast<void*>(simulator->get_register(Simulator::r11));
+  state->lr = reinterpret_cast<void*>(simulator->get_register(Simulator::lr));
 #elif V8_TARGET_ARCH_ARM64
   state->pc = reinterpret_cast<void*>(simulator->pc());
   state->sp = reinterpret_cast<void*>(simulator->sp());
   state->fp = reinterpret_cast<void*>(simulator->fp());
+  state->lr = reinterpret_cast<void*>(simulator->lr());
 #elif V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64
   if (!simulator->has_bad_pc()) {
     state->pc = reinterpret_cast<void*>(simulator->get_pc());
@@ -234,8 +236,10 @@ bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs,
             : reinterpret_cast<void*>(*external_callback_entry_ptr);
   }
 
-  i::SafeStackFrameIterator it(isolate, reinterpret_cast<i::Address>(regs->fp),
+  i::SafeStackFrameIterator it(isolate, reinterpret_cast<i::Address>(regs->pc),
+                               reinterpret_cast<i::Address>(regs->fp),
                                reinterpret_cast<i::Address>(regs->sp),
+                               reinterpret_cast<i::Address>(regs->lr),
                                js_entry_sp);
   if (it.done()) return true;
 

src/snapshot/embedded-data.cc

@@ -290,6 +290,19 @@ uint32_t EmbeddedData::InstructionSizeOfBuiltin(int i) const {
   return metadata[i].instructions_length;
 }
 
+Address EmbeddedData::InstructionStartOfBytecodeHandlers() const {
+  return InstructionStartOfBuiltin(Builtins::kFirstBytecodeHandler);
+}
+
+Address EmbeddedData::InstructionEndOfBytecodeHandlers() const {
+  STATIC_ASSERT(Builtins::kFirstBytecodeHandler + kNumberOfBytecodeHandlers +
+                    2 * kNumberOfWideBytecodeHandlers ==
+                Builtins::builtin_count);
+  int lastBytecodeHandler = Builtins::builtin_count - 1;
+  return InstructionStartOfBuiltin(lastBytecodeHandler) +
+         InstructionSizeOfBuiltin(lastBytecodeHandler);
+}
+
 size_t EmbeddedData::CreateEmbeddedBlobHash() const {
   STATIC_ASSERT(EmbeddedBlobHashOffset() == 0);
   STATIC_ASSERT(EmbeddedBlobHashSize() == kSizetSize);

src/snapshot/embedded-data.h

@@ -57,6 +57,9 @@ class EmbeddedData final {
   Address InstructionStartOfBuiltin(int i) const;
   uint32_t InstructionSizeOfBuiltin(int i) const;
 
+  Address InstructionStartOfBytecodeHandlers() const;
+  Address InstructionEndOfBytecodeHandlers() const;
+
   bool ContainsBuiltin(int i) const { return InstructionSizeOfBuiltin(i) > 0; }
 
   uint32_t AddressForHashing(Address addr) {

src/unwinder.cc

@@ -88,6 +88,9 @@ bool Unwinder::TryUnwindV8Frames(const UnwindState& unwind_state,
     register_state->fp = final_fp;
 
     register_state->pc = next_pc;
+
+    // Link register no longer valid after unwinding.
+    register_state->lr = nullptr;
     return true;
   }
   return false;

test/cctest/test-sampler-api.cc

@@ -49,6 +49,8 @@ class SimulatorHelper {
         simulator_->get_register(v8::internal::Simulator::sp));
     state->fp = reinterpret_cast<void*>(
         simulator_->get_register(v8::internal::Simulator::r11));
+    state->lr = reinterpret_cast<void*>(
+        simulator_->get_register(v8::internal::Simulator::lr));
 #elif V8_TARGET_ARCH_ARM64
     if (simulator_->sp() == 0 || simulator_->fp() == 0) {
       // It's possible that the simulator is interrupted while it is updating
@@ -60,6 +62,7 @@ class SimulatorHelper {
     state->pc = reinterpret_cast<void*>(simulator_->pc());
     state->sp = reinterpret_cast<void*>(simulator_->sp());
     state->fp = reinterpret_cast<void*>(simulator_->fp());
+    state->lr = reinterpret_cast<void*>(simulator_->lr());
 #elif V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64
     state->pc = reinterpret_cast<void*>(simulator_->get_pc());
     state->sp = reinterpret_cast<void*>(