Author: Pavel Labath Date: 2026-05-15T07:19:51Z New Revision: a9927f537528a65464a8b80302026524aee13b5d
URL: https://github.com/llvm/llvm-project/commit/a9927f537528a65464a8b80302026524aee13b5d DIFF: https://github.com/llvm/llvm-project/commit/a9927f537528a65464a8b80302026524aee13b5d.diff LOG: [lldb] Use Expected in BreakpointLocationPredictor (#197730) This makes it easier to not have long-lived Status objects. Forgetting to clear those was a problem I ran into in #196891. Added: Modified: lldb/include/lldb/Core/EmulateInstruction.h lldb/source/Core/EmulateInstruction.cpp lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/EmulateInstruction.h b/lldb/include/lldb/Core/EmulateInstruction.h index 88cdfc646cd7c..6f2d272895dce 100644 --- a/lldb/include/lldb/Core/EmulateInstruction.h +++ b/lldb/include/lldb/Core/EmulateInstruction.h @@ -44,7 +44,7 @@ class SingleStepBreakpointLocationsPredictor { std::unique_ptr<EmulateInstruction> emulator_up) : m_emulator_up{std::move(emulator_up)} {} - virtual BreakpointLocations GetBreakpointLocations(Status &status); + virtual llvm::Expected<BreakpointLocations> GetBreakpointLocations(); virtual llvm::Expected<unsigned> GetBreakpointSize([[maybe_unused]] lldb::addr_t bp_addr) { @@ -58,10 +58,10 @@ class SingleStepBreakpointLocationsPredictor { // in the binary file. Essentially, it reads the value of the PC register, // determines the size of the current instruction (where the PC is pointing), // and returns the sum of these two values. - lldb::addr_t GetNextInstructionAddress(Status &error); + llvm::Expected<lldb::addr_t> GetNextInstructionAddress(); - lldb::addr_t GetBreakpointLocationAddress(lldb::addr_t entry_pc, - Status &error); + llvm::Expected<lldb::addr_t> + GetBreakpointLocationAddress(lldb::addr_t entry_pc); std::unique_ptr<EmulateInstruction> m_emulator_up; bool m_emulation_result = false; diff --git a/lldb/source/Core/EmulateInstruction.cpp b/lldb/source/Core/EmulateInstruction.cpp index 9eb8dc0af3447..6d774c64babbe 100644 --- a/lldb/source/Core/EmulateInstruction.cpp +++ b/lldb/source/Core/EmulateInstruction.cpp @@ -26,6 +26,7 @@ #include "lldb/lldb-private-interfaces.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/ErrorExtras.h" #include <cstring> #include <memory> @@ -613,53 +614,49 @@ bool EmulateInstruction::CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) { return false; } -BreakpointLocations -SingleStepBreakpointLocationsPredictor::GetBreakpointLocations(Status &status) { +llvm::Expected<BreakpointLocations> +SingleStepBreakpointLocationsPredictor::GetBreakpointLocations() { if (!m_emulator_up->ReadInstruction()) { // try to get at least the size of next instruction to set breakpoint. - lldb::addr_t next_pc = GetNextInstructionAddress(status); - return BreakpointLocations{next_pc}; + llvm::Expected<addr_t> next_pc = GetNextInstructionAddress(); + if (next_pc) + return BreakpointLocations{*next_pc}; + return next_pc.takeError(); } - auto entry_pc = m_emulator_up->ReadPC(); - if (!entry_pc) { - status = Status("Can't read PC"); - return {}; - } + std::optional<addr_t> entry_pc = m_emulator_up->ReadPC(); + if (!entry_pc) + return llvm::createStringError("Can't read PC"); m_emulation_result = m_emulator_up->EvaluateInstruction( eEmulateInstructionOptionAutoAdvancePC); - lldb::addr_t next_pc = GetBreakpointLocationAddress(*entry_pc, status); - return BreakpointLocations{next_pc}; + llvm::Expected<addr_t> next_pc = GetBreakpointLocationAddress(*entry_pc); + if (next_pc) + return BreakpointLocations{*next_pc}; + return next_pc.takeError(); } -lldb::addr_t SingleStepBreakpointLocationsPredictor::GetNextInstructionAddress( - Status &error) { - auto instr_size = m_emulator_up->GetLastInstrSize(); - if (!instr_size) { - error = Status("Read instruction failed!"); - return LLDB_INVALID_ADDRESS; - } +llvm::Expected<addr_t> +SingleStepBreakpointLocationsPredictor::GetNextInstructionAddress() { + std::optional<uint32_t> instr_size = m_emulator_up->GetLastInstrSize(); + if (!instr_size) + return llvm::createStringError("Read instruction failed!"); - auto pc = m_emulator_up->ReadPC(); - if (!pc) { - error = Status("Can't read PC"); - return LLDB_INVALID_ADDRESS; - } + std::optional<addr_t> pc = m_emulator_up->ReadPC(); + if (!pc) + return llvm::createStringError("Can't read PC"); lldb::addr_t next_pc = *pc + *instr_size; return next_pc; } -lldb::addr_t +llvm::Expected<addr_t> SingleStepBreakpointLocationsPredictor::GetBreakpointLocationAddress( - lldb::addr_t entry_pc, Status &error) { - auto addr = m_emulator_up->ReadPC(); - if (!addr) { - error = Status("Can't read PC"); - return LLDB_INVALID_ADDRESS; - } + lldb::addr_t entry_pc) { + std::optional<addr_t> addr = m_emulator_up->ReadPC(); + if (!addr) + return llvm::createStringError("Can't read PC"); lldb::addr_t pc = *addr; if (m_emulation_result) { @@ -678,6 +675,5 @@ SingleStepBreakpointLocationsPredictor::GetBreakpointLocationAddress( // The instruction emulation failed after it modified the PC. It is an // unknown error where we can't continue because the next instruction is // modifying the PC but we don't know how. - error = Status("Instruction emulation failed unexpectedly."); - return LLDB_INVALID_ADDRESS; + return llvm::createStringError("Instruction emulation failed unexpectedly."); } diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp index 4f4e6779072d2..1abedc050af63 100644 --- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp +++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp @@ -24,6 +24,7 @@ #include "lldb/Utility/Stream.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/Support/ErrorExtras.h" #include "llvm/Support/MathExtras.h" #include <optional> @@ -1951,27 +1952,23 @@ bool EmulateInstructionRISCV::SupportsThisArch(const ArchSpec &arch) { return arch.GetTriple().isRISCV(); } -BreakpointLocations -RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointLocations( - Status &status) { +llvm::Expected<BreakpointLocations> +RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointLocations() { EmulateInstructionRISCV *riscv_emulator = static_cast<EmulateInstructionRISCV *>(m_emulator_up.get()); - auto pc = riscv_emulator->ReadPC(); - if (!pc) { - status = Status("Can't read PC"); - return {}; - } + std::optional<addr_t> pc = riscv_emulator->ReadPC(); + if (!pc) + return llvm::createStringError("Can't read PC"); auto inst = riscv_emulator->ReadInstructionAt(*pc); if (!inst) { // Can't read instruction. Try default handler. - return SingleStepBreakpointLocationsPredictor::GetBreakpointLocations( - status); + return SingleStepBreakpointLocationsPredictor::GetBreakpointLocations(); } if (FoundLoadReserve(inst->decoded)) - return HandleAtomicSequence(*pc, status); + return HandleAtomicSequence(*pc); if (FoundStoreConditional(inst->decoded)) { // Ill-formed atomic sequence (SC doesn't have corresponding LR @@ -1982,10 +1979,10 @@ RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointLocations( "RISCVSingleStepBreakpointLocationsPredictor::%s: can't find " "corresponding load reserve instruction", __FUNCTION__); - return {*pc + (inst->is_rvc ? 2u : 4u)}; + return BreakpointLocations{*pc + (inst->is_rvc ? 2u : 4u)}; } - return SingleStepBreakpointLocationsPredictor::GetBreakpointLocations(status); + return SingleStepBreakpointLocationsPredictor::GetBreakpointLocations(); } llvm::Expected<unsigned> @@ -2006,9 +2003,9 @@ RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointSize( return 4; } -BreakpointLocations +llvm::Expected<BreakpointLocations> RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence( - lldb::addr_t pc, Status &error) { + lldb::addr_t pc) { EmulateInstructionRISCV *riscv_emulator = static_cast<EmulateInstructionRISCV *>(m_emulator_up.get()); @@ -2024,10 +2021,8 @@ RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence( std::vector<lldb::addr_t> bp_addrs; do { inst = riscv_emulator->ReadInstructionAt(pc); - if (!inst) { - error = Status("Can't read instruction"); - return {}; - } + if (!inst) + return llvm::createStringError("Can't read instruction"); if (B *branch = std::get_if<B>(&inst->decoded)) bp_addrs.push_back(pc + SignExt(branch->imm)); @@ -2047,7 +2042,7 @@ RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence( "RISCVSingleStepBreakpointLocationsPredictor::%s: can't find " "corresponding store conditional instruction", __FUNCTION__); - return {entry_pc + (lr_inst->is_rvc ? 2u : 4u)}; + return BreakpointLocations{entry_pc + (lr_inst->is_rvc ? 2u : 4u)}; } lldb::addr_t exit_pc = pc; diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h index c196a9bb9ce82..2c2cf8cc5ad6c 100644 --- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h +++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h @@ -15,7 +15,7 @@ #include "lldb/Interpreter/OptionValue.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/RegisterValue.h" -#include "lldb/Utility/Status.h" +#include "llvm/Support/Error.h" #include <optional> namespace lldb_private { @@ -27,7 +27,7 @@ class RISCVSingleStepBreakpointLocationsPredictor std::unique_ptr<EmulateInstruction> emulator) : SingleStepBreakpointLocationsPredictor{std::move(emulator)} {} - BreakpointLocations GetBreakpointLocations(Status &status) override; + llvm::Expected<BreakpointLocations> GetBreakpointLocations() override; llvm::Expected<unsigned> GetBreakpointSize(lldb::addr_t bp_addr) override; @@ -42,7 +42,7 @@ class RISCVSingleStepBreakpointLocationsPredictor std::holds_alternative<SC_D>(inst); } - BreakpointLocations HandleAtomicSequence(lldb::addr_t pc, Status &error); + llvm::Expected<BreakpointLocations> HandleAtomicSequence(lldb::addr_t pc); static constexpr size_t s_max_atomic_sequence_length = 64; }; diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp index e962b5ae939b0..e5aaef67b5e70 100644 --- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp +++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp @@ -89,7 +89,6 @@ static size_t WriteMemoryCallback(EmulateInstruction *instruction, void *baton, Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping( NativeThreadProtocol &thread) { - Status error; NativeProcessProtocol &process = thread.GetProcess(); NativeRegisterContext ®ister_context = thread.GetRegisterContext(); const ArchSpec &arch = process.GetArchitecture(); @@ -111,26 +110,25 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping( EmulateInstruction::CreateBreakpointLocationPredictor( std::move(emulator_up)); - BreakpointLocations candidates = - bp_locations_predictor->GetBreakpointLocations(error); - if (error.Fail()) - return error; + llvm::Expected<BreakpointLocations> candidates = + bp_locations_predictor->GetBreakpointLocations(); + if (!candidates) + return Status::FromError(candidates.takeError()); - for (addr_t bp_addr : candidates) { + for (addr_t bp_addr : *candidates) { if (process.HasSoftwareBreakpoint(bp_addr)) continue; - auto bp_size = bp_locations_predictor->GetBreakpointSize(bp_addr); + llvm::Expected<unsigned> bp_size = + bp_locations_predictor->GetBreakpointSize(bp_addr); if (auto err = bp_size.takeError()) - return Status(toString(std::move(err))); + return Status::FromError(std::move(err)); - error = process.SetBreakpoint(bp_addr, *bp_size, /*hardware=*/false); + Status error = process.SetBreakpoint(bp_addr, *bp_size, /*hardware=*/false); // If setting the breakpoint fails because pc is out of the address // space, ignore it and let the debugee segfault. - if (error.GetError() == EIO || error.GetError() == EFAULT) { - error.Clear(); + if (error.GetError() == EIO || error.GetError() == EFAULT) continue; - } if (error.Fail()) return error; @@ -138,6 +136,6 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping( } m_threads_stepping_with_breakpoint.emplace(thread.GetID(), - std::move(candidates)); - return error; + std::move(*candidates)); + return Status(); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
