Skip to content

Commit

Permalink
riscv: simplify state management during examine
Browse files Browse the repository at this point in the history
This also fixes a bug when, after `examine` completion, the target still
has  `unknown` status. To reproduce this one spike, it is enough to do
the following:

---
// make sure spike harts are halted
openocd ... -c init -c 'echo "[targets]"'
---

this behavior is quite dangerous and leads to segfaults in some cases

Change-Id: I13915f7038ad6d0251d56d2d519fbad9a2f13c18
Signed-off-by: Parshintsev Anatoly <[email protected]>
  • Loading branch information
aap-sc committed Aug 15, 2023
1 parent 9260101 commit 0fbdf8c
Showing 1 changed file with 14 additions and 24 deletions.
38 changes: 14 additions & 24 deletions src/target/riscv/riscv-013.c
Original file line number Diff line number Diff line change
Expand Up @@ -1958,35 +1958,25 @@ static int examine(struct target *target)
if (dm013_select_hart(target, info->index) != ERROR_OK)
return ERROR_FAIL;

enum riscv_hart_state state;
if (riscv_get_hart_state(target, &state) != ERROR_OK)
target->state = TARGET_UNKNOWN;
target->debug_reason = DBG_REASON_UNDEFINED;

enum riscv_hart_state state_at_examine_start;
if (riscv_get_hart_state(target, &state_at_examine_start) != ERROR_OK)
return ERROR_FAIL;
bool halted = (state == RISCV_STATE_HALTED);
if (!halted) {
const bool hart_halted_at_examine_start = state_at_examine_start == RISCV_STATE_HALTED;
if (!hart_halted_at_examine_start) {
r->prepped = true;
if (riscv013_halt_go(target) != ERROR_OK) {
LOG_TARGET_ERROR(target, "Fatal: Hart %d failed to halt during %s",
info->index, __func__);
return ERROR_FAIL;
}
target->state = TARGET_HALTED;
target->debug_reason = DBG_REASON_DBGRQ;
}
/* FIXME: This is needed since register_read_direct relies on target->state
* to work correctly, so, if target->state does not represent current state
* of target, e.g. if a target is halted, but target->state is
* TARGET_UNKNOWN, it can fail early, (e.g. accessing registers via program
* buffer can not be done atomically on a running hart becuse mstatus can't
* be prepared for a register access and then restored)
* See https://github.com/riscv/riscv-openocd/pull/842#discussion_r1179414089
*/
const enum target_state saved_tgt_state = target->state;
const enum target_debug_reason saved_dbg_reason = target->debug_reason;
if (target->state != TARGET_HALTED) {
target->state = TARGET_HALTED;
target->debug_reason = DBG_REASON_DBGRQ;
}

target->state = TARGET_HALTED;
target->debug_reason = hart_halted_at_examine_start ? DBG_REASON_UNDEFINED : DBG_REASON_DBGRQ;

/* Without knowing anything else we can at least mess with the
* program buffer. */
r->debug_buffer_size = info->progbufsize;
Expand Down Expand Up @@ -2059,13 +2049,13 @@ static int examine(struct target *target)
if (set_dcsr_ebreak(target, false) != ERROR_OK)
return ERROR_FAIL;

target->state = saved_tgt_state;
target->debug_reason = saved_dbg_reason;

if (!halted) {
if (state_at_examine_start == RISCV_STATE_RUNNING) {
riscv013_step_or_resume_current_hart(target, false);
target->state = TARGET_RUNNING;
target->debug_reason = DBG_REASON_NOTHALTED;
} else if (state_at_examine_start == RISCV_STATE_HALTED) {
target->state = TARGET_HALTED;
target->debug_reason = DBG_REASON_UNDEFINED;
}

if (target->smp) {
Expand Down

0 comments on commit 0fbdf8c

Please sign in to comment.