Author: tberghammer Date: Wed Feb 10 04:42:13 2016 New Revision: 260368 URL: http://llvm.org/viewvc/llvm-project?rev=260368&view=rev Log: Fix handling of the arm IT instruction in the unwinder
The IT instruction can specify condition code for up to 4 consecutive instruction and it is used quite often by clang in epilogues causing an issue when trying to unwind from locations covered by the IT instruction and for locatins inmediately after the IT instruction. Changes made to fix it: * Introduce the concept of conditional instruction block what is a list of consecutive instructions with the same condition. We update the unwind information during the conditional instruction block and when we reach the end of it (first instruction with a differemt condition) then we restore the unwind information we had before the condition. * Fix a bug in the ARM instruction emulator where neither PC nor the ITSTATE was advanced when we reached an instruction what we can't decode. After the change we have no regression on android-arm running the regular test suit and TestStandardUnwind also passes when running it with clang as the compiler (previously it failed on an IT instruction). Differential revision: http://reviews.llvm.org/D16814 Modified: lldb/trunk/include/lldb/Core/EmulateInstruction.h lldb/trunk/include/lldb/Symbol/UnwindPlan.h lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp lldb/trunk/source/Symbol/UnwindPlan.cpp Modified: lldb/trunk/include/lldb/Core/EmulateInstruction.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/EmulateInstruction.h?rev=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/include/lldb/Core/EmulateInstruction.h (original) +++ lldb/trunk/include/lldb/Core/EmulateInstruction.h Wed Feb 10 04:42:13 2016 @@ -384,6 +384,11 @@ public: const RegisterInfo *reg_info, const RegisterValue ®_value); + // Type to represent the condition of an instruction. The UINT32 value is reserved for the + // unconditional case and all other value can be used in an architecture dependent way. + typedef uint32_t InstructionCondition; + static const InstructionCondition UnconditionalCondition = UINT32_MAX; + EmulateInstruction (const ArchSpec &arch); ~EmulateInstruction() override = default; @@ -403,8 +408,8 @@ public: virtual bool EvaluateInstruction (uint32_t evaluate_options) = 0; - virtual bool - IsInstructionConditional() { return false; } + virtual InstructionCondition + GetInstructionCondition() { return UnconditionalCondition; } virtual bool TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data) = 0; Modified: lldb/trunk/include/lldb/Symbol/UnwindPlan.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/UnwindPlan.h?rev=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/include/lldb/Symbol/UnwindPlan.h (original) +++ lldb/trunk/include/lldb/Symbol/UnwindPlan.h Wed Feb 10 04:42:13 2016 @@ -539,7 +539,7 @@ public: AppendRow (const RowSP& row_sp); void - InsertRow (const RowSP& row_sp); + InsertRow (const RowSP& row_sp, bool replace_existing = false); // Returns a pointer to the best row for the given offset into the function's instructions. // If offset is -1 it indicates that the function start is unknown - the final row in the UnwindPlan is returned. 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=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp (original) +++ lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp Wed Feb 10 04:42:13 2016 @@ -13587,10 +13587,7 @@ EmulateInstructionARM::EvaluateInstructi opcode_data = GetThumbOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa); else if (m_opcode_mode == eModeARM) opcode_data = GetARMOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa); - - if (opcode_data == NULL) - return false; - + const bool auto_advance_pc = evaluate_options & eEmulateInstructionOptionAutoAdvancePC; m_ignore_conditions = evaluate_options & eEmulateInstructionOptionIgnoreConditions; @@ -13614,16 +13611,19 @@ EmulateInstructionARM::EvaluateInstructi if (!success) return false; } - - // Call the Emulate... function. - success = (this->*opcode_data->callback) (m_opcode.GetOpcode32(), opcode_data->encoding); - if (!success) - return false; + + // Call the Emulate... function if we managed to decode the opcode. + if (opcode_data) + { + success = (this->*opcode_data->callback) (m_opcode.GetOpcode32(), opcode_data->encoding); + if (!success) + return false; + } // Advance the ITSTATE bits to their values for the next instruction if we haven't just executed // an IT instruction what initialized it. if (m_opcode_mode == eModeThumb && m_it_session.InITBlock() && - opcode_data->callback != &EmulateInstructionARM::EmulateIT) + (opcode_data == nullptr || opcode_data->callback != &EmulateInstructionARM::EmulateIT)) m_it_session.ITAdvance(); if (auto_advance_pc) @@ -13631,30 +13631,28 @@ EmulateInstructionARM::EvaluateInstructi uint32_t after_pc_value = ReadRegisterUnsigned (eRegisterKindDWARF, dwarf_pc, 0, &success); if (!success) return false; - + if (auto_advance_pc && (after_pc_value == orig_pc_value)) { - if (opcode_data->size == eSize32) - after_pc_value += 4; - else if (opcode_data->size == eSize16) - after_pc_value += 2; - + after_pc_value += m_opcode.GetByteSize(); + EmulateInstruction::Context context; context.type = eContextAdvancePC; context.SetNoArgs(); if (!WriteRegisterUnsigned (context, eRegisterKindDWARF, dwarf_pc, after_pc_value)) return false; - } } return true; } -bool -EmulateInstructionARM::IsInstructionConditional() +EmulateInstruction::InstructionCondition +EmulateInstructionARM::GetInstructionCondition() { const uint32_t cond = CurrentCond (m_opcode.GetOpcode32()); - return cond != 0xe && cond != 0xf && cond != UINT32_MAX; + if (cond == 0xe || cond == 0xf || cond == UINT32_MAX) + return EmulateInstruction::UnconditionalCondition; + return cond; } bool 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=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h (original) +++ lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.h Wed Feb 10 04:42:13 2016 @@ -166,8 +166,8 @@ public: bool EvaluateInstruction (uint32_t evaluate_options) override; - bool - IsInstructionConditional() override; + InstructionCondition + GetInstructionCondition() override; bool TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data) override; 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=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp (original) +++ lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp Wed Feb 10 04:42:13 2016 @@ -125,6 +125,10 @@ UnwindAssemblyInstEmulation::GetNonCallS RegisterInfo ra_reg_info; m_inst_emulator_ap->GetRegisterInfo (eRegisterKindGeneric, LLDB_REGNUM_GENERIC_RA, ra_reg_info); + // The architecture dependent condition code of the last processed instruction. + EmulateInstruction::InstructionCondition last_condition = EmulateInstruction::UnconditionalCondition; + lldb::addr_t condition_block_start_offset = 0; + for (size_t idx=0; idx<num_instructions; ++idx) { m_curr_row_modified = false; @@ -146,7 +150,41 @@ UnwindAssemblyInstEmulation::GetNonCallS UnwindPlan::Row *newrow = new UnwindPlan::Row; *newrow = *it->second.first; m_curr_row.reset(newrow); - m_register_values = it->second.second;; + m_register_values = it->second.second; + } + + m_inst_emulator_ap->SetInstruction (inst->GetOpcode(), + inst->GetAddress(), + exe_ctx.GetTargetPtr()); + + if (last_condition != m_inst_emulator_ap->GetInstructionCondition()) + { + if (m_inst_emulator_ap->GetInstructionCondition() != EmulateInstruction::UnconditionalCondition && + saved_unwind_states.count(current_offset) == 0) + { + // If we don't have a saved row for the current offset then save our + // current state because we will have to restore it after the + // conditional block. + auto new_row = std::make_shared<UnwindPlan::Row>(*m_curr_row.get()); + saved_unwind_states.insert({current_offset, {new_row, m_register_values}}); + } + + // If the last instruction was conditional with a different condition + // then the then current condition then restore the condition. + if (last_condition != EmulateInstruction::UnconditionalCondition) + { + const auto& saved_state = saved_unwind_states.at(condition_block_start_offset); + m_curr_row = std::make_shared<UnwindPlan::Row>(*saved_state.first); + m_curr_row->SetOffset(current_offset); + m_register_values = saved_state.second; + bool replace_existing = true; // The last instruction might already + // created a row for this offset and + // we want to overwrite it. + unwind_plan.InsertRow(std::make_shared<UnwindPlan::Row>(*m_curr_row), replace_existing); + } + + // We are starting a new conditional block at the catual offset + condition_block_start_offset = current_offset; } if (log && log->GetVerbose ()) @@ -158,9 +196,7 @@ UnwindAssemblyInstEmulation::GetNonCallS log->PutCString (strm.GetData()); } - m_inst_emulator_ap->SetInstruction (inst->GetOpcode(), - inst->GetAddress(), - exe_ctx.GetTargetPtr()); + last_condition = m_inst_emulator_ap->GetInstructionCondition(); m_inst_emulator_ap->EvaluateInstruction (eEmulateInstructionOptionIgnoreConditions); @@ -503,8 +539,7 @@ UnwindAssemblyInstEmulation::WriteRegist log->PutCString(strm.GetData()); } - if (!instruction->IsInstructionConditional()) - SetRegisterValue (*reg_info, reg_value); + SetRegisterValue (*reg_info, reg_value); switch (context.type) { @@ -566,45 +601,42 @@ UnwindAssemblyInstEmulation::WriteRegist case EmulateInstruction::eContextPopRegisterOffStack: { - if (!instruction->IsInstructionConditional()) + 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) { - 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) + switch (context.info_type) { - switch (context.info_type) - { - case EmulateInstruction::eInfoTypeAddress: - if (m_pushed_regs.find(reg_num) != m_pushed_regs.end() && - context.info.address == m_pushed_regs[reg_num]) - { - m_curr_row->SetRegisterLocationToSame(reg_num, - false /*must_replace*/); - m_curr_row_modified = true; - } - break; - case EmulateInstruction::eInfoTypeISA: - assert((generic_regnum == LLDB_REGNUM_GENERIC_PC || - generic_regnum == LLDB_REGNUM_GENERIC_FLAGS) && - "eInfoTypeISA used for poping a register other the the PC/FLAGS"); - if (generic_regnum != LLDB_REGNUM_GENERIC_FLAGS) - { - m_curr_row->SetRegisterLocationToSame(reg_num, - false /*must_replace*/); - m_curr_row_modified = true; - } - break; - default: - assert(false && "unhandled case, add code to handle this!"); - break; - } + case EmulateInstruction::eInfoTypeAddress: + if (m_pushed_regs.find(reg_num) != m_pushed_regs.end() && + context.info.address == m_pushed_regs[reg_num]) + { + m_curr_row->SetRegisterLocationToSame(reg_num, + false /*must_replace*/); + m_curr_row_modified = true; + } + break; + case EmulateInstruction::eInfoTypeISA: + assert((generic_regnum == LLDB_REGNUM_GENERIC_PC || + generic_regnum == LLDB_REGNUM_GENERIC_FLAGS) && + "eInfoTypeISA used for poping a register other the the PC/FLAGS"); + if (generic_regnum != LLDB_REGNUM_GENERIC_FLAGS) + { + m_curr_row->SetRegisterLocationToSame(reg_num, + false /*must_replace*/); + m_curr_row_modified = true; + } + break; + default: + assert(false && "unhandled case, add code to handle this!"); + break; } } } break; case EmulateInstruction::eContextSetFramePointer: - if (!m_fp_is_cfa && !instruction->IsInstructionConditional()) + if (!m_fp_is_cfa) { m_fp_is_cfa = true; m_cfa_reg_info = *reg_info; @@ -619,7 +651,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 && !instruction->IsInstructionConditional()) + if (!m_fp_is_cfa) { m_curr_row->GetCFAValue().SetIsRegisterPlusOffset( m_curr_row->GetCFAValue().GetRegisterNumber(), Modified: lldb/trunk/source/Symbol/UnwindPlan.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/UnwindPlan.cpp?rev=260368&r1=260367&r2=260368&view=diff ============================================================================== --- lldb/trunk/source/Symbol/UnwindPlan.cpp (original) +++ lldb/trunk/source/Symbol/UnwindPlan.cpp Wed Feb 10 04:42:13 2016 @@ -338,7 +338,7 @@ UnwindPlan::AppendRow (const UnwindPlan: } void -UnwindPlan::InsertRow (const UnwindPlan::RowSP &row_sp) +UnwindPlan::InsertRow (const UnwindPlan::RowSP &row_sp, bool replace_existing) { collection::iterator it = m_row_list.begin(); while (it != m_row_list.end()) { @@ -349,6 +349,8 @@ UnwindPlan::InsertRow (const UnwindPlan: } if (it == m_row_list.end() || (*it)->GetOffset() != row_sp->GetOffset()) m_row_list.insert(it, row_sp); + else if (replace_existing) + *it = row_sp; } UnwindPlan::RowSP _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
