[Lldb-commits] [PATCH] D61733: Breakpad: Generate unwind plans from STACK CFI records

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added inline comments.


================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:418
+  if (name == ".ra")
+    return resolver.ResolveNumber(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
+  return ResolveRegister(resolver, name);
----------------
clayborg wrote:
> LLDB_REGNUM_GENERIC_RA? Do we want the PC here or do we want the link register?
It looks a bit weird, but I believe it should be the PC (and I have checked that things unwind correctly this way), because we are specifying value for the PC in the parent frame (which is the same as the return address of the current frame). Or, to put it another way, breakpad uses ".ra" even on platforms which do not have a LLDB_REGNUM_GENERIC_RA register (like x86).


================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:491-492
+  llvm::Optional<StackCFIRecord> init_record = StackCFIRecord::parse(*It);
+  assert(init_record.hasValue());
+  assert(init_record->Size.hasValue());
+
----------------
clayborg wrote:
> will this code be ok if the assertions are compiled out?
Yes, because we make sure that we only store the address of a syntactically correct STACK CFI INIT record in the `m_unwind_data` map (down on line 645)


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

https://reviews.llvm.org/D61733