[LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

[zequanwu]

Summary

This adds stack scanning as fallback unwind plan to find a value that looks like
return address. If the return address is not valid, continue scanning from next
locatoin on stack when trying fallback unwind and the active row is raSearch.

[clayborg]

It is unclear how this patch works. Was there support for searching for the return address on the stack already? And this patch is just adding the a few register unwind rules for when/if the return address _is_ found for x86?

This will need tests.

lldb/include/lldb/Symbol/UnwindPlan.h

225

What is this magic number/bit here? Is it ok to clobber bit zero here?

263

Are we assuming "search_offset" must be aligned to at least a 4 bit boundary so that we can put something in bit zero?

267

You are using & and | in other locations, and now we are using modulo? I would just keep with the & operators

lldb/include/lldb/Target/ABI.h

99

Add a default implementation that returns false so you don't have to change all of the classes that don't implement this.

lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h

30–33 ↗

(On Diff #424268)

Don't make the CreateStackWalkingUnwindPlan pure virtual if most of the implementations are just "return false;". I would make a default implementation in the base class that just returns false.

lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp

734

What code actually does the search for a return address? Is that already available somewhere in the unwind plans? Reading through this plan I see that it sets the CFA to be RA search but I fail to see any searching going on.

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1258 ↗

(On Diff #424268)

all changes in this file don't seem related to this patch? Revert?

[clayborg]

If we do find what looks like a return address, is there any validation done on the instruction before the return address to see if it is a function call instruction? That would be the best way to validate that something on the stack just doesn't look like a return address and is actually just a random function pointer. I would like to see validation on anything that looks like a return address like:

• make sure it doesn't point to the first address of a function since if it actually is a return address is should be in the middle of a function
• make sure the previous instruction was a function call to the function the current function from which we are unwinding if we have function bounds for the current frame
• if we don't have function bounds for the current function, we can benefit from knowing the function start address in the current frame if we can find the call instruction since that will help us parse the correct function prologue

[jasonmolenda]

This is obviously building on Pavel's work of https://reviews.llvm.org/D66638 for Win32 systems. What problem with that existing patchset is this addressing? Pavel's original patch assumes that we can retrieve a function's expected stack frame size, and the size of the caller's arguments on the stack, plus a bit of searching around that location, as he describes. It seems like in ABIWindows_x86_64::CreateStackWalkingUnwindPlan you're adding a scheme that tries to look at the area directly around $sp for a return pc? This seems quite fragile by itself.

The idea of a stack walk based purely on looking at stack contents, outside of the Windows ABI, is interesting, but as Pavel mentions in his descriptions, you have to be careful to not get tricked by function pointers on the stack (for return-pc's) and pointers to stack memory (for spilled fp or sp values). If we have an ABI that always writes fp+return-pc to stack at the start of the stack frame for non-leaf frames, we could maybe do something as a last resort, I haven't thought about it.

Greg's feedback is all solid. I'm more curious about what higher level fix is being pursued with this patch. Pavel is clearly the person who knows the most about this part of the unwinder.

[jasonmolenda]

I guess the other comment I would make is whether we should have a new ivar, instead of using the unused 0th bit of search_offset.

fwiw I wasn't real clear in my previous comment. As you say in the title of this, you're trying to backtrace through a case where you have no symbol file, and I'm guessing the Windows ABI doesn't use a frame pointer register (rbp) that is is spilled to stack at the start of a stack frame by convention. The ArchitecturalDefaultUnwindPlan's that we use on ABIs where you see saved-pc followed by saved-fp can do a stack walk following the chain of those two spilled registers pretty reliably, once they're off of the leaf frame. This isn't the case on Windows right. You have nothing but a saved return address, and the stack pointer is not spilled unless there's some kind of variable length stack frame I'm guessing.

[zequanwu]

Thanks for the feedback. As you pointed out that stack scanning without symbol file info is fragile due to function pointers in stack and we don't know the parameter size pushed into the stack.

Originally, I want lldb to do continue unwinding when it encounters some frames are in modules that don't have symbol info and no rbp value, since we have some crash reports with such problem. But later I found that we actually do have the symbol files for those modules. It seems not necessary anymore...

[labath]

The patch description could definitely use more details about the motivation for the change, and a description of how it works. Apart from the fact that it uses the raSearch feature I introduced a while back, I don't know much about how it works (and given the planned changes, I am not going to try to understand that), but I think I can provide some details/thoughts about the motivation.

(IIUC), the motivating case is some code in kernel32.dll, which ends up setting rbp to zero. I don't know why it does that, but I suspect it may have something to do with the windows syscall convention (I know that ebp is used in i386 linux, for instance). While it would be definitely interesting to understand why it does that, I don't think it really matters -- one can never really exclude the possibility that some code (inline asm perhaps) will be messing with the frame pointer. So, scanning the stack for return addresses as a *last* last resort (the architectural default plan is already a kind of a last resort option) seems like it could be useful, regardless of the architecture (although the actual algorithm may differ somewhat for x86, which stores the return address on the stack directly, and say arm, which uses a link register). And given that the choice is between "showing nothing" and "showing something potentially incorrect", I don't think we need to be extra careful about vetting this address. All the checks Greg mentioned seem fine, but I don't see a compelling reason to implement them (all of them), especially given that they would not be useful in this case -- all we have is a minidump core file with some stack memory and a list of loaded modules (not necessarily the modules themselves). In this case, all we can do is to check whether the pointer points to some known code region (which is already done IIRC).

[clayborg]

In D124198#3466889, @labath wrote:

The patch description could definitely use more details about the motivation for the change, and a description of how it works. Apart from the fact that it uses the raSearch feature I introduced a while back, I don't know much about how it works (and given the planned changes, I am not going to try to understand that), but I think I can provide some details/thoughts about the motivation.

(IIUC), the motivating case is some code in kernel32.dll, which ends up setting rbp to zero. I don't know why it does that, but I suspect it may have something to do with the windows syscall convention (I know that ebp is used in i386 linux, for instance). While it would be definitely interesting to understand why it does that, I don't think it really matters -- one can never really exclude the possibility that some code (inline asm perhaps) will be messing with the frame pointer. So, scanning the stack for return addresses as a *last* last resort (the architectural default plan is already a kind of a last resort option) seems like it could be useful, regardless of the architecture (although the actual algorithm may differ somewhat for x86, which stores the return address on the stack directly, and say arm, which uses a link register). And given that the choice is between "showing nothing" and "showing something potentially incorrect", I don't think we need to be extra careful about vetting this address. All the checks Greg mentioned seem fine, but I don't see a compelling reason to implement them (all of them), especially given that they would not be useful in this case -- all we have is a minidump core file with some stack memory and a list of loaded modules (not necessarily the modules themselves). In this case, all we can do is to check whether the pointer points to some known code region (which is already done IIRC).

We wrote a command in python called "sbt" and we run this when stacks are truncated when loading minidumps. It often gives us pretty bad results and you would be surprised how many things on the stack look like code pointers. So if we do have the ability to do the checks when we do have the code and can try to verify that the thing on the stack is a return address, then it would be great to do so to avoid the noise. And if we are in code that has no symbols and we don't know the start address of the function, it would be VERY helpful to correctly backtrace if we _can_ determine the start address of the current frame's function if we don't know it as then we can correctly parse the prologue. So adding these checks might actually get us back on track and allow us not to have to use the RA search. The flow would be:

• enable RA search in a frame that has no function bounds info from the object files or runtime info
• find the right thing on the stack that is the correct RA by finding the instruction before the RA and figuring out what address it calls
• use the new function address to create a real stack frame that uses opcode parsing

So there is real benefit to doing these checks IMHO as it can help us get back on track in live debug sessions. Core file sessions results will vary depending on it we actually have the original object files or not, many times we do not have this info, in which case we would just enable RA search and we might be able to unwind the PC, but not other registers if we can't actually find the prologue or if we don't have stack frame details from breakpad

[clayborg]

If anyone wants to see the "sbt" python command, I am happy to share it

[clayborg]

"sbt" == stack back trace

Just looks at the stack for things that look like code and tries to make something that looks like a backtrace

[jingham]

I also wonder if this shouldn't require a separate flag to turn on this search, or be a separate command in the way Greg did. People rely on backtraces being accurate. It's fine to add something more like a desperation play - something is sometimes better than nothing, but if we are presenting a view that we don't have full confidence in, we really should tell users that so they can factor it into their analysis.

[labath]

In D124198#3468039, @clayborg wrote:

So there is real benefit to doing these checks IMHO as it can help us get back on track in live debug sessions. Core file sessions results will vary depending on it we actually have the original object files or not, many times we do not have this info, in which case we would just enable RA search and we might be able to unwind the PC, but not other registers if we can't actually find the prologue or if we don't have stack frame details from breakpad

I'm not saying we can't do that, but I don't think it has done be done straight in the first version. A lot of these checks are really just heuristics and can have false positives. For example, if we disqualify return addresses pointing to the first instruction of a function, then these cases won't work:

• noreturn functions (the call could be the last instruction in the previous function)
• signal handler frames, where the handler's return address points to the first instruction of the trampoline (because it didn't really call it, but it still knows how to unwind from it)

I think it would be better discuss individual checks independently of the overall infrastructure (and ideally with more data).

That said, it would definitely be nice give some indication of how much do we trust the backtrace that we've computed. A separate command or a setting kind of achieve that, but they're very coarse-grained (you don't know how much trust to put into a particular backtrace), and they need the user to know of their existence. I am wondering whether attaching some kind of flag to a particular frame (similar to how we add [opt] for optimized functions) to indicate that it may not be entirely correct could be a better option.

[zequanwu]

Update: address some inline comments.

For testing, I don't know how to create minidump in yaml format like the one in lldb/test/Shell/SymbolFile/Breakpad/Inputs/unwind-via-stack-win.yaml. Anyone knows how? Or have other ideas for testing?

I am wondering whether attaching some kind of flag to a particular frame (similar to how we add [opt] for optimized functions) to indicate that it may not be entirely correct could be a better option.

I'm lean towards this idea that adding a flag to the frame unwinded from stack scanning, maybe all frames after that one should have the flag.

lldb/include/lldb/Symbol/UnwindPlan.h

225

When the last bit isn't set, it's the first ra search.

The reason why we want to know this info is because after we got cfa via stack scanning in TryFallbackUnwindPlan when initializing zeroth/nonzeroth frames in RegisterContextUnwind, we will use that cfa for unwind next frame. If the next frame is valid, we are done. Otherwise, we get the incorrect cfa for the parent frame. Then we need to redo the stack scanning to find parent frame's cfa (UnwindLLDB::GetOneMoreFrame) but should skip the region that we have already scanned. So, the search_offset tells how many bytes should we skip.

263

search_offset is usually the multiple of 4 or 8, so we can use the last bit.

lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp

734

This sets the unwind plan row to be ra search. The actual ra search happens on RegisterContextUnwind::ReadFrameAddress at the case UnwindPlan::Row::FAValue::isRaSearch.

[zequanwu]

Use extra bool variable for is_first_search, because search_offset might be an odd number if parameter byte size in stack is an odd number.

lldb/include/lldb/Symbol/UnwindPlan.h

263

Updated:
Use extra bool variable for is_first_search, because search_offset might be an odd number if parameter byte size in stack is an odd number.

lldb/include/lldb/Symbol/UnwindPlan.h

public:
       bool operator==(const FAValue &rhs) const;
 
       bool operator!=(const FAValue &rhs) const { return !(*this == rhs); }
 
       void SetUnspecified() { m_type = unspecified; }
 
       bool IsUnspecified() const { return m_type == unspecified; }
 
-      void SetRaSearch(int32_t offset) {
+      bool IsRaSearch() const { return m_type == isRaSearch; }
+
+      void SetRaSearch(int32_t search_offset, int32_t cfa_offset) {
+        m_value.ra_search.search_offset = search_offset;
+        m_value.ra_search.cfa_offset = cfa_offset;
+        m_value.ra_search.is_first_search = m_type == unspecified;
         m_type = isRaSearch;
-        m_value.ra_search_offset = offset;
       }
 
       bool IsRegisterPlusOffset() const {
         return m_type == isRegisterPlusOffset;
       }
 
       void SetIsRegisterPlusOffset(uint32_t reg_num, int32_t offset) {
         m_type = isRegisterPlusOffset;
         m_value.reg.reg_num = reg_num;
public:
       uint32_t GetRegisterNumber() const {
         if (m_type == isRegisterDereferenced || m_type == isRegisterPlusOffset)
           return m_value.reg.reg_num;
         return LLDB_INVALID_REGNUM;
       }
 
       ValueType GetValueType() const { return m_type; }
 
+      int32_t GetRaSearchOffset() const {
+        return m_type == isRaSearch ? m_value.ra_search.search_offset : 0;
+      }
+
+      bool IsFirstRaSearch() const { return m_value.ra_search.is_first_search; }
+
       int32_t GetOffset() const {
         switch (m_type) {
           case isRegisterPlusOffset:
             return m_value.reg.offset;
           case isRaSearch:
-            return m_value.ra_search_offset;
+            return m_value.ra_search.cfa_offset;
           default:
             return 0;
         }
       }
 
       void IncOffset(int32_t delta) {
         if (m_type == isRegisterPlusOffset)
           m_value.reg.offset += delta;
private:
           int32_t offset;
         } reg;
         // For m_type == isDWARFExpression
         struct {
           const uint8_t *opcodes;
           uint16_t length;
         } expr;
         // For m_type == isRaSearch
-        int32_t ra_search_offset;
+        struct {
+          bool is_first_search;
+          int32_t search_offset;
+          int32_t cfa_offset;
+        } ra_search;
       } m_value;
     }; // class FAValue
 
     Row();
 
     bool operator==(const Row &rhs) const;
 
     bool GetRegisterInfo(uint32_t reg_num,

lldb/include/lldb/Target/ABI.h

protected:
   ///
   /// This ABI object may not have been created with a Process object,
   /// or the Process object may no longer be alive.  Be sure to handle
   /// the case where the shared pointer returned does not have an
   /// object inside it.
   lldb::ProcessSP GetProcessSP() const { return m_process_wp.lock(); }
 
 public:
+  virtual bool CreateStackWalkingUnwindPlan(UnwindPlan &unwind_plan) {
+    return false;
+  }
+
   virtual bool CreateFunctionEntryUnwindPlan(UnwindPlan &unwind_plan) = 0;
 
   virtual bool CreateDefaultUnwindPlan(UnwindPlan &unwind_plan) = 0;
 
   virtual bool RegisterIsVolatile(const RegisterInfo *reg_info) = 0;
 
   virtual bool
   GetFallbackRegisterLocation(const RegisterInfo *reg_info,

lldb/include/lldb/Target/RegisterContextUnwind.h

private:
   void UnwindLogMsg(const char *fmt, ...) __attribute__((format(printf, 2, 3)));
 
   void UnwindLogMsgVerbose(const char *fmt, ...)
       __attribute__((format(printf, 2, 3)));
 
   bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
                                      int &valid_pc_offset);
 
-  lldb::addr_t GetReturnAddressHint(int32_t plan_offset);
+  lldb::addr_t GetParameterStackSize();
 
   lldb_private::Thread &m_thread;
 
   ///
   // The following tell us how to retrieve the CALLER's register values (ie the
   // "previous" frame, aka the frame above)
   // i.e. where THIS frame saved them
   ///

lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h

public:
   SetReturnValueObject(lldb::StackFrameSP &frame_sp,
                        lldb::ValueObjectSP &new_value) override;
 
   lldb::ValueObjectSP
   GetReturnValueObjectImpl(lldb_private::Thread &thread,
                            lldb_private::CompilerType &type) const override;
 
   bool
+  CreateStackWalkingUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool
   CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
 
   bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
 
   bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
 
   // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {

lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp

lldb::addr_t storage_addr =
         (uint64_t)thread.GetRegisterContext()->ReadRegisterAsUnsigned(rax_id,
                                                                       0);
     return_valobj_sp = ValueObjectMemory::Create(
         &thread, "", Address(storage_addr, nullptr), return_compiler_type);
   }
   return return_valobj_sp;
 }
 
+bool ABIWindows_x86_64::CreateStackWalkingUnwindPlan(UnwindPlan &unwind_plan) {
+  unwind_plan.Clear();
+  unwind_plan.SetRegisterKind(eRegisterKindLLDB);
+
+  uint32_t sp_reg_num = dwarf_rsp;
+  uint32_t pc_reg_num = dwarf_rip;
+
+  UnwindPlan::RowSP row(new UnwindPlan::Row);
+  row->GetCFAValue().SetRaSearch(0, 8);
+  row->SetUnspecifiedRegistersAreUndefined(true);
+
+  row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, -8, true);
+  row->SetRegisterLocationToIsCFAPlusOffset(sp_reg_num, 0, true);
+  unwind_plan.AppendRow(row);
+  unwind_plan.SetSourceName("x86_64 stack-walking unwind plan");
+  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
+  return true;
+}
+
 // This defines the CFA as rsp+8
 // the saved pc is at CFA-8 (i.e. rsp+0)
 // The saved rsp is CFA+0
 
 bool ABIWindows_x86_64::CreateFunctionEntryUnwindPlan(UnwindPlan &unwind_plan) {
   unwind_plan.Clear();
   unwind_plan.SetRegisterKind(eRegisterKindDWARF);
 

lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

if (const RegisterInfo *info = ResolveRegister(triple, resolver, name))
           node_alloc, info->kinds[eRegisterKindLLDB]);
     return nullptr;
   };
 
   // We assume the first value will be the CFA. It is usually called T0, but
   // clang will use T1, if it needs to realign the stack.
   auto *symbol = llvm::dyn_cast<postfix::SymbolNode>(it->second);
   if (symbol && symbol->GetName() == ".raSearch") {
-    row_sp->GetCFAValue().SetRaSearch(record->LocalSize +
-                                      record->SavedRegisterSize);
+    row_sp->GetCFAValue().SetRaSearch(
+        record->LocalSize + record->SavedRegisterSize, 0);
   } else {
     if (!postfix::ResolveSymbols(it->second, symbol_resolver)) {
       LLDB_LOG(log, "Resolving symbols in `{0}` failed.",
                record->ProgramString);
       return nullptr;
     }
     llvm::ArrayRef<uint8_t> saved  = SaveAsDWARF(*it->second);
     row_sp->GetCFAValue().SetIsDWARFExpression(saved.data(), saved.size());

lldb/source/Symbol/UnwindPlan.cpp

else
     s.Printf("reg(%u)", reg_num);
 }
 
 bool UnwindPlan::Row::FAValue::
 operator==(const UnwindPlan::Row::FAValue &rhs) const {
   if (m_type == rhs.m_type) {
     switch (m_type) {
     case unspecified:
+      return true;
     case isRaSearch:
-      return m_value.ra_search_offset == rhs.m_value.ra_search_offset;
+      return m_value.ra_search.search_offset ==
+                 rhs.m_value.ra_search.search_offset &&
+             m_value.ra_search.cfa_offset == rhs.m_value.ra_search.cfa_offset;
 
     case isRegisterPlusOffset:
       return m_value.reg.offset == rhs.m_value.reg.offset;
 
     case isRegisterDereferenced:
       return m_value.reg.reg_num == rhs.m_value.reg.reg_num;
 
     case isDWARFExpression:
case isDWARFExpression:
     DumpDWARFExpr(s,
                   llvm::makeArrayRef(m_value.expr.opcodes, m_value.expr.length),
                   thread);
     break;
   case unspecified:
     s.PutCString("unspecified");
     break;
   case isRaSearch:
-    s.Printf("RaSearch@SP%+d", m_value.ra_search_offset);
+    s.Printf("RaSearch@SP%+d, offset=%+d", m_value.ra_search.search_offset,
+             GetOffset());
     break;
   }
 }
 
 void UnwindPlan::Row::Clear() {
   m_cfa_value.SetUnspecified();
   m_afa_value.SetUnspecified();
   m_offset = 0;

lldb/source/Target/RegisterContextUnwind.cpp

if (func_unwinders_sp.get() != nullptr)
           process->GetTarget(), m_thread);
 
     if (call_site_unwind_plan.get() != nullptr) {
       m_fallback_unwind_plan_sp = call_site_unwind_plan;
       if (TryFallbackUnwindPlan())
         cfa_status = true;
     }
     if (!cfa_status) {
-      UnwindLogMsg("could not read CFA value for first frame.");
+      UnwindLogMsg("could not read CFA value for first frame. Trying stack "
+                   "walking unwind plan.");
+      if (abi) {
+        UnwindPlanSP stack_walk_unwind_plan =
+            std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
+        if (abi->CreateStackWalkingUnwindPlan(*stack_walk_unwind_plan)) {
+          m_fallback_unwind_plan_sp = stack_walk_unwind_plan;
+          cfa_status = TryFallbackUnwindPlan();
+        }
+      }
+    }
+    if (!cfa_status) {
       m_frame_type = eNotAValidFrame;
       return;
     }
   } else
     ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64
                " afa is 0x%" PRIx64 " using %s UnwindPlan",
void RegisterContextUnwind::InitializeNonZerothFrame() {
 
   const bool allow_section_end = true;
   m_current_pc.SetLoadAddress(pc, &process->GetTarget(), allow_section_end);
 
   // 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.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
       above_trap_handler == false) {
     UnwindLogMsg("using architectural default unwind method");
 
     // Test the pc value to see if we know it's in an unmapped/non-executable
     // region of memory.
void RegisterContextUnwind::InitializeNonZerothFrame() {
 
   if (!active_row.get()) {
     m_frame_type = eNotAValidFrame;
     UnwindLogMsg("could not find unwind row for this pc");
     return;
   }
 
   if (!ReadFrameAddress(row_register_kind, active_row->GetCFAValue(), m_cfa)) {
-    UnwindLogMsg("failed to get cfa");
+    UnwindLogMsg("failed to get cfa. Trying stack walking unwind plan.");
+    bool cfa_status = false;
+    if (abi) {
+      UnwindPlanSP stack_walk_unwind_plan =
+          std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
+      if (abi->CreateStackWalkingUnwindPlan(*stack_walk_unwind_plan)) {
+        m_fallback_unwind_plan_sp = stack_walk_unwind_plan;
+        cfa_status = TryFallbackUnwindPlan();
+      }
+    }
+    if (!cfa_status) {
       m_frame_type = eNotAValidFrame;
       return;
     }
+  }
 
   ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
   UnwindLogMsg("m_cfa = 0x%" PRIx64 " m_afa = 0x%" PRIx64, m_cfa, m_afa);
 
   if (CheckIfLoopingStack()) {
     TryFallbackUnwindPlan();
     if (CheckIfLoopingStack()) {
if (regnum.GetAsKind(unwindplan_registerkind) == LLDB_INVALID_REGNUM) {
       return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
     }
     // The architecture default unwind plan marks unknown registers as
     // Undefined so that we don't forward them up the stack when a
     // jitted stack frame may have overwritten them.  But when the
     // arch default unwind plan is used as the Fast Unwind Plan, we
     // need to recognize this & switch over to the Full Unwind Plan
     // to see what unwind rule that (more knoweldgeable, probably)
-    // UnwindPlan has.  If the full UnwindPlan says the register 
+    // UnwindPlan has.  If the full UnwindPlan says the register
     // location is Undefined, then it really is.
     if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
                                     unwindplan_regloc) &&
         !unwindplan_regloc.IsUndefined()) {
       UnwindLogMsg(
           "supplying caller's saved %s (%d)'s location using FastUnwindPlan",
           regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
       have_unwindplan_regloc = true;
if (m_full_unwind_plan_sp) {
 
       // If we're fetching the saved pc and this UnwindPlan defines a
       // ReturnAddress register (e.g. lr on arm), look for the return address
       // register number in the UnwindPlan's row.
       if (pc_regnum.IsValid() && pc_regnum == regnum &&
           m_full_unwind_plan_sp->GetReturnAddressRegister() !=
               LLDB_INVALID_REGNUM) {
         // If this is a trap handler frame, we should have access to
-        // the complete register context when the interrupt/async 
+        // the complete register context when the interrupt/async
         // signal was received, we should fetch the actual saved $pc
         // value instead of the Return Address register.
         // If $pc is not available, fall back to the RA reg.
         UnwindPlan::Row::RegisterLocation scratch;
         if (m_frame_type == eTrapHandlerFrame &&
-            active_row->GetRegisterInfo 
-              (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+            active_row->GetRegisterInfo(
+                pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
           UnwindLogMsg("Providing pc register instead of rewriting to "
                        "RA reg because this is a trap handler and there is "
                        "a location for the saved pc register value.");
         } else {
           return_address_reg.init(
               m_thread, m_full_unwind_plan_sp->GetRegisterKind(),
               m_full_unwind_plan_sp->GetReturnAddressRegister());
           regnum = return_address_reg;

 
 bool RegisterContextUnwind::TryFallbackUnwindPlan() {
   if (m_fallback_unwind_plan_sp.get() == nullptr)
     return false;
 
   if (m_full_unwind_plan_sp.get() == nullptr)
     return false;
 
-  if (m_full_unwind_plan_sp.get() == m_fallback_unwind_plan_sp.get() ||
-      m_full_unwind_plan_sp->GetSourceName() ==
-          m_fallback_unwind_plan_sp->GetSourceName()) {
-    return false;
-  }
-
   // If a compiler generated unwind plan failed, trying the arch default
   // unwindplan isn't going to do any better.
   if (m_full_unwind_plan_sp->GetSourcedFromCompiler() == eLazyBoolYes)
     return false;
 
   // Get the caller's pc value and our own CFA value. Swap in the fallback
   // unwind plan, re-fetch the caller's pc value and CFA value. If they're the
   // same, then the fallback unwind plan provides no benefit.
bool RegisterContextUnwind::TryFallbackUnwindPlan() {
   addr_t old_afa = m_afa;
 
   m_registers.clear();
 
   m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
 
   UnwindPlan::RowSP active_row =
       m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
-
+  UnwindPlan::Row::FAValue &fa = active_row->GetCFAValue();
   if (active_row &&
-      active_row->GetCFAValue().GetValueType() !=
-          UnwindPlan::Row::FAValue::unspecified) {
+      fa.GetValueType() != UnwindPlan::Row::FAValue::unspecified) {
     addr_t new_cfa;
-    if (!ReadFrameAddress(m_fallback_unwind_plan_sp->GetRegisterKind(),
-                            active_row->GetCFAValue(), new_cfa) ||
-        new_cfa == 0 || new_cfa == 1 || new_cfa == LLDB_INVALID_ADDRESS) {
+    // If itn's not first attempt and ra search, update ra search offset before
+    // searching for cfa. Previous attempts may fail due to bad pc.
+    if (fa.IsRaSearch() && old_cfa != LLDB_INVALID_ADDRESS) {
+      addr_t sp;
+      if (!ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, sp)) {
         UnwindLogMsg("failed to get cfa with fallback unwindplan");
         m_fallback_unwind_plan_sp.reset();
         m_full_unwind_plan_sp = original_full_unwind_plan_sp;
         return false;
       }
+      Process &process = *m_thread.GetProcess();
+      // It's the next stack offset location so that sp + next_search_offset ==
+      // old_cfa + process.GetAddressByteSize().
+      addr_t next_search_offset =
+          old_cfa - sp - fa.GetOffset() + process.GetAddressByteSize();
+      fa.SetRaSearch(next_search_offset, fa.GetOffset());
+    }
+    if (ReadFrameAddress(m_fallback_unwind_plan_sp->GetRegisterKind(), fa,
+                         new_cfa) ||
+        new_cfa == 0 || new_cfa == 1 || new_cfa == LLDB_INVALID_ADDRESS) {
+
       m_cfa = new_cfa;
 
       ReadFrameAddress(m_fallback_unwind_plan_sp->GetRegisterKind(),
                        active_row->GetAFAValue(), m_afa);
 
       if (SavedLocationForRegister(pc_regnum.GetAsKind(eRegisterKindLLDB),
                                    regloc) ==
           UnwindLLDB::RegisterSearchResult::eRegisterFound) {
         const RegisterInfo *reg_info =
             GetRegisterInfoAtIndex(pc_regnum.GetAsKind(eRegisterKindLLDB));
         if (reg_info) {
           RegisterValue reg_value;
           if (ReadRegisterValueFromRegisterLocation(regloc, reg_info,
                                                     reg_value)) {
             new_caller_pc_value = reg_value.GetAsUInt64();
             if (ProcessSP process_sp = m_thread.GetProcess()) {
               if (ABISP abi = process_sp->GetABI())
                 new_caller_pc_value = abi->FixCodeAddress(new_caller_pc_value);
             }
           }
         }
       }
 
       if (new_caller_pc_value == LLDB_INVALID_ADDRESS) {
         UnwindLogMsg("failed to get a pc value for the caller frame with the "
                      "fallback unwind plan");
         m_fallback_unwind_plan_sp.reset();
         m_full_unwind_plan_sp = original_full_unwind_plan_sp;
         m_cfa = old_cfa;
         m_afa = old_afa;
         return false;
       }
 
-    if (old_caller_pc_value == new_caller_pc_value &&
-        m_cfa == old_cfa &&
+      if (old_caller_pc_value == new_caller_pc_value && m_cfa == old_cfa &&
           m_afa == old_afa) {
         UnwindLogMsg("fallback unwind plan got the same values for this frame "
                      "CFA and caller frame pc, not using");
         m_fallback_unwind_plan_sp.reset();
         m_full_unwind_plan_sp = original_full_unwind_plan_sp;
         return false;
       }
 
-    UnwindLogMsg("trying to unwind from this function with the UnwindPlan '%s' "
-                 "because UnwindPlan '%s' failed.",
-                 m_fallback_unwind_plan_sp->GetSourceName().GetCString(),
-                 original_full_unwind_plan_sp->GetSourceName().GetCString());
+      UnwindLogMsg(
+          "trying to unwind from this function with the UnwindPlan '%s' "
 
-    // 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);
+    } else {
+      UnwindLogMsg("failed to get cfa with fallback unwindplan");
+      m_fallback_unwind_plan_sp.reset();
+      m_full_unwind_plan_sp = original_full_unwind_plan_sp;
+      return false;
+    }
   }
 
   return true;
 }
 
 bool RegisterContextUnwind::ForceSwitchToFallbackUnwindPlan() {
   if (m_fallback_unwind_plan_sp.get() == nullptr)
     return false;
if (dwarfexpr.Evaluate(&exe_ctx, this, 0, nullptr, nullptr, result,
       return true;
     }
     UnwindLogMsg("Failed to set CFA value via DWARF expression: %s",
                  error.AsCString());
     break;
   }
   case UnwindPlan::Row::FAValue::isRaSearch: {
     Process &process = *m_thread.GetProcess();
-    lldb::addr_t return_address_hint = GetReturnAddressHint(fa.GetOffset());
-    if (return_address_hint == LLDB_INVALID_ADDRESS)
+    addr_t sp;
+    if (!ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, sp))
       return false;
+    addr_t return_address_hint = sp + fa.GetRaSearchOffset();
+    if (fa.IsFirstRaSearch())
+      return_address_hint += GetParameterStackSize();
     const unsigned max_iterations = 256;
     for (unsigned i = 0; i < max_iterations; ++i) {
       Status st;
-      lldb::addr_t candidate_addr =
+      addr_t candidate_addr =
           return_address_hint + i * process.GetAddressByteSize();
-      lldb::addr_t candidate =
-          process.ReadPointerFromMemory(candidate_addr, st);
+      if (!process.GetABI()->CallFrameAddressIsValid(candidate_addr +
+                                                     fa.GetOffset()))
+        continue;
+      addr_t candidate = process.ReadPointerFromMemory(candidate_addr, st);
       if (st.Fail()) {
         UnwindLogMsg("Cannot read memory at 0x%" PRIx64 ": %s", candidate_addr,
                      st.AsCString());
         return false;
       }
       Address addr;
       uint32_t permissions;
       if (process.GetLoadAddressPermissions(candidate, permissions) &&
           permissions & lldb::ePermissionsExecutable) {
-        address = candidate_addr;
+        address = candidate_addr + fa.GetOffset();
         UnwindLogMsg("Heuristically found CFA: 0x%" PRIx64, address);
         return true;
       }
     }
     UnwindLogMsg("No suitable CFA found");
     break;
   }
   default:
     return false;
   }
   return false;
 }
 
-lldb::addr_t RegisterContextUnwind::GetReturnAddressHint(int32_t plan_offset) {
-  addr_t hint;
-  if (!ReadGPRValue(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, hint))
-    return LLDB_INVALID_ADDRESS;
-  if (!m_sym_ctx.module_sp || !m_sym_ctx.symbol)
-    return LLDB_INVALID_ADDRESS;
-
-  hint += plan_offset;
-
+addr_t RegisterContextUnwind::GetParameterStackSize() {
   if (auto next = GetNextFrame()) {
     if (!next->m_sym_ctx.module_sp || !next->m_sym_ctx.symbol)
-      return LLDB_INVALID_ADDRESS;
+      return 0;
     if (auto expected_size =
             next->m_sym_ctx.module_sp->GetSymbolFile()->GetParameterStackSize(
                 *next->m_sym_ctx.symbol))
-      hint += *expected_size;
+      return *expected_size;
     else {
       UnwindLogMsgVerbose("Could not retrieve parameter size: %s",
                           llvm::toString(expected_size.takeError()).c_str());
-      return LLDB_INVALID_ADDRESS;
+      return 0;
     }
   }
-  return hint;
+  return 0;
 }
 
 // Retrieve a general purpose register value for THIS frame, as saved by the
 // NEXT frame, i.e. the frame that
 // this frame called.  e.g.
 //
 //  foo () { }
 //  bar () { foo (); }

lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test

 # CHECK-LABEL: image show-unwind -n call_many
 # CHECK: UNWIND PLANS for unwind-via-stack-win.exe`call_many
 # CHECK: Symbol file UnwindPlan:
 # CHECK: This UnwindPlan originally sourced from breakpad STACK WIN
 # CHECK: This UnwindPlan is sourced from the compiler: yes.
 # CHECK: This UnwindPlan is valid at all instruction locations: no.
 # CHECK: This UnwindPlan is for a trap handler function: no.
 # CHECK: Address range of this UnwindPlan: [unwind-via-stack-win.exe..module_image + 4112-0x0000107d)
-# CHECK: row[0]:    0: CFA=RaSearch@SP+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
+# CHECK: row[0]:    0: CFA=RaSearch@SP+0, offset=+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
 
 image show-unwind -n nonzero_frame_size
 # CHECK-LABEL: image show-unwind -n nonzero_frame_size
 # CHECK: UNWIND PLANS for unwind-via-stack-win.exe`nonzero_frame_size
 # CHECK: Symbol file UnwindPlan:
-# CHECK: row[0]:    0: CFA=RaSearch@SP+12 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
+# CHECK: row[0]:    0: CFA=RaSearch@SP+12, offset=+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
 
 # Then, some invalid rules.
 image show-unwind -n complex_rasearch
 # CHECK-LABEL: image show-unwind -n complex_rasearch
 # CHECK: UNWIND PLANS for unwind-via-stack-win.exe`complex_rasearch
 # CHECK-NOT: Symbol file
 
 image show-unwind -n esp_rasearch