[Lldb-commits] [lldb] a9927f5 - [lldb] Use Expected in BreakpointLocationPredictor (#197730)

via lldb-commits lldb-commits at lists.llvm.org
Fri May 15 00:19:57 PDT 2026


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 &register_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();
 }


        


More information about the lldb-commits mailing list