[Lldb-commits] [lldb] [lldb] Use Expected in BreakpointLocationPredictor (PR #197730)
via lldb-commits
lldb-commits at lists.llvm.org
Thu May 14 09:00:28 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-risc-v
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
This makes it easier to not have long-lived Status objects. Forgetting to clear those was a problem I ran into in #<!-- -->196891.
---
Full diff: https://github.com/llvm/llvm-project/pull/197730.diff
5 Files Affected:
- (modified) lldb/include/lldb/Core/EmulateInstruction.h (+4-4)
- (modified) lldb/source/Core/EmulateInstruction.cpp (+28-32)
- (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp (+15-20)
- (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h (+3-3)
- (modified) lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp (+12-14)
``````````diff
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..bed8463b05159 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::createStringErrorV("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::createStringErrorV("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::createStringErrorV("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::createStringErrorV("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::createStringErrorV("Instruction emulation failed unexpectedly.");
}
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 4f4e6779072d2..04b19a46a894c 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::createStringErrorV("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::createStringErrorV("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();
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/197730
More information about the lldb-commits
mailing list