Fix PC adjustment in StackFrame::GetSymbolContext

Summary:
Update StackFrame::GetSymbolContext to mirror the logic in
RegisterContextLLDB::InitializeNonZerothFrame that knows not to do the
pc decrement when the given frame is a signal trap handler frame or the
parent of one, because the pc may not follow a call in these frames.
Accomplish this by adding a behaves_like_zeroth_frame field to
lldb_private::StackFrame, set to true for the zeroth frame, for
signal handler frames, and for parents of signal handler frames.

Also add logic to propagate the signal handler flag from UnwindPlan to
the FrameType on the RegisterContextLLDB it generates, and factor out a
helper to resolve symbol and address range for an Address now that we
need to invoke it in four places.

Reviewers: jasonmolenda, clayborg, jfb

Reviewed By: jasonmolenda

Subscribers: labath, dexonsmith, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D64993

llvm-svn: 367691

8 files changed, 94 insertions, 55 deletions

diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
index 7d473bff820..83fdb011f5a 100644
--- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
+++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
@@ -51,13 +51,15 @@ HistoryUnwind::DoCreateRegisterContextForFrame(StackFrame *frame) {
 }
 
 bool HistoryUnwind::DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
-                                          lldb::addr_t &pc) {
+                                          lldb::addr_t &pc,
+                                          bool &behaves_like_zeroth_frame) {
   // FIXME do not throw away the lock after we acquire it..
   std::unique_lock<std::recursive_mutex> guard(m_unwind_mutex);
   guard.unlock();
   if (frame_idx < m_pcs.size()) {
     cfa = frame_idx;
     pc = m_pcs[frame_idx];
+    behaves_like_zeroth_frame = (frame_idx == 0);
     return true;
   }
   return false;
diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h
index 6c4522e6b35..4d16608bd8c 100644
--- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h
+++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h
@@ -29,7 +29,8 @@ protected:
   DoCreateRegisterContextForFrame(StackFrame *frame) override;
 
   bool DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
-                             lldb::addr_t &pc) override;
+                             lldb::addr_t &pc,
+                             bool &behaves_like_zeroth_frame) override;
   uint32_t DoGetFrameCount() override;
 
 private:
diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp b/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
index 62f38dcd85a..27341267a8a 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
@@ -150,15 +150,8 @@ void RegisterContextLLDB::InitializeZerothFrame() {
     UnwindLogMsg("using architectural default unwind method");
   }
 
-  // We require either a symbol or function in the symbols context to be
-  // successfully filled in or this context is of no use to us.
-  const SymbolContextItem resolve_scope =
-      eSymbolContextFunction | eSymbolContextSymbol;
-  if (pc_module_sp.get() && (pc_module_sp->ResolveSymbolContextForAddress(
-                                 m_current_pc, resolve_scope, m_sym_ctx) &
-                             resolve_scope)) {
-    m_sym_ctx_valid = true;
-  }
+  AddressRange addr_range;
+  m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
 
   if (m_sym_ctx.symbol) {
     UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'",
@@ -172,9 +165,6 @@ void RegisterContextLLDB::InitializeZerothFrame() {
                  current_pc);
   }
 
-  AddressRange addr_range;
-  m_sym_ctx.GetAddressRange(resolve_scope, 0, false, addr_range);
-
   if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
     m_frame_type = eTrapHandlerFrame;
   } else {
@@ -436,24 +426,8 @@ void RegisterContextLLDB::InitializeNonZerothFrame() {
     return;
   }
 
-  bool resolve_tail_call_address = false; // m_current_pc can be one past the
-                                          // address range of the function...
-  // If the saved pc does not point to a function/symbol because it is beyond
-  // the bounds of the correct function and there's no symbol there, we do
-  // *not* want ResolveSymbolContextForAddress to back up the pc by 1, because
-  // then we might not find the correct unwind information later. Instead, let
-  // ResolveSymbolContextForAddress fail, and handle the case via
-  // decr_pc_and_recompute_addr_range below.
-  const SymbolContextItem resolve_scope =
-      eSymbolContextFunction | eSymbolContextSymbol;
-  uint32_t resolved_scope = pc_module_sp->ResolveSymbolContextForAddress(
-      m_current_pc, resolve_scope, m_sym_ctx, resolve_tail_call_address);
-
-  // We require either a symbol or function in the symbols context to be
-  // successfully filled in or this context is of no use to us.
-  if (resolve_scope & resolved_scope) {
-    m_sym_ctx_valid = true;
-  }
+  AddressRange addr_range;
+  m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
 
   if (m_sym_ctx.symbol) {
     UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'", pc,
@@ -467,11 +441,6 @@ void RegisterContextLLDB::InitializeNonZerothFrame() {
                  pc);
   }
 
-  AddressRange addr_range;
-  if (!m_sym_ctx.GetAddressRange(resolve_scope, 0, false, addr_range)) {
-    m_sym_ctx_valid = false;
-  }
-
   bool decr_pc_and_recompute_addr_range;
 
   if (!m_sym_ctx_valid) {
@@ -512,18 +481,8 @@ void RegisterContextLLDB::InitializeNonZerothFrame() {
     Address temporary_pc;
     temporary_pc.SetLoadAddress(pc - 1, &process->GetTarget());
     m_sym_ctx.Clear(false);
-    m_sym_ctx_valid = false;
-    SymbolContextItem resolve_scope =
-        eSymbolContextFunction | eSymbolContextSymbol;
-
-    ModuleSP temporary_module_sp = temporary_pc.GetModule();
-    if (temporary_module_sp &&
-        temporary_module_sp->ResolveSymbolContextForAddress(
-            temporary_pc, resolve_scope, m_sym_ctx) &
-            resolve_scope) {
-      if (m_sym_ctx.GetAddressRange(resolve_scope, 0, false, addr_range))
-        m_sym_ctx_valid = true;
-    }
+    m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
+
     UnwindLogMsg("Symbol is now %s",
                  GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
   }
@@ -573,6 +532,7 @@ void RegisterContextLLDB::InitializeNonZerothFrame() {
     active_row =
         m_fast_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
     row_register_kind = m_fast_unwind_plan_sp->GetRegisterKind();
+    PropagateTrapHandlerFlagFromUnwindPlan(m_fast_unwind_plan_sp);
     if (active_row.get() && log) {
       StreamString active_row_strm;
       active_row->Dump(active_row_strm, m_fast_unwind_plan_sp.get(), &m_thread,
@@ -585,6 +545,7 @@ void RegisterContextLLDB::InitializeNonZerothFrame() {
     if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
       active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
       row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
+      PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
       if (active_row.get() && log) {
         StreamString active_row_strm;
         active_row->Dump(active_row_strm, m_full_unwind_plan_sp.get(),
@@ -1708,6 +1669,7 @@ bool RegisterContextLLDB::TryFallbackUnwindPlan() {
     // We've copied the fallback unwind plan into the full - now clear the
     // fallback.
     m_fallback_unwind_plan_sp.reset();
+    PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
   }
 
   return true;
@@ -1751,6 +1713,8 @@ bool RegisterContextLLDB::ForceSwitchToFallbackUnwindPlan() {
 
     m_cfa = new_cfa;
 
+    PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
+
     UnwindLogMsg("switched unconditionally to the fallback unwindplan %s",
                  m_full_unwind_plan_sp->GetSourceName().GetCString());
     return true;
@@ -1758,6 +1722,53 @@ bool RegisterContextLLDB::ForceSwitchToFallbackUnwindPlan() {
   return false;
 }
 
+void RegisterContextLLDB::PropagateTrapHandlerFlagFromUnwindPlan(
+    lldb::UnwindPlanSP unwind_plan) {
+  if (unwind_plan->GetUnwindPlanForSignalTrap() != eLazyBoolYes) {
+    // Unwind plan does not indicate trap handler.  Do nothing.  We may
+    // already be flagged as trap handler flag due to the symbol being
+    // in the trap handler symbol list, and that should take precedence.
+    return;
+  } else if (m_frame_type != eNormalFrame) {
+    // If this is already a trap handler frame, nothing to do.
+    // If this is a skip or debug or invalid frame, don't override that.
+    return;
+  }
+
+  m_frame_type = eTrapHandlerFrame;
+
+  if (m_current_offset_backed_up_one != m_current_offset) {
+    // We backed up the pc by 1 to compute the symbol context, but
+    // now need to undo that because the pc of the trap handler
+    // frame may in fact be the first instruction of a signal return
+    // trampoline, rather than the instruction after a call.  This
+    // happens on systems where the signal handler dispatch code, rather
+    // than calling the handler and being returned to, jumps to the
+    // handler after pushing the address of a return trampoline on the
+    // stack -- on these systems, when the handler returns, control will
+    // be transferred to the return trampoline, so that's the best
+    // symbol we can present in the callstack.
+    UnwindLogMsg("Resetting current offset and re-doing symbol lookup; "
+                 "old symbol was %s",
+                 GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
+    m_current_offset_backed_up_one = m_current_offset;
+
+    AddressRange addr_range;
+    m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
+
+    UnwindLogMsg("Symbol is now %s",
+                 GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
+
+    ExecutionContext exe_ctx(m_thread.shared_from_this());
+    Process *process = exe_ctx.GetProcessPtr();
+    Target *target = &process->GetTarget();
+
+    m_start_pc = addr_range.GetBaseAddress();
+    m_current_offset =
+        m_current_pc.GetLoadAddress(target) - m_start_pc.GetLoadAddress(target);
+  }
+}
+
 bool RegisterContextLLDB::ReadFrameAddress(
     lldb::RegisterKind row_register_kind, UnwindPlan::Row::FAValue &fa,
     addr_t &address) {
diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h b/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
index 64dd394d233..1ca663906ed 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
@@ -120,6 +120,10 @@ private:
   bool IsTrapHandlerSymbol(lldb_private::Process *process,
                            const lldb_private::SymbolContext &m_sym_ctx) const;
 
+  /// Check if the given unwind plan indicates a signal trap handler, and
+  /// update frame type and symbol context if so.
+  void PropagateTrapHandlerFlagFromUnwindPlan(lldb::UnwindPlanSP unwind_plan);
+
   // Provide a location for where THIS function saved the CALLER's register
   // value
   // Or a frame "below" this one saved it, i.e. a function called by this one,
diff --git a/lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp b/lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
index 4b135351593..74fc90e8854 100644
--- a/lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
+++ b/lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
@@ -390,7 +390,8 @@ bool UnwindLLDB::AddOneMoreFrame(ABI *abi) {
   return true;
 }
 
-bool UnwindLLDB::DoGetFrameInfoAtIndex(uint32_t idx, addr_t &cfa, addr_t &pc) {
+bool UnwindLLDB::DoGetFrameInfoAtIndex(uint32_t idx, addr_t &cfa, addr_t &pc,
+                                       bool &behaves_like_zeroth_frame) {
   if (m_frames.size() == 0) {
     if (!AddFirstFrame())
       return false;
@@ -405,6 +406,24 @@ bool UnwindLLDB::DoGetFrameInfoAtIndex(uint32_t idx, addr_t &cfa, addr_t &pc) {
   if (idx < m_frames.size()) {
     cfa = m_frames[idx]->cfa;
     pc = m_frames[idx]->start_pc;
+    if (idx == 0) {
+      // Frame zero always behaves like it.
+      behaves_like_zeroth_frame = true;
+    } else if (m_frames[idx - 1]->reg_ctx_lldb_sp->IsTrapHandlerFrame()) {
+      // This could be an asynchronous signal, thus the
+      // pc might point to the interrupted instruction rather
+      // than a post-call instruction
+      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 {
+      behaves_like_zeroth_frame = false;
+    }
     return true;
   }
   return false;
diff --git a/lldb/source/Plugins/Process/Utility/UnwindLLDB.h b/lldb/source/Plugins/Process/Utility/UnwindLLDB.h
index c512929c185..ff5db39730b 100644
--- a/lldb/source/Plugins/Process/Utility/UnwindLLDB.h
+++ b/lldb/source/Plugins/Process/Utility/UnwindLLDB.h
@@ -73,7 +73,8 @@ protected:
   uint32_t DoGetFrameCount() override;
 
   bool DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
-                             lldb::addr_t &start_pc) override;
+                             lldb::addr_t &start_pc,
+                             bool &behaves_like_zeroth_frame) override;
 
   lldb::RegisterContextSP
   DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;
diff --git a/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp b/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
index 7dc5a5f5fdd..558edeec1a3 100644
--- a/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
+++ b/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
@@ -43,9 +43,8 @@ uint32_t UnwindMacOSXFrameBackchain::DoGetFrameCount() {
   return m_cursors.size();
 }
 
-bool UnwindMacOSXFrameBackchain::DoGetFrameInfoAtIndex(uint32_t idx,
-                                                       addr_t &cfa,
-                                                       addr_t &pc) {
+bool UnwindMacOSXFrameBackchain::DoGetFrameInfoAtIndex(
+    uint32_t idx, addr_t &cfa, addr_t &pc, bool &behaves_like_zeroth_frame) {
   const uint32_t frame_count = GetFrameCount();
   if (idx < frame_count) {
     if (m_cursors[idx].pc == LLDB_INVALID_ADDRESS)
@@ -55,6 +54,7 @@ bool UnwindMacOSXFrameBackchain::DoGetFrameInfoAtIndex(uint32_t idx,
 
     pc = m_cursors[idx].pc;
     cfa = m_cursors[idx].fp;
+    behaves_like_zeroth_frame = (idx == 0);
 
     return true;
   }
diff --git a/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h b/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
index 2208bcc2f2e..f0bde90a53b 100644
--- a/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
+++ b/lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
@@ -26,7 +26,8 @@ protected:
   uint32_t DoGetFrameCount() override;
 
   bool DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
-                             lldb::addr_t &pc) override;
+                             lldb::addr_t &pc,
+                             bool &behaves_like_zeroth_frame) override;
 
   lldb::RegisterContextSP
   DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;