Fixes issue where the 'next' command in the CLI fails.
PiperOrigin-RevId: 657634577
Change-Id: Id3c42432c4b6c92ff157fabaa7896a74f78514f9
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;
}
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;
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.
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:
