Skip to content

Commit 10e9923

Browse files
author
Tamas Berghammer
committedFeb 10, 2016
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 llvm-svn: 260368
1 parent 1b6dacb commit 10e9923

File tree

6 files changed

+101
-64
lines changed

6 files changed

+101
-64
lines changed
 

Diff for: ‎lldb/include/lldb/Core/EmulateInstruction.h

+7-2
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ class EmulateInstruction :
384384
const RegisterInfo *reg_info,
385385
const RegisterValue &reg_value);
386386

387+
// Type to represent the condition of an instruction. The UINT32 value is reserved for the
388+
// unconditional case and all other value can be used in an architecture dependent way.
389+
typedef uint32_t InstructionCondition;
390+
static const InstructionCondition UnconditionalCondition = UINT32_MAX;
391+
387392
EmulateInstruction (const ArchSpec &arch);
388393

389394
~EmulateInstruction() override = default;
@@ -403,8 +408,8 @@ class EmulateInstruction :
403408
virtual bool
404409
EvaluateInstruction (uint32_t evaluate_options) = 0;
405410

406-
virtual bool
407-
IsInstructionConditional() { return false; }
411+
virtual InstructionCondition
412+
GetInstructionCondition() { return UnconditionalCondition; }
408413

409414
virtual bool
410415
TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data) = 0;

Diff for: ‎lldb/include/lldb/Symbol/UnwindPlan.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ class UnwindPlan {
539539
AppendRow (const RowSP& row_sp);
540540

541541
void
542-
InsertRow (const RowSP& row_sp);
542+
InsertRow (const RowSP& row_sp, bool replace_existing = false);
543543

544544
// Returns a pointer to the best row for the given offset into the function's instructions.
545545
// If offset is -1 it indicates that the function start is unknown - the final row in the UnwindPlan is returned.

Diff for: ‎lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp

+18-20
Original file line numberDiff line numberDiff line change
@@ -13587,10 +13587,7 @@ EmulateInstructionARM::EvaluateInstruction (uint32_t evaluate_options)
1358713587
opcode_data = GetThumbOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa);
1358813588
else if (m_opcode_mode == eModeARM)
1358913589
opcode_data = GetARMOpcodeForInstruction (m_opcode.GetOpcode32(), m_arm_isa);
13590-
13591-
if (opcode_data == NULL)
13592-
return false;
13593-
13590+
1359413591
const bool auto_advance_pc = evaluate_options & eEmulateInstructionOptionAutoAdvancePC;
1359513592
m_ignore_conditions = evaluate_options & eEmulateInstructionOptionIgnoreConditions;
1359613593

@@ -13614,47 +13611,48 @@ EmulateInstructionARM::EvaluateInstruction (uint32_t evaluate_options)
1361413611
if (!success)
1361513612
return false;
1361613613
}
13617-
13618-
// Call the Emulate... function.
13619-
success = (this->*opcode_data->callback) (m_opcode.GetOpcode32(), opcode_data->encoding);
13620-
if (!success)
13621-
return false;
13614+
13615+
// Call the Emulate... function if we managed to decode the opcode.
13616+
if (opcode_data)
13617+
{
13618+
success = (this->*opcode_data->callback) (m_opcode.GetOpcode32(), opcode_data->encoding);
13619+
if (!success)
13620+
return false;
13621+
}
1362213622

1362313623
// Advance the ITSTATE bits to their values for the next instruction if we haven't just executed
1362413624
// an IT instruction what initialized it.
1362513625
if (m_opcode_mode == eModeThumb && m_it_session.InITBlock() &&
13626-
opcode_data->callback != &EmulateInstructionARM::EmulateIT)
13626+
(opcode_data == nullptr || opcode_data->callback != &EmulateInstructionARM::EmulateIT))
1362713627
m_it_session.ITAdvance();
1362813628

1362913629
if (auto_advance_pc)
1363013630
{
1363113631
uint32_t after_pc_value = ReadRegisterUnsigned (eRegisterKindDWARF, dwarf_pc, 0, &success);
1363213632
if (!success)
1363313633
return false;
13634-
13634+
1363513635
if (auto_advance_pc && (after_pc_value == orig_pc_value))
1363613636
{
13637-
if (opcode_data->size == eSize32)
13638-
after_pc_value += 4;
13639-
else if (opcode_data->size == eSize16)
13640-
after_pc_value += 2;
13641-
13637+
after_pc_value += m_opcode.GetByteSize();
13638+
1364213639
EmulateInstruction::Context context;
1364313640
context.type = eContextAdvancePC;
1364413641
context.SetNoArgs();
1364513642
if (!WriteRegisterUnsigned (context, eRegisterKindDWARF, dwarf_pc, after_pc_value))
1364613643
return false;
13647-
1364813644
}
1364913645
}
1365013646
return true;
1365113647
}
1365213648

13653-
bool
13654-
EmulateInstructionARM::IsInstructionConditional()
13649+
EmulateInstruction::InstructionCondition
13650+
EmulateInstructionARM::GetInstructionCondition()
1365513651
{
1365613652
const uint32_t cond = CurrentCond (m_opcode.GetOpcode32());
13657-
return cond != 0xe && cond != 0xf && cond != UINT32_MAX;
13653+
if (cond == 0xe || cond == 0xf || cond == UINT32_MAX)
13654+
return EmulateInstruction::UnconditionalCondition;
13655+
return cond;
1365813656
}
1365913657

1366013658
bool

Diff for: ‎lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ class EmulateInstructionARM : public EmulateInstruction
166166
bool
167167
EvaluateInstruction (uint32_t evaluate_options) override;
168168

169-
bool
170-
IsInstructionConditional() override;
169+
InstructionCondition
170+
GetInstructionCondition() override;
171171

172172
bool
173173
TestEmulation (Stream *out_stream, ArchSpec &arch, OptionValueDictionary *test_data) override;

Diff for: ‎lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp

+70-38
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly (AddressRange&
125125
RegisterInfo ra_reg_info;
126126
m_inst_emulator_ap->GetRegisterInfo (eRegisterKindGeneric, LLDB_REGNUM_GENERIC_RA, ra_reg_info);
127127

128+
// The architecture dependent condition code of the last processed instruction.
129+
EmulateInstruction::InstructionCondition last_condition = EmulateInstruction::UnconditionalCondition;
130+
lldb::addr_t condition_block_start_offset = 0;
131+
128132
for (size_t idx=0; idx<num_instructions; ++idx)
129133
{
130134
m_curr_row_modified = false;
@@ -146,7 +150,41 @@ UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly (AddressRange&
146150
UnwindPlan::Row *newrow = new UnwindPlan::Row;
147151
*newrow = *it->second.first;
148152
m_curr_row.reset(newrow);
149-
m_register_values = it->second.second;;
153+
m_register_values = it->second.second;
154+
}
155+
156+
m_inst_emulator_ap->SetInstruction (inst->GetOpcode(),
157+
inst->GetAddress(),
158+
exe_ctx.GetTargetPtr());
159+
160+
if (last_condition != m_inst_emulator_ap->GetInstructionCondition())
161+
{
162+
if (m_inst_emulator_ap->GetInstructionCondition() != EmulateInstruction::UnconditionalCondition &&
163+
saved_unwind_states.count(current_offset) == 0)
164+
{
165+
// If we don't have a saved row for the current offset then save our
166+
// current state because we will have to restore it after the
167+
// conditional block.
168+
auto new_row = std::make_shared<UnwindPlan::Row>(*m_curr_row.get());
169+
saved_unwind_states.insert({current_offset, {new_row, m_register_values}});
170+
}
171+
172+
// If the last instruction was conditional with a different condition
173+
// then the then current condition then restore the condition.
174+
if (last_condition != EmulateInstruction::UnconditionalCondition)
175+
{
176+
const auto& saved_state = saved_unwind_states.at(condition_block_start_offset);
177+
m_curr_row = std::make_shared<UnwindPlan::Row>(*saved_state.first);
178+
m_curr_row->SetOffset(current_offset);
179+
m_register_values = saved_state.second;
180+
bool replace_existing = true; // The last instruction might already
181+
// created a row for this offset and
182+
// we want to overwrite it.
183+
unwind_plan.InsertRow(std::make_shared<UnwindPlan::Row>(*m_curr_row), replace_existing);
184+
}
185+
186+
// We are starting a new conditional block at the catual offset
187+
condition_block_start_offset = current_offset;
150188
}
151189

152190
if (log && log->GetVerbose ())
@@ -158,9 +196,7 @@ UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly (AddressRange&
158196
log->PutCString (strm.GetData());
159197
}
160198

161-
m_inst_emulator_ap->SetInstruction (inst->GetOpcode(),
162-
inst->GetAddress(),
163-
exe_ctx.GetTargetPtr());
199+
last_condition = m_inst_emulator_ap->GetInstructionCondition();
164200

165201
m_inst_emulator_ap->EvaluateInstruction (eEmulateInstructionOptionIgnoreConditions);
166202

@@ -503,8 +539,7 @@ UnwindAssemblyInstEmulation::WriteRegister (EmulateInstruction *instruction,
503539
log->PutCString(strm.GetData());
504540
}
505541

506-
if (!instruction->IsInstructionConditional())
507-
SetRegisterValue (*reg_info, reg_value);
542+
SetRegisterValue (*reg_info, reg_value);
508543

509544
switch (context.type)
510545
{
@@ -566,45 +601,42 @@ UnwindAssemblyInstEmulation::WriteRegister (EmulateInstruction *instruction,
566601

567602
case EmulateInstruction::eContextPopRegisterOffStack:
568603
{
569-
if (!instruction->IsInstructionConditional())
604+
const uint32_t reg_num = reg_info->kinds[m_unwind_plan_ptr->GetRegisterKind()];
605+
const uint32_t generic_regnum = reg_info->kinds[eRegisterKindGeneric];
606+
if (reg_num != LLDB_INVALID_REGNUM && generic_regnum != LLDB_REGNUM_GENERIC_SP)
570607
{
571-
const uint32_t reg_num = reg_info->kinds[m_unwind_plan_ptr->GetRegisterKind()];
572-
const uint32_t generic_regnum = reg_info->kinds[eRegisterKindGeneric];
573-
if (reg_num != LLDB_INVALID_REGNUM && generic_regnum != LLDB_REGNUM_GENERIC_SP)
608+
switch (context.info_type)
574609
{
575-
switch (context.info_type)
576-
{
577-
case EmulateInstruction::eInfoTypeAddress:
578-
if (m_pushed_regs.find(reg_num) != m_pushed_regs.end() &&
579-
context.info.address == m_pushed_regs[reg_num])
580-
{
581-
m_curr_row->SetRegisterLocationToSame(reg_num,
582-
false /*must_replace*/);
583-
m_curr_row_modified = true;
584-
}
585-
break;
586-
case EmulateInstruction::eInfoTypeISA:
587-
assert((generic_regnum == LLDB_REGNUM_GENERIC_PC ||
588-
generic_regnum == LLDB_REGNUM_GENERIC_FLAGS) &&
589-
"eInfoTypeISA used for poping a register other the the PC/FLAGS");
590-
if (generic_regnum != LLDB_REGNUM_GENERIC_FLAGS)
591-
{
592-
m_curr_row->SetRegisterLocationToSame(reg_num,
593-
false /*must_replace*/);
594-
m_curr_row_modified = true;
595-
}
596-
break;
597-
default:
598-
assert(false && "unhandled case, add code to handle this!");
599-
break;
600-
}
610+
case EmulateInstruction::eInfoTypeAddress:
611+
if (m_pushed_regs.find(reg_num) != m_pushed_regs.end() &&
612+
context.info.address == m_pushed_regs[reg_num])
613+
{
614+
m_curr_row->SetRegisterLocationToSame(reg_num,
615+
false /*must_replace*/);
616+
m_curr_row_modified = true;
617+
}
618+
break;
619+
case EmulateInstruction::eInfoTypeISA:
620+
assert((generic_regnum == LLDB_REGNUM_GENERIC_PC ||
621+
generic_regnum == LLDB_REGNUM_GENERIC_FLAGS) &&
622+
"eInfoTypeISA used for poping a register other the the PC/FLAGS");
623+
if (generic_regnum != LLDB_REGNUM_GENERIC_FLAGS)
624+
{
625+
m_curr_row->SetRegisterLocationToSame(reg_num,
626+
false /*must_replace*/);
627+
m_curr_row_modified = true;
628+
}
629+
break;
630+
default:
631+
assert(false && "unhandled case, add code to handle this!");
632+
break;
601633
}
602634
}
603635
}
604636
break;
605637

606638
case EmulateInstruction::eContextSetFramePointer:
607-
if (!m_fp_is_cfa && !instruction->IsInstructionConditional())
639+
if (!m_fp_is_cfa)
608640
{
609641
m_fp_is_cfa = true;
610642
m_cfa_reg_info = *reg_info;
@@ -619,7 +651,7 @@ UnwindAssemblyInstEmulation::WriteRegister (EmulateInstruction *instruction,
619651
case EmulateInstruction::eContextAdjustStackPointer:
620652
// If we have created a frame using the frame pointer, don't follow
621653
// subsequent adjustments to the stack pointer.
622-
if (!m_fp_is_cfa && !instruction->IsInstructionConditional())
654+
if (!m_fp_is_cfa)
623655
{
624656
m_curr_row->GetCFAValue().SetIsRegisterPlusOffset(
625657
m_curr_row->GetCFAValue().GetRegisterNumber(),

Diff for: ‎lldb/source/Symbol/UnwindPlan.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ UnwindPlan::AppendRow (const UnwindPlan::RowSP &row_sp)
338338
}
339339

340340
void
341-
UnwindPlan::InsertRow (const UnwindPlan::RowSP &row_sp)
341+
UnwindPlan::InsertRow (const UnwindPlan::RowSP &row_sp, bool replace_existing)
342342
{
343343
collection::iterator it = m_row_list.begin();
344344
while (it != m_row_list.end()) {
@@ -349,6 +349,8 @@ UnwindPlan::InsertRow (const UnwindPlan::RowSP &row_sp)
349349
}
350350
if (it == m_row_list.end() || (*it)->GetOffset() != row_sp->GetOffset())
351351
m_row_list.insert(it, row_sp);
352+
else if (replace_existing)
353+
*it = row_sp;
352354
}
353355

354356
UnwindPlan::RowSP

0 commit comments

Comments
 (0)
Please sign in to comment.