labath updated this revision to Diff 219071.
labath marked an inline comment as done.
labath edited the summary of this revision.
labath added a comment.

- s/FoundHeuristically/RaSearch
- max_iterations:=256
- tweak the handling of "own frame size" instead of having the unwinder ask for 
it, it is now embedded directly into the unwind plan. This automatically makes 
it possible to have different frame sizes at different points in the function, 
and also reduces the api surface a bit. The "parameter size" is still in a 
separate function, because that belongs to a different frame/function/module.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66638/new/

https://reviews.llvm.org/D66638

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/UnwindPlan.h
  lit/SymbolFile/Breakpad/Inputs/unwind-via-raSearch.syms
  lit/SymbolFile/Breakpad/unwind-via-raSearch.test
  source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  source/Plugins/Process/Utility/RegisterContextLLDB.h
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  source/Symbol/UnwindPlan.cpp

Index: source/Symbol/UnwindPlan.cpp
===================================================================
--- source/Symbol/UnwindPlan.cpp
+++ source/Symbol/UnwindPlan.cpp
@@ -170,7 +170,8 @@
   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;
 
     case isRegisterPlusOffset:
       return m_value.reg.offset == rhs.m_value.reg.offset;
@@ -205,9 +206,12 @@
                   llvm::makeArrayRef(m_value.expr.opcodes, m_value.expr.length),
                   thread);
     break;
-  default:
+  case unspecified:
     s.PutCString("unspecified");
     break;
+  case isRaSearch:
+    s.Printf("RaSearch@SP%+d", m_value.ra_search_offset);
+    break;
   }
 }
 
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
===================================================================
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
@@ -135,6 +135,8 @@
 
   void AddSymbols(Symtab &symtab) override;
 
+  llvm::Expected<lldb::addr_t> GetParameterStackSize(Symbol &symbol) override;
+
   lldb::UnwindPlanSP
   GetUnwindPlan(const Address &address,
                 const RegisterInfoResolver &resolver) override;
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===================================================================
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -374,6 +374,20 @@
   symtab.CalculateSymbolSizes();
 }
 
+llvm::Expected<lldb::addr_t>
+SymbolFileBreakpad::GetParameterStackSize(Symbol &symbol) {
+  ParseUnwindData();
+  if (auto *entry = m_unwind_data->win.FindEntryThatContains(
+          symbol.GetAddress().GetFileAddress())) {
+    auto record = StackWinRecord::parse(
+        *LineIterator(*m_objfile_sp, Record::StackWin, entry->data));
+    assert(record.hasValue());
+    return record->ParameterSize;
+  }
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "Parameter size unknown.");
+}
+
 static llvm::Optional<std::pair<llvm::StringRef, llvm::StringRef>>
 GetRule(llvm::StringRef &unwind_rules) {
   // Unwind rules are of the form
@@ -585,12 +599,19 @@
 
   // 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.
-  if (!postfix::ResolveSymbols(it->second, symbol_resolver)) {
-    LLDB_LOG(log, "Resolving symbols in `{0}` failed.", record->ProgramString);
-    return nullptr;
+  auto *symbol = llvm::dyn_cast<postfix::SymbolNode>(it->second);
+  if (symbol && symbol->GetName() == ".raSearch") {
+    row_sp->GetCFAValue().SetRaSearch(record->LocalSize +
+                                      record->SavedRegisterSize);
+  } 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());
   }
-  llvm::ArrayRef<uint8_t> saved = SaveAsDWARF(*it->second);
-  row_sp->GetCFAValue().SetIsDWARFExpression(saved.data(), saved.size());
 
   // Replace the node value with InitialValueNode, so that subsequent
   // expressions refer to the CFA value instead of recomputing the whole
Index: source/Plugins/Process/Utility/RegisterContextLLDB.h
===================================================================
--- source/Plugins/Process/Utility/RegisterContextLLDB.h
+++ source/Plugins/Process/Utility/RegisterContextLLDB.h
@@ -201,6 +201,8 @@
   bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
                                      int &valid_pc_offset);
 
+  lldb::addr_t GetReturnAddressHint(int32_t plan_offset);
+
   lldb_private::Thread &m_thread;
 
   ///
Index: source/Plugins/Process/Utility/RegisterContextLLDB.cpp
===================================================================
--- source/Plugins/Process/Utility/RegisterContextLLDB.cpp
+++ source/Plugins/Process/Utility/RegisterContextLLDB.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
@@ -1852,12 +1853,65 @@
                  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)
+      return false;
+    const unsigned max_iterations = 256;
+    for (unsigned i = 0; i < max_iterations; ++i) {
+      Status st;
+      lldb::addr_t candidate_addr =
+          return_address_hint + i * process.GetAddressByteSize();
+      lldb::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;
+      if (addr.SetLoadAddress(candidate, &process.GetTarget()) &&
+          addr.GetSection()->GetPermissions() & lldb::ePermissionsExecutable) {
+        address = candidate_addr;
+        UnwindLogMsg("Heuristically found CFA: 0x%" PRIx64, address);
+        return true;
+      }
+    }
+    UnwindLogMsg("No suitable CFA found");
+    break;
+  }
   default:
     return false;
   }
   return false;
 }
 
+lldb::addr_t RegisterContextLLDB::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;
+
+  if (auto next = GetNextFrame()) {
+    if (!next->m_sym_ctx.module_sp || !next->m_sym_ctx.symbol)
+      return LLDB_INVALID_ADDRESS;
+    if (auto expected_size =
+            next->m_sym_ctx.module_sp->GetSymbolFile()->GetParameterStackSize(
+                *next->m_sym_ctx.symbol))
+      hint += *expected_size;
+    else {
+      UnwindLogMsgVerbose("Could not retrieve parameter size: %s",
+                          llvm::toString(expected_size.takeError()).c_str());
+      return LLDB_INVALID_ADDRESS;
+    }
+  }
+  return hint;
+}
+
 // 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.
Index: lit/SymbolFile/Breakpad/unwind-via-raSearch.test
===================================================================
--- /dev/null
+++ lit/SymbolFile/Breakpad/unwind-via-raSearch.test
@@ -0,0 +1,43 @@
+# RUN: yaml2obj %S/Inputs/unwind-via-stack-win.yaml > %t
+# RUN: %lldb -c %t \
+# RUN:   -o "target symbols add %S/Inputs/unwind-via-raSearch.syms" \
+# RUN:   -s %s -b | FileCheck %s
+
+# First check that unwind plan generation works correctly.
+# This function has a "typical" unwind rule.
+image show-unwind -n call_many
+# 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: Address range of this UnwindPlan: [unwind-via-stack-win.exe..module_image + 16-0x0000007d)
+# CHECK: row[0]:    0: CFA=RaSearch@SP+0 => esp=DW_OP_pick 0x00, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x00, 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 0x00, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x00, 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
+# CHECK-LABEL: image show-unwind -n esp_rasearch
+# CHECK: UNWIND PLANS for unwind-via-stack-win.exe`esp_rasearch
+# CHECK-NOT: Symbol file
+
+# And finally, check that backtracing works as a whole by unwinding a simple
+# stack.
+thread backtrace
+# CHECK-LABEL: thread backtrace
+# CHECK: frame #0: 0x000b1092 unwind-via-stack-win.exe`many_pointer_args
+# CHECK: frame #1: 0x000b1079 unwind-via-stack-win.exe`call_many + 105
+# CHECK: frame #2: 0x000b1085 unwind-via-stack-win.exe`main + 5
+# CHECK: frame #3: 0x77278494 kernel32.dll
+# CHECK-NOT: frame
Index: lit/SymbolFile/Breakpad/Inputs/unwind-via-raSearch.syms
===================================================================
--- /dev/null
+++ lit/SymbolFile/Breakpad/Inputs/unwind-via-raSearch.syms
@@ -0,0 +1,15 @@
+MODULE windows x86 897DD83EA8C8411897F3A925EE4BF7411 unwind-via-stack-win.pdb
+INFO CODE_ID 5D499B5C5000 unwind-via-stack-win.exe
+PUBLIC  0 0 dummy
+PUBLIC 10 0 call_many
+PUBLIC 80 0 main
+PUBLIC 90 0 many_pointer_args
+PUBLIC 100 0 complex_rasearch
+PUBLIC 110 0 esp_rasearch
+PUBLIC 120 0 nonzero_frame_size
+STACK WIN 4 10 6d 0 0 0 0 0 0 1 $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + =
+STACK WIN 4 80 8 0 0 0 0 0 0 1 $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + =
+STACK WIN 4 90 5 0 0 50 0 0 0 1 $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + =
+STACK WIN 4 100 4 0 0 0 0 0 0 1 $T0 .raSearch 80 + = $eip $T0 ^ = $esp $T0 4 + =
+STACK WIN 4 110 4 0 0 0 0 0 0 1 $T0 .raSearch = $eip $T0 ^ = $esp .raSearch 4 + =
+STACK WIN 4 120 4 0 0 0 4 8 0 1 $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + =
Index: include/lldb/Symbol/UnwindPlan.h
===================================================================
--- include/lldb/Symbol/UnwindPlan.h
+++ include/lldb/Symbol/UnwindPlan.h
@@ -201,7 +201,8 @@
         unspecified,            // not specified
         isRegisterPlusOffset,   // FA = register + offset
         isRegisterDereferenced, // FA = [reg]
-        isDWARFExpression       // FA = eval(dwarf_expr)
+        isDWARFExpression,      // FA = eval(dwarf_expr)
+        isRaSearch,             // FA = SP + offset + ???
       };
 
       FAValue() : m_type(unspecified), m_value() {}
@@ -214,6 +215,11 @@
 
       bool IsUnspecified() const { return m_type == unspecified; }
 
+      void SetRaSearch(int32_t offset) {
+        m_type = isRaSearch;
+        m_value.ra_search_offset = offset;
+      }
+
       bool IsRegisterPlusOffset() const {
         return m_type == isRegisterPlusOffset;
       }
@@ -250,9 +256,14 @@
       ValueType GetValueType() const { return m_type; }
 
       int32_t GetOffset() const {
-        if (m_type == isRegisterPlusOffset)
-          return m_value.reg.offset;
-        return 0;
+        switch (m_type) {
+          case isRegisterPlusOffset:
+            return m_value.reg.offset;
+          case isRaSearch:
+            return m_value.ra_search_offset;
+          default:
+            return 0;
+        }
       }
 
       void IncOffset(int32_t delta) {
@@ -304,6 +315,8 @@
           const uint8_t *opcodes;
           uint16_t length;
         } expr;
+        // For m_type == isRaSearch
+        int32_t ra_search_offset;
       } m_value;
     }; // class FAValue
 
Index: include/lldb/Symbol/SymbolFile.h
===================================================================
--- include/lldb/Symbol/SymbolFile.h
+++ include/lldb/Symbol/SymbolFile.h
@@ -19,8 +19,8 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/lldb-private.h"
-
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/Errc.h"
 
 #include <mutex>
 
@@ -242,6 +242,13 @@
     return nullptr;
   }
 
+  /// Return the number of stack bytes taken up by the parameters to this
+  /// function.
+  virtual llvm::Expected<lldb::addr_t> GetParameterStackSize(Symbol &symbol) {
+    return llvm::createStringError(make_error_code(llvm::errc::not_supported),
+                                   "Operation not supported.");
+  }
+
   virtual void Dump(Stream &s);
 
 protected:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to