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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to