[Lldb-commits] [PATCH] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.


================
Comment at: source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:875-876
+             row->GetCFAValue().GetRegisterNumber() == m_lldb_fp_regnum) {
+      current_sp_bytes_offset_from_cfa =
+          row->GetCFAValue().GetOffset() - stack_offset;
+    }
----------------
labath wrote:
> tberghammer wrote:
> > Shouldn't you change the unwind information for the CFA here? For me saying CFA=rbp seems like an incorrect thing to do, but not sure what would be the correct value (Undefined? IsSame?). The impact is if an other register (or a local variable) have a location specified as CFA+off then after this instruction it will point to bogus location.
> I think there has been some misunderstanding, as the your comment makes no sense to me. :)
> 
> This code only fires if CFA=rbp+offset, and that remains valid even after this instruction -- `lea` does not change the value of the rbp register, so any register rule that was valid before this instruction will remain valid after it. This only begins to make a difference after we process the `pop %rbp` instruction -- then we will update the CFA rule to read `CFA=rsp+current_sp_bytes_offset_from_cfa`.
You are right, please ignore my comment. I somehow assumed the `lea` instruction will change the value of `rbp` as well not just `rsp`.


https://reviews.llvm.org/D34750