Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugger: Step-over WAIT can fail #1194

Closed
tomcw opened this issue Mar 7, 2023 · 12 comments
Closed

Debugger: Step-over WAIT can fail #1194

tomcw opened this issue Mar 7, 2023 · 12 comments
Assignees
Milestone

Comments

@tomcw
Copy link
Contributor

tomcw commented Mar 7, 2023

Repro:

  • A=$FF ; WAIT for $28882 cycles
  • PC is at code: JSR WAIT ($FCA8)
  • CTRL+SPACE (to step-over)
  • Debugger stops inside WAIT ($FCAC) after $280E8 cycles, and with $1A on the stack
  • NB. At this point you can't SHIFT+SPACE to step-out, as the WAIT value ($1A) is still on the stack, so you need to single-step until after the PLA.

NB. Step-over succeeds for JSR WAIT for other values of A, eg. $78, $7F, $F0, $F8, $FD.
But also fails for A=$FE (with $0E on the stack).

@tomcw
Copy link
Contributor Author

tomcw commented Mar 7, 2023

The reason for it stopping in the WAIT function is that the Debugger's CmdStepOver() (from an initial JSR) only attempts to do 0xFFFF steps before stopping.

So a crude fix would be to just up this max number of steps.

But it'd be a better user experience to report that the step-over cmd stopped because it'd done the max number of steps.

@audetto
Copy link
Contributor

audetto commented Mar 11, 2023

Why not move to 64 bit integers for all cycle related counting?
Including nExecutedCycles which I have always found super complicated.

@Michaelangel007
Copy link
Contributor

Michaelangel007 commented Mar 12, 2023

This is a known issue.

The 64K limit was placed to avoid the debugger "hanging" while it continues to run code. I don't have any problems with it being updated to say a 20-bit number.

@sicklittlemonkey
Copy link
Contributor

sicklittlemonkey commented Mar 12, 2023 via email

@audetto
Copy link
Contributor

audetto commented Mar 12, 2023

Why not move to 64 bit integers for all cycle related counting? Including nExecutedCycles which I have always found super complicated.

Not quite the same thing. Ignore my comment.

@Michaelangel007
Copy link
Contributor

That's not a bad idea Nick! i.e. Every 64K cycles print a warning to the console.

Warning: Stepping-over taking a long time. > 64K cycles.

The hotkey is a little more complicated since it seems we should have two keys along with adding a new (sub-state) to the debugger:

  • ESC Cancel stepping-over
  • ??? Keep stepping over

@tomcw
Copy link
Contributor Author

tomcw commented Mar 12, 2023

You could just default to "keep stepping-over", so you only need ESC to cancel.

Warning: Step-over taking a long time (> 64K opcodes). ESC to cancel operation.

@Michaelangel007
Copy link
Contributor

Michaelangel007 commented Mar 19, 2023

Repro:

<F7>
BPX 302
<F7>
CALL-151
300:A9 FF 20 A8 FC 60
E003G
CALL 768
<Ctrl-Space>

@Michaelangel007
Copy link
Contributor

Michaelangel007 commented Mar 19, 2023

Trivial fix addresses the CPU stopping inside WAIT with A=FF.

g_nDebugSteps = 0xFFFFF; // GH #1194

I've requested feedback from Nox Archaist (Mark and qkumba via Discord) to see if they want/need a higher value.

@Michaelangel007 Michaelangel007 self-assigned this Mar 20, 2023
@Michaelangel007 Michaelangel007 added this to the 1.30.15 milestone Mar 20, 2023
@Michaelangel007
Copy link
Contributor

Screenshots showing new behavior in PR #1203

tomcw pushed a commit that referenced this issue Mar 31, 2023
. QoL cleanup (show RTS address) for step-over failure cases
. Add source code for repro test 1 and 2
@tomcw
Copy link
Contributor Author

tomcw commented Mar 31, 2023

The fix from @Michaelangel007 was:

  • Up the max steps to 0xFFFFF (from 0xFFFF)
  • If step-out succeeds the debugger' feedback is:
    • "Stop reason: Register PC matches 'Go until' address $nnnn"
  • If step-out fails, new (QoL) feedback from the debugger is:
    • "ERROR: Didn't step over JSR! (RTS $nnnn not found!)"
    • "INFO: Didn't step over JSR! (RTS $nnnn on top of stack.)"
    • "WARN: Didn't step over JSR! (Stack has RTS $nnnn but needs fixup: $xx bytes)"

@Michaelangel007: this is assigned to you, so please close if you are happy that all work has been done.

@Michaelangel007
Copy link
Contributor

Michaelangel007 commented Mar 31, 2023

Thanks for the merge and summary! I bumped up the debugger version to 2.9.1.17 for this issue (db5b668) and updated the Debugger_Changelog.txt. (Forgot to do that in the PR.)

@tomcw When the next version of AppleWin is about to be released (1.30.15) I'll need to bump the debugger version to 2.9.2.0.
I've added task #1206 for milestone 52:

  • Bump Debugger version to 2.9.2 for 1.30.15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants