Fixes issue where the 'next' command in the CLI fails.

PiperOrigin-RevId: 657634577
Change-Id: Id3c42432c4b6c92ff157fabaa7896a74f78514f9

cheriot/cheriot_top.cc

diff --git a/cheriot/cheriot_top.cc b/cheriot/cheriot_top.cc
index 2078de5..c357f0e 100644
--- a/cheriot/cheriot_top.cc
+++ b/cheriot/cheriot_top.cc
@@ -889,7 +889,21 @@
 }
 
 absl::StatusOr<Instruction *> CheriotTop::GetInstruction(uint64_t address) {
-  auto inst = cheriot_decode_cache_->GetDecodedInstruction(address);
+  // If requesting the instruction at an action point, we need to write the
+  // original instruction back to memory before getting the disassembly.
+  bool inst_swap = rv_ap_manager_->IsActionPointActive(address);
+  if (inst_swap) {
+    (void)rv_ap_manager_->ap_memory_interface()->WriteOriginalInstruction(
+        address);
+  }
+  // Get the decoded instruction.
+  Instruction *inst = cheriot_decode_cache_->GetDecodedInstruction(address);
+  inst->IncRef();
+  // Swap back if required.
+  if (inst_swap) {
+    (void)rv_ap_manager_->ap_memory_interface()->WriteBreakpointInstruction(
+        address);
+  }
   return inst;
 }
 
@@ -899,23 +913,11 @@
     return absl::FailedPreconditionError("GetDissasembly: Core must be halted");
   }
 
-  Instruction *inst = nullptr;
-  // If requesting the disassembly for an instruction at an action point, we
-  // need to write the original instruction back to memory before getting the
-  // disassembly.
-  bool inst_swap = rv_ap_manager_->IsActionPointActive(address);
-  if (inst_swap) {
-    (void)rv_ap_manager_->ap_memory_interface()->WriteOriginalInstruction(
-        address);
-  }
-  // Get the decoded instruction.
-  inst = cheriot_decode_cache_->GetDecodedInstruction(address);
+  auto res = GetInstruction(address);
+  if (!res.ok()) return res.status();
+  Instruction *inst = res.value();
   auto disasm = inst != nullptr ? inst->AsString() : "Invalid instruction";
-  // Swap back if required.
-  if (inst_swap) {
-    (void)rv_ap_manager_->ap_memory_interface()->WriteBreakpointInstruction(
-        address);
-  }
+  inst->DecRef();
   return disasm;
 }
 

cheriot/cheriot_top.h

diff --git a/cheriot/cheriot_top.h b/cheriot/cheriot_top.h
index eb7415d..6c0f5dd 100644
--- a/cheriot/cheriot_top.h
+++ b/cheriot/cheriot_top.h
@@ -112,6 +112,10 @@
   absl::Status ClearDataWatchpoint(uint64_t address,
                                    AccessType access_type) override;
   void SetBreakOnControlFlowChange(bool value) override;
+
+  // If successful, returns a pointer to the instruction at the given address.
+  // The instruction object is IncRef'ed, and the caller must DecRef the object
+  // when it is done with it.
   absl::StatusOr<Instruction *> GetInstruction(uint64_t address) override;
   absl::StatusOr<std::string> GetDisassembly(uint64_t address) override;
 

cheriot/debug_command_shell.cc

diff --git a/cheriot/debug_command_shell.cc b/cheriot/debug_command_shell.cc
index cbb9ff9..43537b4 100644
--- a/cheriot/debug_command_shell.cc
+++ b/cheriot/debug_command_shell.cc
@@ -1441,12 +1441,14 @@
     return absl::UnavailableError(inst_res.status().message());
   }
   auto *inst = inst_res.value();
+  auto opcode = inst->opcode();
   // If it's not a jump-and-link, it's a single step.
-  if ((inst->opcode() != *isa32::OpcodeEnum::kCheriotJal) &&
-      (inst->opcode() != *isa32::OpcodeEnum::kCheriotJalr) &&
-      (inst->opcode() != *isa32::OpcodeEnum::kCheriotJalrCra) &&
-      (inst->opcode() != *isa32::OpcodeEnum::kCheriotCjal) &&
-      (inst->opcode() != *isa32::OpcodeEnum::kCheriotCjalrCra)) {
+  if ((opcode != *isa32::OpcodeEnum::kCheriotJal) &&
+      (opcode != *isa32::OpcodeEnum::kCheriotJalr) &&
+      (opcode != *isa32::OpcodeEnum::kCheriotJalrCra) &&
+      (opcode != *isa32::OpcodeEnum::kCheriotCjal) &&
+      (opcode != *isa32::OpcodeEnum::kCheriotCjalrCra)) {
+    inst->DecRef();
     return core_access_[current_core_].debug_interface->Step(1).status();
   }
   // If it is a jump-and-link, we have to set a breakpoint on the instruction
@@ -1454,6 +1456,7 @@
   // on another breakpoint. Either way, we then remove the breakpoint we
   // inserted and return.
   uint64_t bp_address = pcc + inst->size();
+  inst->DecRef();
   bool bp_set = false;
   // See if there is a bp on that address already, if so, don't try to set
   // another one.

cheriot/test/librenode_mpact_cheriot_so_test.cc

diff --git a/cheriot/test/librenode_mpact_cheriot_so_test.cc b/cheriot/test/librenode_mpact_cheriot_so_test.cc
index 38a3522..9496c20 100644
--- a/cheriot/test/librenode_mpact_cheriot_so_test.cc
+++ b/cheriot/test/librenode_mpact_cheriot_so_test.cc
@@ -10,7 +10,7 @@
 
 constexpr char kFileName[] = "librenode_mpact_cheriot.so";
 
-constexpr char kDepotPath[] = "cheriot/";
+constexpr char kDepotPath[] = "third_party/mpact_cheriot/";
 
 class LibRenodeMpactCheriotSoTest : public ::testing::Test {
  protected: