llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-risc-v Author: Jason Molenda (jasonmolenda) <details> <summary>Changes</summary> I noticed a few of the ABI plugins were not setting UnspecifiedRegistersAreUndefined()==true in their UnwindPlan rows. This prevents the unwind engine from trying to fetch registers from any newer stack frames. The arch default unwind plans are used by lldb when it doesn't have any accurate information about the registers a function has spilled to stack, or where -- it only knows enough to continue walking the stack, assuming all the functions have stored the caller's pc & fp to stack. It is the last-ditch fallback when we backtrace through no-symbol / no-unwind info code. Normally when the unwind engine does not see a non-volatile aka callee-preserved register mentioned in an UnwindPlan, that means this function did not save the register to stack, it left the reg contents unmodified. So if we are looking for x12 in frame 3, and frame 2 doesn't mention x12 in its UnwindPlan, we will look in frame 1 to see if it spilled x12 to stack. If not, then we look at frame 0 and if it also hasn't spilled it, we copy the x12 value from the live register context and use it in frame 3. Arch default unwind plans are dangerous if they allow this to happen because we don't know if a stack frame saved a register to stack and then changed the value in the register. It's unsafe for us to make any assumptions about any register not explicitly listed in the UnwindPlan. I added debug-build asserts for all uses of an ArchDefaultUnwindPlan to enforce this and catch any future introductions of the mistake. Very easy to do unintentionally in new ABIs. --- Full diff: https://github.com/llvm/llvm-project/pull/203684.diff 7 Files Affected: - (modified) lldb/include/lldb/Symbol/UnwindPlan.h (+1-1) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+7) - (modified) lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp (+1) - (modified) lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp (+1) - (modified) lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp (+1) - (modified) lldb/source/Symbol/FuncUnwinders.cpp (+10-1) - (modified) lldb/source/Target/RegisterContextUnwind.cpp (+15) ``````````diff diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h index fe8081f83c590..99d948f8b8a8c 100644 --- a/lldb/include/lldb/Symbol/UnwindPlan.h +++ b/lldb/include/lldb/Symbol/UnwindPlan.h @@ -409,7 +409,7 @@ class UnwindPlan { m_unspecified_registers_are_undefined = unspec_is_undef; } - bool GetUnspecifiedRegistersAreUndefined() { + bool GetUnspecifiedRegistersAreUndefined() const { return m_unspecified_registers_are_undefined; } diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index 56e0dfef34444..49b2c43c52222 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -3786,6 +3786,13 @@ class CommandObjectTargetModulesShowUnwind : public CommandObjectParsed { ABISP abi_sp = process->GetABI(); if (abi_sp) { if (UnwindPlanSP plan_sp = abi_sp->CreateDefaultUnwindPlan()) { +#ifndef NDEBUG + if (plan_sp && plan_sp->GetRowCount() > 0) + assert(plan_sp->GetRowAtIndex(0) + ->GetUnspecifiedRegistersAreUndefined() && + "Default UnwindPlan must set " + "UnspecifiedRegistersAreUndefined to true"); +#endif result.GetOutputStream().Printf("Arch default UnwindPlan:\n"); plan_sp->Dump(result.GetOutputStream(), thread.get(), LLDB_INVALID_ADDRESS); diff --git a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp index d2a2cbe0643ac..0c4495be9a7ba 100644 --- a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp +++ b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp @@ -566,6 +566,7 @@ UnwindPlanSP ABISysV_loongarch::CreateDefaultUnwindPlan() { // have been spilled to stack already. row.SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, reg_size * -2, true); row.SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, reg_size * -1, true); + row.SetUnspecifiedRegistersAreUndefined(true); auto plan_sp = std::make_shared<UnwindPlan>(eRegisterKindGeneric); plan_sp->AppendRow(std::move(row)); diff --git a/lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp b/lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp index af7ef03b94c4c..da2c01b4a2ad4 100644 --- a/lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp +++ b/lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp @@ -331,6 +331,7 @@ UnwindPlanSP ABISysV_msp430::CreateDefaultUnwindPlan() { row.SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, -2, true); row.SetRegisterLocationToIsCFAPlusOffset(sp_reg_num, 0, true); row.SetRegisterLocationToUnspecified(fp_reg_num, true); + row.SetUnspecifiedRegistersAreUndefined(true); auto plan_sp = std::make_shared<UnwindPlan>(eRegisterKindDWARF); plan_sp->AppendRow(std::move(row)); diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp index bf0e5a15ad790..f056b7958c063 100644 --- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp +++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp @@ -753,6 +753,7 @@ UnwindPlanSP ABISysV_riscv::CreateDefaultUnwindPlan() { // have been spilled to stack already. row.SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, reg_size * -2, true); row.SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, reg_size * -1, true); + row.SetUnspecifiedRegistersAreUndefined(true); auto plan_sp = std::make_shared<UnwindPlan>(eRegisterKindGeneric); plan_sp->AppendRow(std::move(row)); diff --git a/lldb/source/Symbol/FuncUnwinders.cpp b/lldb/source/Symbol/FuncUnwinders.cpp index fffc35d7f6177..c0052fff280c6 100644 --- a/lldb/source/Symbol/FuncUnwinders.cpp +++ b/lldb/source/Symbol/FuncUnwinders.cpp @@ -459,8 +459,17 @@ FuncUnwinders::GetUnwindPlanArchitectureDefault(Thread &thread) { ProcessSP process_sp(thread.CalculateProcess()); if (process_sp) { - if (ABI *abi = process_sp->GetABI().get()) + if (ABI *abi = process_sp->GetABI().get()) { m_unwind_plan_arch_default_sp = abi->CreateDefaultUnwindPlan(); +#ifndef NDEBUG + if (m_unwind_plan_arch_default_sp && + m_unwind_plan_arch_default_sp->GetRowCount() > 0) + assert(m_unwind_plan_arch_default_sp->GetRowAtIndex(0) + ->GetUnspecifiedRegistersAreUndefined() && + "Default UnwindPlan must set UnspecifiedRegistersAreUndefined " + "to true"); +#endif + } } return m_unwind_plan_arch_default_sp; diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index 139387db7a04f..fe934cb6c9e97 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -442,6 +442,13 @@ void RegisterContextUnwind::InitializeNonZerothFrame() { if (abi_sp) { m_fast_unwind_plan_sp.reset(); m_full_unwind_plan_sp = abi_sp->CreateDefaultUnwindPlan(); +#ifndef NDEBUG + if (m_full_unwind_plan_sp && m_full_unwind_plan_sp->GetRowCount() > 0) + assert(m_full_unwind_plan_sp->GetRowAtIndex(0) + ->GetUnspecifiedRegistersAreUndefined() && + "Default UnwindPlan must set UnspecifiedRegistersAreUndefined " + "to true"); +#endif if (m_frame_type != eSkipFrame) // don't override eSkipFrame { m_frame_type = eNormalFrame; @@ -810,6 +817,14 @@ RegisterContextUnwind::GetFullUnwindPlanForFrame() { ABI *abi = process ? process->GetABI().get() : nullptr; if (abi) { arch_default_unwind_plan_sp = abi->CreateDefaultUnwindPlan(); +#ifndef NDEBUG + if (arch_default_unwind_plan_sp && + arch_default_unwind_plan_sp->GetRowCount() > 0) + assert(arch_default_unwind_plan_sp->GetRowAtIndex(0) + ->GetUnspecifiedRegistersAreUndefined() && + "Default UnwindPlan must set UnspecifiedRegistersAreUndefined to " + "true"); +#endif } else { UNWIND_LOG( log, "unable to get architectural default UnwindPlan from ABI plugin"); `````````` </details> https://github.com/llvm/llvm-project/pull/203684 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
