-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[lldb] Deindent UnwindAssemblyInstEmulation #128874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
by two levels using early returns.
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changesby two levels using early returns. Patch is 22.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128874.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
index 502231002f7f4..3988dcb50f353 100644
--- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -56,251 +56,235 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
if (opcode_data == nullptr || opcode_size == 0)
return false;
- if (range.GetByteSize() > 0 && range.GetBaseAddress().IsValid() &&
- m_inst_emulator_up.get()) {
+ if (range.GetByteSize() == 0 || !range.GetBaseAddress().IsValid() ||
+ !m_inst_emulator_up)
+ return false;
- // The instruction emulation subclass setup the unwind plan for the first
- // instruction.
- m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan);
+ // The instruction emulation subclass setup the unwind plan for the first
+ // instruction.
+ m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan);
- // CreateFunctionEntryUnwind should have created the first row. If it
- // doesn't, then we are done.
- if (unwind_plan.GetRowCount() == 0)
- return false;
+ // CreateFunctionEntryUnwind should have created the first row. If it doesn't,
+ // then we are done.
+ if (unwind_plan.GetRowCount() == 0)
+ return false;
- const bool prefer_file_cache = true;
- DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
- m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(),
- opcode_data, opcode_size, 99999, prefer_file_cache));
+ const bool prefer_file_cache = true;
+ DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
+ m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(),
+ opcode_data, opcode_size, 99999, prefer_file_cache));
- Log *log = GetLog(LLDBLog::Unwind);
+ if (!disasm_sp)
+ return false;
- if (disasm_sp) {
+ Log *log = GetLog(LLDBLog::Unwind);
- m_range_ptr = ⦥
- m_unwind_plan_ptr = &unwind_plan;
+ m_range_ptr = ⦥
+ m_unwind_plan_ptr = &unwind_plan;
+
+ const uint32_t addr_byte_size = m_arch.GetAddressByteSize();
+ const bool show_address = true;
+ const bool show_bytes = true;
+ const bool show_control_flow_kind = false;
+ m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
+ unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister());
+ m_fp_is_cfa = false;
+ m_register_values.clear();
+ m_pushed_regs.clear();
+
+ // Initialize the CFA with a known value. In the 32 bit case it will be
+ // 0x80000000, and in the 64 bit case 0x8000000000000000. We use the address
+ // byte size to be safe for any future address sizes
+ m_initial_sp = (1ull << ((addr_byte_size * 8) - 1));
+ RegisterValue cfa_reg_value;
+ cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size);
+ SetRegisterValue(m_cfa_reg_info, cfa_reg_value);
+
+ const InstructionList &inst_list = disasm_sp->GetInstructionList();
+ const size_t num_instructions = inst_list.GetSize();
+
+ if (num_instructions > 0) {
+ Instruction *inst = inst_list.GetInstructionAtIndex(0).get();
+ const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress();
+
+ // Map for storing the unwind plan row and the value of the registers at a
+ // given offset. When we see a forward branch we add a new entry to this map
+ // with the actual unwind plan row and register context for the target
+ // address of the branch as the current data have to be valid for the target
+ // address of the branch too if we are in the same function.
+ std::map<lldb::addr_t, std::pair<UnwindPlan::RowSP, RegisterValueMap>>
+ saved_unwind_states;
+
+ // Make a copy of the current instruction Row and save it in m_curr_row so
+ // we can add updates as we process the instructions.
+ UnwindPlan::RowSP last_row =
+ std::make_shared<UnwindPlan::Row>(*unwind_plan.GetLastRow());
+ m_curr_row = std::make_shared<UnwindPlan::Row>(*last_row);
+
+ // Add the initial state to the save list with offset 0.
+ saved_unwind_states.insert({0, {last_row, m_register_values}});
+
+ // cache the stack pointer register number (in whatever register numbering
+ // this UnwindPlan uses) for quick reference during instruction parsing.
+ RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo(
+ eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP);
+
+ // 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;
+ m_forward_branch_offset = 0;
+
+ inst = inst_list.GetInstructionAtIndex(idx).get();
+ if (inst) {
+ lldb::addr_t current_offset =
+ inst->GetAddress().GetFileAddress() - base_addr;
+ auto it = saved_unwind_states.upper_bound(current_offset);
+ assert(it != saved_unwind_states.begin() &&
+ "Unwind row for the function entry missing");
+ --it; // Move it to the row corresponding to the current offset
+
+ // If the offset of m_curr_row don't match with the offset we see in
+ // saved_unwind_states then we have to update m_curr_row and
+ // m_register_values based on the saved values. It is happening after we
+ // processed an epilogue and a return to caller instruction.
+ if (it->second.first->GetOffset() != m_curr_row->GetOffset()) {
+ UnwindPlan::Row *newrow = new UnwindPlan::Row;
+ *newrow = *it->second.first;
+ m_curr_row.reset(newrow);
+ m_register_values = it->second.second;
+ // re-set the CFA register ivars to match the new m_curr_row.
+ if (sp_reg_info.name &&
+ m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
+ uint32_t row_cfa_regnum =
+ m_curr_row->GetCFAValue().GetRegisterNumber();
+ lldb::RegisterKind row_kind = m_unwind_plan_ptr->GetRegisterKind();
+ // set m_cfa_reg_info to the row's CFA reg.
+ m_cfa_reg_info =
+ *m_inst_emulator_up->GetRegisterInfo(row_kind, row_cfa_regnum);
+ // set m_fp_is_cfa.
+ if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
+ m_fp_is_cfa = false;
+ else
+ m_fp_is_cfa = true;
+ }
+ }
- const uint32_t addr_byte_size = m_arch.GetAddressByteSize();
- const bool show_address = true;
- const bool show_bytes = true;
- const bool show_control_flow_kind = false;
- m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
- unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister());
- m_fp_is_cfa = false;
- m_register_values.clear();
- m_pushed_regs.clear();
-
- // Initialize the CFA with a known value. In the 32 bit case it will be
- // 0x80000000, and in the 64 bit case 0x8000000000000000. We use the
- // address byte size to be safe for any future address sizes
- m_initial_sp = (1ull << ((addr_byte_size * 8) - 1));
- RegisterValue cfa_reg_value;
- cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size);
- SetRegisterValue(m_cfa_reg_info, cfa_reg_value);
-
- const InstructionList &inst_list = disasm_sp->GetInstructionList();
- const size_t num_instructions = inst_list.GetSize();
-
- if (num_instructions > 0) {
- Instruction *inst = inst_list.GetInstructionAtIndex(0).get();
- const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress();
-
- // Map for storing the unwind plan row and the value of the registers
- // at a given offset. When we see a forward branch we add a new entry
- // to this map with the actual unwind plan row and register context for
- // the target address of the branch as the current data have to be
- // valid for the target address of the branch too if we are in the same
- // function.
- std::map<lldb::addr_t, std::pair<UnwindPlan::RowSP, RegisterValueMap>>
- saved_unwind_states;
-
- // Make a copy of the current instruction Row and save it in m_curr_row
- // so we can add updates as we process the instructions.
- UnwindPlan::RowSP last_row =
- std::make_shared<UnwindPlan::Row>(*unwind_plan.GetLastRow());
- m_curr_row = std::make_shared<UnwindPlan::Row>(*last_row);
-
- // Add the initial state to the save list with offset 0.
- saved_unwind_states.insert({0, {last_row, m_register_values}});
-
- // cache the stack pointer register number (in whatever register
- // numbering this UnwindPlan uses) for quick reference during
- // instruction parsing.
- RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo(
- eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP);
-
- // 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;
- m_forward_branch_offset = 0;
-
- inst = inst_list.GetInstructionAtIndex(idx).get();
- if (inst) {
- lldb::addr_t current_offset =
- inst->GetAddress().GetFileAddress() - base_addr;
- auto it = saved_unwind_states.upper_bound(current_offset);
- assert(it != saved_unwind_states.begin() &&
- "Unwind row for the function entry missing");
- --it; // Move it to the row corresponding to the current offset
-
- // If the offset of m_curr_row don't match with the offset we see
- // in saved_unwind_states then we have to update m_curr_row and
- // m_register_values based on the saved values. It is happening
- // after we processed an epilogue and a return to caller
- // instruction.
- if (it->second.first->GetOffset() != m_curr_row->GetOffset()) {
- UnwindPlan::Row *newrow = new UnwindPlan::Row;
- *newrow = *it->second.first;
- m_curr_row.reset(newrow);
- m_register_values = it->second.second;
- // re-set the CFA register ivars to match the
- // new m_curr_row.
- if (sp_reg_info.name &&
- m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
- uint32_t row_cfa_regnum =
- m_curr_row->GetCFAValue().GetRegisterNumber();
- lldb::RegisterKind row_kind =
- m_unwind_plan_ptr->GetRegisterKind();
- // set m_cfa_reg_info to the row's CFA reg.
- m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
- row_kind, row_cfa_regnum);
- // set m_fp_is_cfa.
- if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
- m_fp_is_cfa = false;
- else
- m_fp_is_cfa = true;
- }
- }
+ m_inst_emulator_up->SetInstruction(inst->GetOpcode(),
+ inst->GetAddress(), nullptr);
+
+ if (last_condition != m_inst_emulator_up->GetInstructionCondition()) {
+ if (m_inst_emulator_up->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}});
+ }
- m_inst_emulator_up->SetInstruction(inst->GetOpcode(),
- inst->GetAddress(), nullptr);
-
- if (last_condition !=
- m_inst_emulator_up->GetInstructionCondition()) {
- if (m_inst_emulator_up->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;
- // re-set the CFA register ivars to match the
- // new m_curr_row.
- if (sp_reg_info.name &&
- m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
- uint32_t row_cfa_regnum =
- m_curr_row->GetCFAValue().GetRegisterNumber();
- lldb::RegisterKind row_kind =
- m_unwind_plan_ptr->GetRegisterKind();
- // set m_cfa_reg_info to the row's CFA reg.
- m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
- row_kind, row_cfa_regnum);
- // set m_fp_is_cfa.
- if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
- m_fp_is_cfa = false;
- else
- m_fp_is_cfa = true;
- }
- 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 actual offset
- condition_block_start_offset = current_offset;
+ // 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;
+ // re-set the CFA register ivars to match the new m_curr_row.
+ if (sp_reg_info.name &&
+ m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
+ uint32_t row_cfa_regnum =
+ m_curr_row->GetCFAValue().GetRegisterNumber();
+ lldb::RegisterKind row_kind =
+ m_unwind_plan_ptr->GetRegisterKind();
+ // set m_cfa_reg_info to the row's CFA reg.
+ m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
+ row_kind, row_cfa_regnum);
+ // set m_fp_is_cfa.
+ if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
+ m_fp_is_cfa = false;
+ else
+ m_fp_is_cfa = true;
}
+ // The last instruction might already created a row for this offset
+ // and we want to overwrite it.
+ bool replace_existing = true;
+ unwind_plan.InsertRow(
+ std::make_shared<UnwindPlan::Row>(*m_curr_row),
+ replace_existing);
+ }
- if (log && log->GetVerbose()) {
- StreamString strm;
- lldb_private::FormatEntity::Entry format;
- FormatEntity::Parse("${frame.pc}: ", format);
- inst->Dump(&strm, inst_list.GetMaxOpcocdeByteSize(), show_address,
- show_bytes, show_control_flow_kind, nullptr, nullptr,
- nullptr, &format, 0);
- log->PutString(strm.GetString());
- }
+ // We are starting a new conditional block at the actual offset
+ condition_block_start_offset = current_offset;
+ }
- last_condition = m_inst_emulator_up->GetInstructionCondition();
-
- m_inst_emulator_up->EvaluateInstruction(
- eEmulateInstructionOptionIgnoreConditions);
-
- // If the current instruction is a branch forward then save the
- // current CFI information for the offset where we are branching.
- if (m_forward_branch_offset != 0 &&
- range.ContainsFileAddress(inst->GetAddress().GetFileAddress() +
- m_forward_branch_offset)) {
- auto newrow =
- std::make_shared<UnwindPlan::Row>(*m_curr_row.get());
- newrow->SetOffset(current_offset + m_forward_branch_offset);
- saved_unwind_states.insert(
- {current_offset + m_forward_branch_offset,
- {newrow, m_register_values}});
- unwind_plan.InsertRow(newrow);
- }
+ if (log && log->GetVerbose()) {
+ StreamString strm;
+ lldb_private::FormatEntity::Entry format;
+ FormatEntity::Parse("${frame.pc}: ", format);
+ inst->Dump(&strm, inst_list.GetMaxOpcocdeByteSize(), show_address,
+ show_bytes, show_control_flow_kind, nullptr, nullptr,
+ nullptr, &format, 0);
+ log->PutString(strm.GetString());
+ }
- // Were there any changes to the CFI while evaluating this
- // instruction?
- if (m_curr_row_modified) {
- // Save the modified row if we don't already have a CFI row in
- // the current address
- if (saved_unwind_states.count(
- current_offset + inst->GetOpcode().GetByteSize()) == 0) {
- m_curr_row->SetOffset(current_offset +
- inst->GetOpcode().GetByteSize());
- unwind_plan.InsertRow(m_curr_row);
- saved_unwind_states.insert(
- {current_offset + inst->GetOpcode().GetByteSize(),
- {m_curr_row, m_register_values}});
-
- // Allocate a new Row for m_curr_row, copy the current state
- // into it
- UnwindPlan::Row *newrow = new UnwindPlan::Row;
- *newrow = *m_curr_row.get();
- m_curr_row.reset(newrow);
- }
- }
+ last_condition = m_inst_emulator_up->GetInstructionCondition();
+
+ m_inst_emulator_up->EvaluateInstruction(
+ eEmulateInstructionOptionIgnoreConditions);
+
+ // If the current instruction is a branch forward then save the current
+ // CFI information for the offset where we are branching.
+ if (m_forward_branch_offset != 0 &&
+ range.ContainsFileAddress(inst->GetAddress().GetFileAddress() +
+ m_forward_branch_offset)) {
+ auto newrow = std::make_...
[truncated]
|
JDevlieghere
approved these changes
Feb 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⬅️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
by three levels using early returns/continues.