Debugger: Implement base code tracing logic. and feature to auto-step through code. #10771
Conversation
|
If I should be worried about unintentional logic changes, or if someone wants to test various versions, I can re-push with the update split into a second commit. |
|
Rebase. I changed the register autostep context menu a little. Before you had to click on the column that lists registers, but now you have to click on the data columns. I think it's more inline with how the menu system is setup. It's a little annoying to get the actual register (r12 for example). If there's some way I can rewrite the logic to be more clear, let me know. Not sure if I should setup some kind of switch for it. |
|
@TryTwo |
|
Thanks for taking the time to review! I think I cleaned up the AutoStep returns quite a bit. |
|
Thanks for all the work on reviewing this! About breakpoints. It's a bit mixed. Would someone want to know the location of the value they are tracking when at an arbitrary code line with a BP? Secondly, would running into breakpoints that aren't relevant be annoying? You might have to juggle breakpoints if they are triggered. |
TryTwo
force-pushed
the
PR_AutoStep
branch
3 times, most recently
from
ab045a2
to
5dfdbee
Compare
|
I added the register in question to the menu item. It had an added bonus of letting me filter out code lines that won't work with the autostep ("mtctr r3" for example has no comma in it, so would produce an empty QString, which would disable the menu as there's no target to list). |
While I don't have a stance whether which behaviour should be preferred (that's why I referred to it as a bug/feature?), I do think it's important to make its behaviour clear for the user as it's currently not obvious. That's why it should be written somewhere that (instruction/memory) breakpoints are ignored. A user might be looking for several states update at the same time using memory BPs. Using this new feature |
|
If I wanted to check breakpoints, do you think building a list of addresses that are active (M)BPs and then checking against that list would be good? |
|
I'd rather have this kind of feature added in a follow-up PR, that's why I was suggesting to just change the text. Ideally, we'd need proper functions to be exposed to know if Step/StepIn/StepOut function succeeded and if a (M)BP was hit. Then, we'd need a function exposed from PowerPC to run the code until some conditions happen. That way we could have both, one ignoring BP and one which doesn't. Something similar to what some debuggers did, like VS with Run To Cursor / Force Run To Cursor. |
|
Rebased. Added the "Run until (ignoring breakpoints)" to code and register widget. |
| trace.AutoStepping(true); | ||
| emit Host::GetInstance()->UpdateDisasmDialog(); | ||
|
|
||
| // Could add timed out popup |
There was a problem hiding this comment.
The return value of AutoStepping isn't checked and we don't have any way to know if it succeeded or not. Can't the method in CodeViewWidget be refactored and used here? Maybe in a member function of the CodeTrace class?
There was a problem hiding this comment.
I tacked the register autostep on as a quick way to navigate to a register in CodeView. I can check for an error message, but I don't think it's necessary for succeeding.
Not sure what I would put in a CodeTrace member function to share the logic. I don't think I can do any Qt stuff in CodeTrace, since it's not a widget. Open to suggestions.
There was a problem hiding this comment.
Fair point regarding Qt stuff, I can't think of anything pretty either. What you've currently implemented instead sounds mostly fine.
…egister breakpoints. Add CodeViewWidget autostepping to track a value.Debugger
| trace.AutoStepping(true); | ||
| emit Host::GetInstance()->UpdateDisasmDialog(); | ||
|
|
||
| // Could add timed out popup |
There was a problem hiding this comment.
Fair point regarding Qt stuff, I can't think of anything pretty either. What you've currently implemented instead sounds mostly fine.
Adds CodeTrace file and two "simple" uses for it: Auto-stepping through code to track a value from the PC, and register widget auto-stepping until a target register appears.
When right-clicking PC in codeview, new options appear. Operates on target (left-most) register in instruction. Tracks the target value, not just the register, so tracking can hop to another register or memory if the value is shuffled there. May track multiple registers at once if the value is duplicated.
It can timeout if it takes too long, but will give you the option to keep going. When you get a hit, you can keep all the registers/memory that contains the value loaded, and continue tracking. The pop-up window will trap focus so the option to continue running isn't invalidated by performing actions outside the pop-up, which is a little annoying..
Mainly designed for a target value being shuffled through temp memory and registers for thousands of lines of code before being used. I've spent hours manually tracking values through code and memory load/stores, but this does it in a few clicks.
Part of my incremental effort to work up to my old code trace PR #8838. I thought this would be a smaller step, but it turned out to require more effort and code than I originally intended. If the codetrace.cpp is more verbose than expected, it's to prepare for the ability to record and search what happened in a codetrace widget, if I get that finished. I also left out backtracing logic, because it can't be used in auto-stepping, but I could re-include it just so the codetrace file is more complete.
Not sure if codetrace.cpp is in the correct folder.
Limited support for PS and various specialized instructions.
The tiny function IsInstructionLoadStore shows up in the codeview widget as well, not sure if I should point them all to one place.