[Lldb-commits] [lldb] r232619 - Initial Assembly profiler for mips64

[Jason Molenda]

Hi, a small followup to this patch.  I noticed that our arm UnwindProfiles are having problems recently and tracked it down to the change in UnwindAssemblyInstEmulation.  The patch adds code to UnwindAssemblyInstEmulation which recognizes EmulateInstruction::eContextRegisterLoad -- a register saved into another register.

The problem is when we save the caller's register value on the stack and then *reuse* that register for something else.  For instance, an armv7 code sequence like

    0x286b38 <+0>:   push   {r4, r5, r6, r7, lr}
    0x286b3a <+2>:   add    r7, sp, #0xc
[...]
    0x286b52 <+26>:  blx    0xa334bc                  ; symbol stub for: objc_msgSend
    0x286b56 <+30>:  mov    r7, r7

r7 is being used for a frame pointer in this code.  We save the caller's frame pointer on the stack in the first instruction.  Then at <+30>, we do a register-to-register mov of r7.  The instruction emulation should recognize the initial save but then any changes to r7 should be ignored for the rest of the function.

(I don't know what 'mov r7,r7' accomplishes in arm - that looks like a no-op to me but maybe it has some behavior that I don't recognize; I'm not an arm expert)

I *can* work around this with a patch like (I omitted the indentation of the block of code so the patch is easier to read).  When we've got a "THIS register was saved into ..." instruction, we only update the unwind rule for THIS if it hasn't already been saved.

Index: source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===================================================================
--- source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp	(revision 235572)
+++ source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp	(working copy)
@@ -639,6 +639,10 @@
                 const uint32_t reg_num = reg_info->kinds[unwind_reg_kind];
                 if (reg_num != LLDB_INVALID_REGNUM)
                 {
+                    UnwindPlan::Row::RegisterLocation current_reg_unwind;
+                    if (m_curr_row->GetRegisterInfo (reg_num, current_reg_unwind) == false
+                        || current_reg_unwind.IsSame() == true)
+                    {
                     m_curr_row->SetRegisterLocationToRegister (reg_num, reg_num, must_replace);
                     m_curr_row_modified = true;
                     m_curr_insn_restored_a_register = true;
@@ -658,6 +662,7 @@
                             }
                         }
                     }
+                    }
                 }
             }
             break;


Can you try this on your mips architecture and see if it causes problems?

fwiw I have to do something like this with the x86 instruction unwinder too.  Once a register is saved, I ignore any "saves" of that register for the rest of the function.

J

> On Mar 18, 2015, at 2:21 AM, Bhushan D. Attarde <Bhushan.Attarde at imgtec.com> wrote:
> 
> Author: bhushan.attarde
> Date: Wed Mar 18 04:21:29 2015
> New Revision: 232619
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=232619&view=rev
> Log:
> Initial Assembly profiler for mips64
> 
> Summary:
> This is initial implementation of assembly profiler which only scans prologue/epilogue assembly instructions to create CFI instructions.
> 
> Reviewers: clayborg, jasonmolenda
> 
> Differential Revision: http://reviews.llvm.org/D7696