[Lldb-commits] [lldb] r242519 - Improve conditional opcode handling in emulation based unwinding

Tamas Berghammer tberghammer at google.com
Fri Jul 17 04:44:14 PDT 2015


Author: tberghammer
Date: Fri Jul 17 06:44:14 2015
New Revision: 242519

URL: http://llvm.org/viewvc/llvm-project?rev=242519&view=rev
Log:
Improve conditional opcode handling in emulation based unwinding

Don't chane the CFI information when a conditional instruction
is emulated (eg.: popeq {r0, pc}) because the CFI for the next
instruction should be the same as the CFI for the current instruction.

Differential revision: http://reviews.llvm.org/D11258

Modified:
    lldb/trunk/include/lldb/Core/EmulateInstruction.h
    lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
    lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
    lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
    lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
    lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
    lldb/trunk/test/functionalities/unwind/standard/TestStandardUnwind.py

Modified: lldb/trunk/include/lldb/Core/EmulateInstruction.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/EmulateInstruction.h?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/EmulateInstruction.h (original)
+++ lldb/trunk/include/lldb/Core/EmulateInstruction.h Fri Jul 17 06:44:14 2015
@@ -404,7 +404,10 @@ public:
 
     virtual bool
     EvaluateInstruction (uint32_t evaluate_options) = 0;
-    
+
+    virtual bool
+    IsInstructionConditional() { return false; }
+
     virtual bool
     TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data) = 0;
 

Modified: lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp (original)
+++ lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp Fri Jul 17 06:44:14 2015
@@ -387,9 +387,8 @@ EmulateInstructionARM::EmulatePUSH (cons
     }
 #endif
 
-    bool conditional = false;
     bool success = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         const uint32_t addr_byte_size = GetAddressByteSize();
         const addr_t sp = ReadCoreReg (SP_REG, &success);
@@ -442,10 +441,7 @@ EmulateInstructionARM::EmulatePUSH (cons
         uint32_t i;
         
         EmulateInstruction::Context context;
-        if (conditional)
-            context.type = EmulateInstruction::eContextRegisterStore;
-        else
-            context.type = EmulateInstruction::eContextPushRegisterOnStack;
+        context.type = EmulateInstruction::eContextPushRegisterOnStack;
         RegisterInfo reg_info;
         RegisterInfo sp_reg;
         GetRegisterInfo (eRegisterKindDWARF, dwarf_sp, sp_reg);
@@ -511,8 +507,7 @@ EmulateInstructionARM::EmulatePOP (const
 
     bool success = false;
 
-    bool conditional = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         const uint32_t addr_byte_size = GetAddressByteSize();
         const addr_t sp = ReadCoreReg (SP_REG, &success);
@@ -574,10 +569,7 @@ EmulateInstructionARM::EmulatePOP (const
         uint32_t i, data;
         
         EmulateInstruction::Context context;
-        if (conditional)
-            context.type = EmulateInstruction::eContextRegisterLoad;
-        else
-            context.type = EmulateInstruction::eContextPopRegisterOffStack;
+        context.type = EmulateInstruction::eContextPopRegisterOffStack;
         
         RegisterInfo sp_reg;
         GetRegisterInfo (eRegisterKindDWARF, dwarf_sp, sp_reg);
@@ -1971,9 +1963,8 @@ EmulateInstructionARM::EmulateSTRRtSP (c
     }
 #endif
 
-    bool conditional = false;
     bool success = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         const uint32_t addr_byte_size = GetAddressByteSize();
         const addr_t sp = ReadCoreReg (SP_REG, &success);
@@ -2018,10 +2009,7 @@ EmulateInstructionARM::EmulateSTRRtSP (c
             addr = sp;
         
         EmulateInstruction::Context context;
-        if (conditional)
-            context.type = EmulateInstruction::eContextRegisterStore;
-        else
-            context.type = EmulateInstruction::eContextPushRegisterOnStack;
+        context.type = EmulateInstruction::eContextPushRegisterOnStack;
         RegisterInfo sp_reg;
         RegisterInfo dwarf_reg;
 
@@ -2082,8 +2070,7 @@ EmulateInstructionARM::EmulateVPUSH (con
 #endif
 
     bool success = false;
-    bool conditional = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         const uint32_t addr_byte_size = GetAddressByteSize();
         const addr_t sp = ReadCoreReg (SP_REG, &success);
@@ -2125,10 +2112,8 @@ EmulateInstructionARM::EmulateVPUSH (con
         uint32_t i;
         
         EmulateInstruction::Context context;
-        if (conditional)
-            context.type = EmulateInstruction::eContextRegisterStore;
-        else
-            context.type = EmulateInstruction::eContextPushRegisterOnStack;
+        context.type = EmulateInstruction::eContextPushRegisterOnStack;
+
         RegisterInfo dwarf_reg;
         RegisterInfo sp_reg;
         GetRegisterInfo (eRegisterKindDWARF, dwarf_sp, sp_reg);
@@ -2178,8 +2163,7 @@ EmulateInstructionARM::EmulateVPOP (cons
 #endif
 
     bool success = false;
-    bool conditional = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         const uint32_t addr_byte_size = GetAddressByteSize();
         const addr_t sp = ReadCoreReg (SP_REG, &success);
@@ -2222,10 +2206,8 @@ EmulateInstructionARM::EmulateVPOP (cons
         uint64_t data; // uint64_t to accommodate 64-bit registers.
         
         EmulateInstruction::Context context;
-        if (conditional)
-            context.type = EmulateInstruction::eContextRegisterLoad;
-        else
-            context.type = EmulateInstruction::eContextPopRegisterOffStack;
+        context.type = EmulateInstruction::eContextPopRegisterOffStack;
+
         RegisterInfo dwarf_reg;
         RegisterInfo sp_reg;
         GetRegisterInfo (eRegisterKindDWARF, dwarf_sp, sp_reg);
@@ -3462,8 +3444,7 @@ EmulateInstructionARM::EmulateLDM (const
 #endif
             
     bool success = false;
-    bool conditional = false;
-    if (ConditionPassed(opcode, &conditional))
+    if (ConditionPassed(opcode))
     {
         uint32_t n;
         uint32_t registers = 0;
@@ -3535,12 +3516,7 @@ EmulateInstructionARM::EmulateLDM (const
                 context.type = EmulateInstruction::eContextRegisterPlusOffset;
                 context.SetRegisterPlusOffset (dwarf_reg, offset);
                 if (wback && (n == 13)) // Pop Instruction
-                {
-                    if (conditional)
-                        context.type = EmulateInstruction::eContextRegisterLoad;
-                    else
-                        context.type = EmulateInstruction::eContextPopRegisterOffStack;
-                }
+                    context.type = EmulateInstruction::eContextPopRegisterOffStack;
 
                 // R[i] = MemA [address, 4]; address = address + 4;
                 uint32_t data = MemARead (context, base_address + offset, addr_byte_size, 0, &success);
@@ -13076,7 +13052,7 @@ EmulateInstructionARM::ArchVersion ()
 }
 
 bool
-EmulateInstructionARM::ConditionPassed (const uint32_t opcode, bool *is_conditional)
+EmulateInstructionARM::ConditionPassed (const uint32_t opcode)
 {
    // If we are ignoring conditions, then always return true.
    // this allows us to iterate over disassembly code and still
@@ -13084,12 +13060,8 @@ EmulateInstructionARM::ConditionPassed (
    // bits set in the CPSR register...
     if (m_ignore_conditions)
         return true;
-    
-    if (is_conditional)
-        *is_conditional = true;
 
     const uint32_t cond = CurrentCond (opcode);
-    
     if (cond == UINT32_MAX)
         return false;
     
@@ -13149,8 +13121,6 @@ EmulateInstructionARM::ConditionPassed (
     case 7: 
         // Always execute (cond == 0b1110, or the special 0b1111 which gives
         // opcodes different meanings, but always means execution happens.
-        if (is_conditional)
-            *is_conditional = false;
         return true;
     }
 
@@ -13643,6 +13613,13 @@ EmulateInstructionARM::EvaluateInstructi
 }
 
 bool
+EmulateInstructionARM::IsInstructionConditional()
+{
+    const uint32_t cond = CurrentCond (m_opcode.GetOpcode32());
+    return cond != 0xe && cond != 0xf && cond != UINT32_MAX;
+}
+
+bool
 EmulateInstructionARM::TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data)
 {
     if (!test_data)

Modified: lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h (original)
+++ lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h Fri Jul 17 06:44:14 2015
@@ -165,7 +165,10 @@ public:
 
     virtual bool
     EvaluateInstruction (uint32_t evaluate_options);
-    
+
+    bool
+    IsInstructionConditional() override;
+
     virtual bool
     TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data);
 
@@ -180,9 +183,7 @@ public:
     ArchVersion();
 
     bool
-    ConditionPassed (const uint32_t opcode, 
-                     bool *is_conditional = NULL);  // Filled in with true if the opcode is a conditional opcode
-                                                    // Filled in with false if the opcode is always executed
+    ConditionPassed (const uint32_t opcode);
 
     uint32_t
     CurrentCond (const uint32_t opcode);

Modified: lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp (original)
+++ lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp Fri Jul 17 06:44:14 2015
@@ -462,7 +462,7 @@ EmulateInstructionARM64::BranchTo (const
 }
 
 bool
-EmulateInstructionARM64::ConditionHolds (const uint32_t cond, bool *is_conditional)
+EmulateInstructionARM64::ConditionHolds (const uint32_t cond)
 {
    // If we are ignoring conditions, then always return true.
    // this allows us to iterate over disassembly code and still
@@ -470,10 +470,7 @@ EmulateInstructionARM64::ConditionHolds
    // bits set in the CPSR register...
     if (m_ignore_conditions)
         return true;
-    
-    if (is_conditional)
-        *is_conditional = true;
-  
+
     bool result = false;
     switch (UnsignedBits(cond, 3, 1))
     {
@@ -499,13 +496,12 @@ EmulateInstructionARM64::ConditionHolds
         result = (m_opcode_pstate.N == m_opcode_pstate.V && m_opcode_pstate.Z == 0);
         break;
     case 7:
-        result = true;
-        if (is_conditional)
-            *is_conditional = false;
-        break;
+        // Always execute (cond == 0b1110, or the special 0b1111 which gives
+        // opcodes different meanings, but always means execution happens.
+        return true;
     }
 
-    if (cond & 1 && cond != 15)
+    if (cond & 1)
         result = !result;
     return result;
 }

Modified: lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h (original)
+++ lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h Fri Jul 17 06:44:14 2015
@@ -272,7 +272,7 @@ protected:
     BranchTo (const Context &context, uint32_t N, lldb::addr_t target);
 
     bool
-    ConditionHolds (const uint32_t cond, bool *is_conditional = nullptr);
+    ConditionHolds (const uint32_t cond);
 
     bool
     UsingAArch32 ();

Modified: lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp (original)
+++ lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp Fri Jul 17 06:44:14 2015
@@ -511,7 +511,8 @@ UnwindAssemblyInstEmulation::WriteRegist
         log->PutCString(strm.GetData());
     }
 
-    SetRegisterValue (*reg_info, reg_value);
+    if (!instruction->IsInstructionConditional())
+        SetRegisterValue (*reg_info, reg_value);
 
     switch (context.type)
     {
@@ -573,18 +574,21 @@ UnwindAssemblyInstEmulation::WriteRegist
 
         case EmulateInstruction::eContextPopRegisterOffStack:
             {
-                const uint32_t reg_num = reg_info->kinds[m_unwind_plan_ptr->GetRegisterKind()];
-                const uint32_t generic_regnum = reg_info->kinds[eRegisterKindGeneric];
-                if (reg_num != LLDB_INVALID_REGNUM && generic_regnum != LLDB_REGNUM_GENERIC_SP)
+                if (!instruction->IsInstructionConditional())
                 {
-                    m_curr_row->SetRegisterLocationToSame (reg_num, /*must_replace*/ false);
-                    m_curr_row_modified = true;
+                    const uint32_t reg_num = reg_info->kinds[m_unwind_plan_ptr->GetRegisterKind()];
+                    const uint32_t generic_regnum = reg_info->kinds[eRegisterKindGeneric];
+                    if (reg_num != LLDB_INVALID_REGNUM && generic_regnum != LLDB_REGNUM_GENERIC_SP)
+                    {
+                        m_curr_row->SetRegisterLocationToSame (reg_num, /*must_replace*/ false);
+                        m_curr_row_modified = true;
+                    }
                 }
             }
             break;
 
         case EmulateInstruction::eContextSetFramePointer:
-            if (!m_fp_is_cfa)
+            if (!m_fp_is_cfa && !instruction->IsInstructionConditional())
             {
                 m_fp_is_cfa = true;
                 m_cfa_reg_info = *reg_info;
@@ -599,7 +603,7 @@ UnwindAssemblyInstEmulation::WriteRegist
         case EmulateInstruction::eContextAdjustStackPointer:
             // If we have created a frame using the frame pointer, don't follow
             // subsequent adjustments to the stack pointer.
-            if (!m_fp_is_cfa)
+            if (!m_fp_is_cfa && !instruction->IsInstructionConditional())
             {
                 m_curr_row->GetCFAValue().SetIsRegisterPlusOffset(
                         m_curr_row->GetCFAValue().GetRegisterNumber(),

Modified: lldb/trunk/test/functionalities/unwind/standard/TestStandardUnwind.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/unwind/standard/TestStandardUnwind.py?rev=242519&r1=242518&r2=242519&view=diff
==============================================================================
--- lldb/trunk/test/functionalities/unwind/standard/TestStandardUnwind.py (original)
+++ lldb/trunk/test/functionalities/unwind/standard/TestStandardUnwind.py Fri Jul 17 06:44:14 2015
@@ -39,7 +39,6 @@ class StandardUnwindTest(TestBase):
                 "_start",                # Base function on the stack
                 "__memcpy_base",         # Function reached by a fall through from the previous function
                 "__memcpy_base_aligned", # Function reached by a fall through from the previous function
-                "__subdf3",              # __aeabi_ui2d jumps into the middle of the function. Possibly missing symbol?
             ]
             no_step_function_names = [
                 "__sync_fetch_and_add_4", # Calls into a special SO where we can't set a breakpoint





More information about the lldb-commits mailing list