Allow RegisterContext to track if behaves-like-frame-0, allow LanguageRuntime for above frame 0

[jasonmolenda]

Summary

This patch allows a RegisterContext to record if it behaves like a 0th frame -- where its pc value should not be decremented before symbolication. It adds a call out to the LanguageRuntime to get a special UnwindPlan when unwinding from the 0th (first) frame, not just further unwinds up the stack. The majority of the patch is the mechanical addition of the new ivar/method to expose this, and the LanguageRuntime hook in InitializeZerothFrame(). This is a follow-on patch to the previous https://reviews.llvm.org/D96839

For the Swift async unwinds, the saved pc values we have are not normal ABI style calls, and backing up the resume pc results in bad symbolication. I've been meaning to add a capability like this to RegisterContext for a while for DWARFExpression::Evaluate where we get a frame's pc value to look up a location list entry too (and do not decr the pc resulting in bogus values being displayed for noreturn calls) - I'll fix that in a separate patch.

I have API tests when the Swift language runtime is present, but in generic lldb we don't have an environment quite like this. In the followup patch to DWARFExpression::Evaluate I'll be able to test RegisterContext::BehavesLikeZerothFrame so there will be some test coverage then.

rdar://problem/70398009

[jasonmolenda]

Another alternative to RegisterContext::BehavesLikeZerothFrame that I've thought of is RegisterContext::GetPCForSymbolication, similar to GetPC() today. I think everyone who is decrementing $pc is doing it for symbolication, so having this hint and then leaving it to everyone to decrement-or-not may not be a great choice. It may be easier for higher-levels if RegisterContext can provide an Address suitable for symbolication directly.

[clayborg]

In D97644#2593228, @jasonmolenda wrote:

Another alternative to RegisterContext::BehavesLikeZerothFrame that I've thought of is RegisterContext::GetPCForSymbolication, similar to GetPC() today. I think everyone who is decrementing $pc is doing it for symbolication, so having this hint and then leaving it to everyone to decrement-or-not may not be a great choice. It may be easier for higher-levels if RegisterContext can provide an Address suitable for symbolication directly.

If "behaves_like_zeroth_frame" is in the register context for the purpose of backing up the address, then I vote for RegisterContext::GetPCForSymbolication(). We can actually backup the PC by the min instruction size if we really want to get fancy. It would be really nice to have this centralized in one location so we don't have a bunch of code locations backing up the PC.

For the Swift async unwinds, the saved pc values we have are not normal ABI style calls, and backing up the resume pc results in bad symbolication. I've been meaning to add a capability like this to RegisterContext for a while for DWARFExpression::Evaluate where we get a frame's pc value to look up a location list entry too (and do not decr the pc resulting in bogus values being displayed for noreturn calls) - I'll fix that in a separate patch.

What does this mean for ReportCrash and other tools that like to just walk the frame pointer chain? Does that still produce valid stack trace, just with some Swift async frames missing?

[jasonmolenda]

Rewrite patch to expose "get pc for symbolication" APIs instead of "behaves like zeroth frame" APIs so that we don't have different parts of lldb decrementing the pc for symbolication. Doing a final round of tests on different targets etc but I believe this is done.

[clayborg]

lldb/source/Target/RegisterContext.cpp

162

Can we decrement the PC by the minimum instruction size? If we can get an ArchSpec we can call:

uint32_t ArchSpec::GetMinimumOpcodeByteSize() const;

lldb/source/Target/StackFrame.cpp

217–226

Should we be using the "bool RegisterContext::GetPCForSymbolication(Address &address)" function here to avoid having more than one place that is subtracting from the PC value?

[jasonmolenda]

lldb/source/Target/RegisterContext.cpp

162

I don't know if it's worth it. For fixed-width architectures, we'll be symbolicating an instruction boundary, but for others, we're just decrementing within the bounds of the instruction. We should not ever show this address anywhere, it's only used for setting context and we need to be within the bounds of the instruction. pc-1 seems fine to me. I don't feel very strongly about this.

lldb/source/Target/StackFrame.cpp

217–226

Yeah, I thought about that as I was doing it. I'll unify these.

[clayborg]

lldb/source/Target/RegisterContext.cpp

162

My only reason to vote for doing this is to avoid creating a lookup address that looks like a thumb address on 32 bit ARM if we subtract 1 from it and the code is available above. That being said, I don't feel strongly about this either.

[jasonmolenda]

lldb/source/Target/StackFrame.cpp

217–226

Ah, I see why they can't be unified. A StackFrame may be set up with a pc value different than the register context for the concrete frame here, e.g. for a tail-call artifical stack frame. It took me a little while to figure out why deferring to the StackFrame's RegisterContext was causing failures in those tests. I added a comment saying why this code needs to decr-pc too, so someone doesn't try to clean these up in the future.

lldb/include/lldb/Target/LanguageRuntime.h

public:
   /// frame is not following the normal ABI conventions.
   /// Instead of using the normal UnwindPlan for the function, we will use
   /// this special UnwindPlan for this one backtrace.
   /// One example of this would be a language that has asynchronous functions,
   /// functions that may not be currently-executing, while waiting on other
   /// asynchronous calls they made, but are part of a logical backtrace that
   /// we want to show the developer because that's how they think of the
   /// program flow.
+  ///
+  /// \param[in] thread
+  ///     The thread that the unwind is happening on.
+  ///
+  /// \param[in] regctx
+  ///     The RegisterContext for the frame we need to create an UnwindPlan.
+  ///     We don't yet have a StackFrame when we're selecting the UnwindPlan.
+  ///
+  /// \param[out] behaves_like_zeroth_frame
+  ///     With normal ABI calls, all stack frames except the zeroth frame need
+  ///     to have the return-pc value backed up by 1 for symbolication purposes.
+  ///     For these LanguageRuntime unwind plans, they may not follow normal ABI
+  ///     calling conventions and the return pc may need to be symbolicated
+  ///     as-is.
+  ///
+  /// \return
+  ///     Returns an UnwindPlan to find the caller frame if it should be used,
+  ///     instead of the UnwindPlan that would normally be used for this
+  ///     function.
   static lldb::UnwindPlanSP
   GetRuntimeUnwindPlan(lldb_private::Thread &thread,
-                       lldb_private::RegisterContext *regctx);
+                       lldb_private::RegisterContext *regctx,
+                       bool &behaves_like_zeroth_frame);
 
 protected:
   // The static GetRuntimeUnwindPlan method above is only implemented in the
   // base class; subclasses may override this protected member if they can
   // provide one of these UnwindPlans.
   virtual lldb::UnwindPlanSP
   GetRuntimeUnwindPlan(lldb::ProcessSP process_sp,
-                       lldb_private::RegisterContext *regctx) {
+                       lldb_private::RegisterContext *regctx,
+                       bool &behaves_like_zeroth_frame) {
     return lldb::UnwindPlanSP();
   }
 
   LanguageRuntime(Process *process);
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_LANGUAGERUNTIME_H

lldb/include/lldb/Target/RegisterContext.h

public:
   const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
                                             uint32_t start_idx = 0);
 
   const RegisterInfo *GetRegisterInfo(lldb::RegisterKind reg_kind,
                                       uint32_t reg_num);
 
   uint64_t GetPC(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
+  /// Get an address suitable for symbolication.
+  /// When symbolicating -- computing line, block, function --
+  /// for a function in the middle of the stack, using the return
+  /// address can lead to unexpected results for the user.
+  /// A function that ends in a tail-call may have another function
+  /// as the "return" address, but it will never actually return.
+  /// Or a noreturn call in the middle of a function is the end of
+  /// a block of instructions, and a DWARF location list entry for
+  /// the return address may be a very different code path with
+  /// incorrect results when printing variables for this frame.
+  ///
+  /// At a source line view, the user expects the current-line indictation
+  /// to point to the function call they're under, not the next source line.
+  ///
+  /// The return address (GetPC()) should always be shown to the user,
+  /// but when computing context, keeping within the bounds of the
+  /// call instruction is what the user expects to see.
+  ///
+  /// \param [out] address
+  ///     An Address object that will be filled in, if a PC can be retrieved.
+  ///
+  /// \return
+  ///     Returns true if the Address param was filled in.
+  bool GetPCForSymbolication(Address &address);
+
   bool SetPC(uint64_t pc);
 
   bool SetPC(Address addr);
 
   uint64_t GetSP(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
   bool SetSP(uint64_t sp);
 
public:
 
   void CalculateExecutionContext(ExecutionContext &exe_ctx) override;
 
   uint32_t GetStopID() const { return m_stop_id; }
 
   void SetStopID(uint32_t stop_id) { m_stop_id = stop_id; }
 
 protected:
+  /// Indicates that this frame is currently executing code,
+  /// that the PC value is not a return-pc but an actual executing
+  /// instruction.  Some places in lldb will treat a return-pc
+  /// value differently than the currently-executing-pc value,
+  /// and this method can indicate if that should be done.
+  /// The base class implementation only uses the frame index,
+  /// but subclasses may have additional information that they
+  /// can use to detect frames in this state, for instance a
+  /// frame above a trap handler (sigtramp etc)..
+  virtual bool BehavesLikeZerothFrame() const {
+    return m_concrete_frame_idx == 0;
+  }
+
   // Classes that inherit from RegisterContext can see and modify these
   Thread &m_thread; // The thread that this register context belongs to.
   uint32_t m_concrete_frame_idx; // The concrete frame index for this register
                                  // context
   uint32_t m_stop_id; // The stop ID that any data in this context is valid for
 private:
   // For RegisterContext only
   RegisterContext(const RegisterContext &) = delete;
   const RegisterContext &operator=(const RegisterContext &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_REGISTERCONTEXT_H

lldb/include/lldb/Target/RegisterContextUnwind.h

public:
   bool IsTrapHandlerFrame() const;
 
   bool GetCFA(lldb::addr_t &cfa);
 
   bool GetStartPC(lldb::addr_t &start_pc);
 
   bool ReadPC(lldb::addr_t &start_pc);
 
+  // Indicates whether this frame *behaves* like frame zero -- the currently
+  // executing frame -- or not.  This can be true in the middle of the stack
+  // above asynchronous trap handlers (sigtramp) for instance.
+  bool BehavesLikeZerothFrame() const override;
+
 private:
   enum FrameType {
     eNormalFrame,
     eTrapHandlerFrame,
     eDebuggerFrame, // a debugger inferior function call frame; we get caller's
                     // registers from debugger
     eSkipFrame,     // The unwind resulted in a bogus frame but may get back on
                     // track so we don't want to give up yet
private:
   lldb::addr_t m_afa;
   lldb_private::Address m_start_pc;
   lldb_private::Address m_current_pc;
 
   int m_current_offset; // how far into the function we've executed; -1 if
                         // unknown
                         // 0 if no instructions have been executed yet.
 
-  int m_current_offset_backed_up_one; // how far into the function we've
-                                      // executed; -1 if unknown
   // 0 if no instructions have been executed yet.
   // On architectures where the return address on the stack points
   // to the instruction after the CALL, this value will have 1
   // subtracted from it.  Else a function that ends in a CALL will
   // have an offset pointing into the next function's address range.
   // m_current_pc has the actual address of the "current" pc.
+  int m_current_offset_backed_up_one; // how far into the function we've
+                                      // executed; -1 if unknown
+
+  bool m_behaves_like_zeroth_frame; // this frame behaves like frame zero
 
   lldb_private::SymbolContext &m_sym_ctx;
   bool m_sym_ctx_valid; // if ResolveSymbolContextForAddress fails, don't try to
                         // use m_sym_ctx
 
   uint32_t m_frame_number; // What stack frame this RegisterContext is
 
   std::map<uint32_t, lldb_private::UnwindLLDB::RegisterLocation>

lldb/include/lldb/Target/StackFrame.h

public:
   /// Get an Address for the current pc value in this StackFrame.
   ///
   /// May not be the same as the actual PC value for inlined stack frames.
   ///
   /// \return
   ///   The Address object set to the current PC value.
   const Address &GetFrameCodeAddress();
 
+  /// Get the current code Address suitable for symbolication,
+  /// may not be the same as GetFrameCodeAddress().
+  ///
+  /// For a frame in the middle of the stack, the return-pc is the
+  /// current code address, but for symbolication purposes the
+  /// return address after a noreturn call may point to the next
+  /// function, a DWARF location list entry that is a completely
+  /// different code path, or the wrong source line.
+  ///
+  /// The address returned should be used for symbolication (source line,
+  /// block, function, DWARF location entry selection) but should NOT
+  /// be shown to the user.  It may not point to an actual instruction
+  /// boundary.
+  ///
+  /// \return
+  ///   The Address object set to the current PC value.
+  Address GetFrameCodeAddressForSymbolication();
+
   /// Change the pc value for a given thread.
   ///
   /// Change the current pc value for the frame on this thread.
   ///
   /// \param[in] pc
   ///     The load address that the pc will be set to.
   ///
   /// \return

lldb/source/Target/LanguageRuntime.cpp

if (exc_breakpt_sp) {
 
     if (is_internal)
       exc_breakpt_sp->SetBreakpointKind("exception");
   }
 
   return exc_breakpt_sp;
 }
 
-UnwindPlanSP LanguageRuntime::GetRuntimeUnwindPlan(Thread &thread,
-                                                   RegisterContext *regctx) {
+UnwindPlanSP
+LanguageRuntime::GetRuntimeUnwindPlan(Thread &thread, RegisterContext *regctx,
+                                      bool &behaves_like_zeroth_frame) {
   ProcessSP process_sp = thread.GetProcess();
   if (!process_sp.get())
     return UnwindPlanSP();
   for (const lldb::LanguageType lang_type : Language::GetSupportedLanguages()) {
     if (LanguageRuntime *runtime = process_sp->GetLanguageRuntime(lang_type)) {
-      UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(process_sp, regctx);
+      UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(
+          process_sp, regctx, behaves_like_zeroth_frame);
       if (plan_sp.get())
         return plan_sp;
     }
   }
   return UnwindPlanSP();
 }
 
 void LanguageRuntime::InitializeCommands(CommandObject *parent) {

lldb/source/Target/RegisterContext.cpp

if (success) {
     if (frame_sp)
       frame_sp->ChangePC(pc);
     else
       m_thread.ClearStackFrames();
   }
   return success;
 }
 
+bool RegisterContext::GetPCForSymbolication(Address &address) {
+  addr_t pc = GetPC(LLDB_INVALID_ADDRESS);
+  if (pc == LLDB_INVALID_ADDRESS)
+    return false;
+  TargetSP target_sp = m_thread.CalculateTarget();
+  if (!target_sp.get())
+    return false;
+
+  if (!BehavesLikeZerothFrame() && pc != 0)
+    pc--;
+  address.SetLoadAddress(pc, target_sp.get());
+  return true;
+}
+
 bool RegisterContext::SetPC(Address addr) {
   TargetSP target_sp = m_thread.CalculateTarget();
   Target *target = target_sp.get();
 
   lldb::addr_t callAddr = addr.GetCallableLoadAddress(target);
   if (callAddr == LLDB_INVALID_ADDRESS)
     return false;
 

lldb/source/Target/RegisterContextUnwind.cpp

RegisterContextUnwind::RegisterContextUnwind(Thread &thread,
                                              const SharedPtr &next_frame,
                                              SymbolContext &sym_ctx,
                                              uint32_t frame_number,
                                              UnwindLLDB &unwind_lldb)
     : RegisterContext(thread, frame_number), m_thread(thread),
       m_fast_unwind_plan_sp(), m_full_unwind_plan_sp(),
       m_fallback_unwind_plan_sp(), m_all_registers_available(false),
       m_frame_type(-1), m_cfa(LLDB_INVALID_ADDRESS),
-      m_afa(LLDB_INVALID_ADDRESS), m_start_pc(),
-      m_current_pc(), m_current_offset(0), m_current_offset_backed_up_one(0),
-      m_sym_ctx(sym_ctx), m_sym_ctx_valid(false), m_frame_number(frame_number),
-      m_registers(), m_parent_unwind(unwind_lldb) {
+      m_afa(LLDB_INVALID_ADDRESS), m_start_pc(), m_current_pc(),
+      m_current_offset(0), m_current_offset_backed_up_one(0),
+      m_behaves_like_zeroth_frame(false), m_sym_ctx(sym_ctx),
+      m_sym_ctx_valid(false), m_frame_number(frame_number), m_registers(),
+      m_parent_unwind(unwind_lldb) {
   m_sym_ctx.Clear(false);
   m_sym_ctx_valid = false;
 
   if (IsFrameZero()) {
     InitializeZerothFrame();
   } else {
     InitializeNonZerothFrame();
   }
void RegisterContextUnwind::InitializeZerothFrame() {
   // this will strip bit zero in case we read a PC from memory or from the LR.
   // (which would be a no-op in frame 0 where we get it from the register set,
   // but still a good idea to make the call here for other ABIs that may
   // exist.)
   ABI *abi = process->GetABI().get();
   if (abi)
     current_pc = abi->FixCodeAddress(current_pc);
 
+  UnwindPlanSP lang_runtime_plan_sp = LanguageRuntime::GetRuntimeUnwindPlan(
+      m_thread, this, m_behaves_like_zeroth_frame);
+  if (lang_runtime_plan_sp.get()) {
+    UnwindLogMsg("This is an async frame");
+  }
+
   // Initialize m_current_pc, an Address object, based on current_pc, an
   // addr_t.
   m_current_pc.SetLoadAddress(current_pc, &process->GetTarget());
 
   // If we don't have a Module for some reason, we're not going to find
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
void RegisterContextUnwind::InitializeZerothFrame() {
 
   // We've set m_frame_type and m_sym_ctx before these calls.
 
   m_fast_unwind_plan_sp = GetFastUnwindPlanForFrame();
   m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
 
   UnwindPlan::RowSP active_row;
   lldb::RegisterKind row_register_kind = eRegisterKindGeneric;
+
+  // If we have LanguageRuntime UnwindPlan for this unwind, use those
+  // rules to find the caller frame instead of the function's normal
+  // UnwindPlans.  The full unwind plan for this frame will be
+  // the LanguageRuntime-provided unwind plan, and there will not be a
+  // fast unwind plan.
+  if (lang_runtime_plan_sp.get()) {
+    active_row =
+        lang_runtime_plan_sp->GetRowForFunctionOffset(m_current_offset);
+    row_register_kind = lang_runtime_plan_sp->GetRegisterKind();
+    if (!ReadFrameAddress(row_register_kind, active_row->GetCFAValue(),
+                          m_cfa)) {
+      UnwindLogMsg("Cannot set cfa");
+    } else {
+      m_full_unwind_plan_sp = lang_runtime_plan_sp;
+      if (log) {
+        StreamString active_row_strm;
+        active_row->Dump(active_row_strm, lang_runtime_plan_sp.get(), &m_thread,
+                         m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
+        UnwindLogMsg("async active row: %s", active_row_strm.GetData());
+      }
+      UnwindLogMsg("m_cfa = 0x%" PRIx64 " m_afa = 0x%" PRIx64, m_cfa, m_afa);
+      UnwindLogMsg(
+          "initialized async frame current pc is 0x%" PRIx64
+          " cfa is 0x%" PRIx64 " afa is 0x%" PRIx64,
+          (uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
+          (uint64_t)m_cfa, (uint64_t)m_afa);
+
+      return;
+    }
+  }
+
   if (m_full_unwind_plan_sp &&
       m_full_unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
     active_row =
         m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
     row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
     if (active_row.get() && log) {
       StreamString active_row_strm;
       active_row->Dump(active_row_strm, m_full_unwind_plan_sp.get(), &m_thread,
void RegisterContextUnwind::InitializeNonZerothFrame() {
 
   // Some languages may have a logical parent stack frame which is
   // not a real stack frame, but the programmer would consider it to
   // be the caller of the frame, e.g. Swift asynchronous frames.
   //
   // A LanguageRuntime may provide an UnwindPlan that is used in this
   // stack trace base on the RegisterContext contents, intsead
   // of the normal UnwindPlans we would use for the return-pc.
-  UnwindPlanSP lang_runtime_plan_sp =
-      LanguageRuntime::GetRuntimeUnwindPlan(m_thread, this);
+  UnwindPlanSP lang_runtime_plan_sp = LanguageRuntime::GetRuntimeUnwindPlan(
+      m_thread, this, m_behaves_like_zeroth_frame);
   if (lang_runtime_plan_sp.get()) {
     UnwindLogMsg("This is an async frame");
   }
 
   addr_t pc;
   if (!ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC, pc)) {
     UnwindLogMsg("could not get pc value");
     m_frame_type = eNotAValidFrame;
if (!m_sym_ctx_valid) {
     // to decrement and recompute.
     decr_pc_and_recompute_addr_range = false;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
     // Signal dispatch may set the return address of the handler it calls to
     // point to the first byte of a return trampoline (like __kernel_rt_sigreturn),
     // so do not decrement and recompute if the symbol we already found is a trap
     // handler.
     decr_pc_and_recompute_addr_range = false;
+  } else if (m_behaves_like_zeroth_frame) {
+    decr_pc_and_recompute_addr_range = false;
   } else {
     // Decrement to find the function containing the call.
     decr_pc_and_recompute_addr_range = true;
   }
 
   // We need to back up the pc by 1 byte and re-search for the Symbol to handle
   // the case where the "saved pc" value is pointing to the next function, e.g.
   // if a function ends with a CALL instruction.
if (next_next_frame && next_next_frame->GetCFA(next_next_frame_cfa)) {
       }
     }
   }
   return false;
 }
 
 bool RegisterContextUnwind::IsFrameZero() const { return m_frame_number == 0; }
 
+bool RegisterContextUnwind::BehavesLikeZerothFrame() const {
+  if (m_frame_number == 0)
+    return true;
+  if (m_behaves_like_zeroth_frame)
+    return true;
+  return false;
+}
+
 // Find a fast unwind plan for this frame, if possible.
 //
 // On entry to this method,
 //
 //   1. m_frame_type should already be set to eTrapHandlerFrame/eDebuggerFrame
 //   if either of those are correct,
 //   2. m_sym_ctx should already be filled in, and
 //   3. m_current_pc should have the current pc value for this frame
if (abi) {
     arch_default_unwind_plan_sp =
         std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
     abi->CreateDefaultUnwindPlan(*arch_default_unwind_plan_sp);
   } else {
     UnwindLogMsg(
         "unable to get architectural default UnwindPlan from ABI plugin");
   }
 
-  bool behaves_like_zeroth_frame = false;
   if (IsFrameZero() || GetNextFrame()->m_frame_type == eTrapHandlerFrame ||
       GetNextFrame()->m_frame_type == eDebuggerFrame) {
-    behaves_like_zeroth_frame = true;
+    m_behaves_like_zeroth_frame = true;
     // If this frame behaves like a 0th frame (currently executing or
     // interrupted asynchronously), all registers can be retrieved.
     m_all_registers_available = true;
   }
 
   // If we've done a jmp 0x0 / bl 0x0 (called through a null function pointer)
   // so the pc is 0x0 in the zeroth frame, we need to use the "unwind at first
   // instruction" arch default UnwindPlan Also, if this Process can report on
   // memory region attributes, any non-executable region means we jumped
   // through a bad function pointer - handle the same way as 0x0. Note, if we
   // have a symbol context & a symbol, we don't want to follow this code path.
   // This is for jumping to memory regions without any information available.
 
   if ((!m_sym_ctx_valid ||
        (m_sym_ctx.function == nullptr && m_sym_ctx.symbol == nullptr)) &&
-      behaves_like_zeroth_frame && m_current_pc.IsValid()) {
+      m_behaves_like_zeroth_frame && m_current_pc.IsValid()) {
     uint32_t permissions;
     addr_t current_pc_addr =
         m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr());
     if (current_pc_addr == 0 ||
         (process &&
          process->GetLoadAddressPermissions(current_pc_addr, permissions) &&
          (permissions & ePermissionsExecutable) == 0)) {
       if (abi) {
if (unwind_plan_sp && unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
                           "DynamicLoader suggested we prefer it",
                           unwind_plan_sp->GetSourceName().GetCString());
       return unwind_plan_sp;
     }
   }
 
   // Typically the NonCallSite UnwindPlan is the unwind created by inspecting
   // the assembly language instructions
-  if (behaves_like_zeroth_frame && process) {
+  if (m_behaves_like_zeroth_frame && process) {
     unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtNonCallSite(
         process->GetTarget(), m_thread);
     if (unwind_plan_sp && unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
       if (unwind_plan_sp->GetSourcedFromCompiler() == eLazyBoolNo) {
         // We probably have an UnwindPlan created by inspecting assembly
         // instructions. The assembly profilers work really well with compiler-
         // generated functions but hand- written assembly can be problematic.
         // We set the eh_frame based unwind plan as our fallback unwind plan if

lldb/source/Target/StackFrame.cpp

if (thread_sp) {
           }
         }
       }
     }
   }
   return m_frame_code_addr;
 }
 
+// This can't be rewritten into a call to
+// RegisterContext::GetPCForSymbolication because this
+// StackFrame may have been constructed with a special pc,
+// e.g. tail-call artificial frames.
+Address StackFrame::GetFrameCodeAddressForSymbolication() {
+  Address lookup_addr(GetFrameCodeAddress());
+  if (!lookup_addr.IsValid())
+    return lookup_addr;
+  if (m_behaves_like_zeroth_frame)
+    return lookup_addr;
+
+  addr_t offset = lookup_addr.GetOffset();
+  if (offset > 0) {
+    lookup_addr.SetOffset(offset - 1);
+  } else {
+    // lookup_addr is the start of a section.  We need do the math on the
+    // actual load address and re-compute the section.  We're working with
+    // a 'noreturn' function at the end of a section.
+    TargetSP target_sp = CalculateTarget();
+    if (target_sp) {
+      addr_t addr_minus_one = lookup_addr.GetOpcodeLoadAddress(
+                                  target_sp.get(), AddressClass::eCode) -
+                              1;
+      lookup_addr.SetOpcodeLoadAddress(addr_minus_one, target_sp.get());
+    }
+  }
+  return lookup_addr;
+}
+
 bool StackFrame::ChangePC(addr_t pc) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   // We can't change the pc value of a history stack frame - it is immutable.
   if (IsHistorical())
     return false;
   m_frame_code_addr.SetRawAddress(pc);
   m_sc.Clear(false);
   m_flags.Reset(0);
if ((m_flags.Get() & resolve_scope) != resolve_scope) {
     // don't have a module. The resolved address section will contain the
     // module to which it belongs
     if (!m_sc.module_sp && m_flags.IsClear(RESOLVED_FRAME_CODE_ADDR))
       GetFrameCodeAddress();
 
     // If this is not frame zero, then we need to subtract 1 from the PC value
     // when doing address lookups since the PC will be on the instruction
     // following the function call instruction...
-
-    Address lookup_addr(GetFrameCodeAddress());
-    if (!m_behaves_like_zeroth_frame && lookup_addr.IsValid()) {
-      addr_t offset = lookup_addr.GetOffset();
-      if (offset > 0) {
-        lookup_addr.SetOffset(offset - 1);
-
-      } else {
-        // lookup_addr is the start of a section.  We need do the math on the
-        // actual load address and re-compute the section.  We're working with
-        // a 'noreturn' function at the end of a section.
-        ThreadSP thread_sp(GetThread());
-        if (thread_sp) {
-          TargetSP target_sp(thread_sp->CalculateTarget());
-          if (target_sp) {
-            addr_t addr_minus_one =
-                lookup_addr.GetLoadAddress(target_sp.get()) - 1;
-            lookup_addr.SetLoadAddress(addr_minus_one, target_sp.get());
-          } else {
-            lookup_addr.SetOffset(offset - 1);
-          }
-        }
-      }
-    }
+    Address lookup_addr(GetFrameCodeAddressForSymbolication());
 
     if (m_sc.module_sp) {
       // We have something in our stack frame symbol context, lets check if we
       // haven't already tried to lookup one of those things. If we haven't
       // then we will do the query.
 
       SymbolContextItem actual_resolve_scope = SymbolContextItem(0);
 

lldb/source/Target/StackFrameList.cpp

if (idx == 0) {
 
       m_frames.push_back(unwind_frame_sp);
     }
 
     assert(unwind_frame_sp);
     SymbolContext unwind_sc = unwind_frame_sp->GetSymbolContext(
         eSymbolContextBlock | eSymbolContextFunction);
     Block *unwind_block = unwind_sc.block;
-    if (unwind_block) {
-      Address curr_frame_address(unwind_frame_sp->GetFrameCodeAddress());
-      TargetSP target_sp = m_thread.CalculateTarget();
-      // Be sure to adjust the frame address to match the address that was
-      // used to lookup the symbol context above. If we are in the first
-      // concrete frame, then we lookup using the current address, else we
-      // decrement the address by one to get the correct location.
-      if (idx > 0) {
-        if (curr_frame_address.GetOffset() == 0) {
-          // If curr_frame_address points to the first address in a section
-          // then after adjustment it will point to an other section. In that
-          // case resolve the address again to the correct section plus
-          // offset form.
-          addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(
-              target_sp.get(), AddressClass::eCode);
-          curr_frame_address.SetOpcodeLoadAddress(
-              load_addr - 1, target_sp.get(), AddressClass::eCode);
-        } else {
-          curr_frame_address.Slide(-1);
-        }
-      }
+    if (unwind_block) {
+      Address curr_frame_address(
+          unwind_frame_sp->GetFrameCodeAddressForSymbolication());
 
       SymbolContext next_frame_sc;
       Address next_frame_address;
 
       while (unwind_sc.GetParentOfInlinedScope(
           curr_frame_address, next_frame_sc, next_frame_address)) {
         next_frame_sc.line_entry.ApplyFileMappings(target_sp);
         behaves_like_zeroth_frame = false;

lldb/source/Target/UnwindLLDB.cpp

if (idx == 0) {
       behaves_like_zeroth_frame = true;
     } else if (m_frames[idx]->reg_ctx_lldb_sp->IsTrapHandlerFrame()) {
       // This frame may result from signal processing installing
       // a pointer to the first byte of a signal-return trampoline
       // in the return address slot of the frame below, so this
       // too behaves like the zeroth frame (i.e. the pc might not
       // be pointing just past a call in it)
       behaves_like_zeroth_frame = true;
+    } else if (m_frames[idx]->reg_ctx_lldb_sp->BehavesLikeZerothFrame()) {
+      behaves_like_zeroth_frame = true;
     } else {
       behaves_like_zeroth_frame = false;
     }
     return true;
   }
   return false;
 }