Skip to content

Conversation

@JanMatCodasip
Copy link
Collaborator

After single step operation, we should not assume that the halt
reason is single-step. There can be a higher-priority halt cause,
e.g. a breakpoint.

The real halt reason should be obtained from the target (dcsr.cause).

After single step operation, we should not assume that the halt
reason is single-step. There can be a higher-priority halt cause,
e.g. a breakpoint.

The real halt reason should be obtained from the target (dcsr.cause).
Copy link
Collaborator

@timsifive timsifive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks right. What testing have you done for this? I'm concerned with some weird case where gdb doesn't work right because it expects that state to be single stepped after issuing that.

@JanMatCodasip
Copy link
Collaborator Author

JanMatCodasip commented Jun 8, 2021

What testing have you done for this?

I've tested this internally against the LLDB debugger (which speaks the GDB Remote protocol, same as GDB). Without the patch, the debugger did not report a watchpoint hit during single-stepping to the user.

For GDB, I need to run riscv-tests/debug, after I resolve some locally occurring riscv-test issues mentioned in #563.

@timsifive
Copy link
Collaborator

Assuming e.g. all the spike32-2 tests from riscv-tests/debug pass then I'm fine merging this.

@JanMatCodasip
Copy link
Collaborator Author

Assuming e.g. all the spike32-2 tests from riscv-tests/debug pass then I'm fine merging this.

I've run the riscv-tests on this change here: JanMatCodasip#3
It turns out the impact of this change is large and many tests now fail. I am parking this change (draft) until I manage to find the cause and fix it.

I still believe reporting the real halt cause to the debugger when stepping is correct approach, yet this change is not sufficient as is and something else needs to be updated as well.

@JanMatCodasip
Copy link
Collaborator Author

Closing this outdated merge request.

The main problem appears to be that riscv_openocd_step() reports resume + halt to GDB every time, but "internal" steps (e.g. when resuming from a trigger) should be hidden from the GDB. This can be fixed in a separate merge request.

@JanMatCodasip JanMatCodasip deleted the fix-halt-reason-after-singlestep branch January 13, 2025 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants