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: Overhaul symbol table parsing (add .mdebug and .sndata support) and add inspector GUI for complex data types #10224
base: master
Are you sure you want to change the base?
Conversation
Improved error handling, simplified AST, etc New commit hash: 605ce7a5657f564d7afce157f132ceb2f3447be5
New commit hash: 001c67f6b66dfe56b2ffb70e5edbe3549a0be65b
I don't know if anybody else agrees, but is there any reason this couldn't be in the debugger? I feel like they are kind of doing similar jobs since we have function symbols in there, and it would be nice to be able to right click on a variable and do "Go to in memory view" etc, all these features just feel like they would be at home as part of the debugger, rather than a separate screen. Great features though! Edit: just to clarify what I was thinking, if it was have it as a tab or something, so it doesn't add extra clutter to the debugger, not all of it on one screen, that might be a bit much! |
There isn't any particular reason, especially now memory consumption is down a lot. That said, I wanted to focus on getting the functionality working first, then worry about integrating it with the existing debugger window. If anyone has more specific design ideas, certainly feel free to suggest them. One constraint I have is that I think the watch window should at least be accessible alongside the rest of the debugger, so people can observe how values change as they step through code, just like with a regular GUI debugger. I think having it as a tab at the bottom would make sense, maybe with the option to pop it out as its own window. Perhaps Qt Dock Widgets or KDDockWidgets would make sense here? |
Yeah no worries. I believe Fobes was thinking about changing the debugger so it was separate (docked?) windows, because the debugger is a bit cramped, so it would be nice to split it out, but I want expecting you to go that far with scope lol |
This will allow users to inspect the values of local variables, and possibly in the future parameters.
Got the stack tab working, with some caveats. The biggest issue is that for some functions the live ranges seem to be invalid. It should be possible to display the values of parameters as well as locals, although the registers stored in the symbol table seem to be the registers used in the body of the function, not the prologue, so maybe there's something to be done there to make that more user friendly if you just break on the first instruction of a function. |
Some further thoughts about integrating this with the debugger window: One challenge I see is how we're going to approach the multiple sources of truth that are the two different symbol tables. I see two different approaches to deal with this:
The latter would probably be cleaner, although also more invasive. |
I agree the latter sounds like a better option, try to keep the symbols unique, we don't really want multiple sources of truth if we can help it, but this is going to be true if it's integrated with the debugger or not. |
Little update: I'm still working on this. Building the new data structures and porting my existing code to use them is taking some time. I'll copy it all into this PR when it's all relatively stable. |
This introduces a new "symbol database" which will allow for the SymbolMap class to replaced entirely, so we can load all the symbols from multiple different symbol tables into a single unified data structure. There are tons of other improvements too, including lots of fixes for STABS parsing edge cases. For now, the data inspector has been modified to use the new version of the code. New commit hash: f79a17c1edf8e53fc226f309407ccccb2c1caf91
62c0937
to
79c2dba
Compare
Noticed an annoying |
New CCC commit hash: fba00ad19ffe3c9e54b757ebf10a83ea727c68e5
Previously MIPSAnalyst::ScanForFunctions would generate functions that overlapped those read in from SNDLL symbol tables since the sizes stored for those symbols would be zero (unknown).
Previously expanding a group node wouldn't automatically update the value displayed for its children, so you would get a delay of up to 1 second until the value was displayed.
This appears to just be a typo.
For example labels.
False alarm. Or rather, the wrong alarm sounded. What I noticed was that symbols sometimes weren't being substituted into the disassembly properly. I turns out this was caused by a typo in this commit. Because instructions disassembled outside of a function won't touch this code path it tricked me into thinking it was a problem with my PR. The flurry of commits is because while investigating this issue I noticed a bunch of other problems, which actually were bugs with this PR. These are all fixed, with the exception of handling some cases where function symbols overlap (e.g. because of overlays which all get dumped into a single symbol table), although it would probably be okay to wait for a future PR to see about handling that better as DWARF support may help with that I also made some other miscellaneous improvements while I was at it. EDIT: |
New CCC commit hash: b0959281abb76f69aeba99c4c5bb3da95894e9d8
Is it possible to squash the commits down into a smaller number? For example, one to add the new demangler, another to swap over to it, etc. It's quite difficult to review 252 commits over hundreds of files, and it's also not rebase-mergeable (which is our preferred technique). |
I'll look into that, although I'll have to learn more about rebasing first. I didn't previously spend too much time on that since the other contributors said it would be squashed anyway. |
For smaller PRs, yeah, we can just squash them on merge. But with 50k lines added, I'd prefer to split it up, to make bisecting issues easier. |
Description of Changes
I have imported symbol table parsing, symbol database and C++ AST code from CCC, entirely replacing the existing symbol map class, and implemented a GUI to inspect data structures in memory using this information.
New screenshot:
Old screenshot:
Some highlights:
Progress:
Possible future extensions:
Rationale behind Changes
Many games shipped with debug symbols in this format, so this would be an extremely useful addition for the modding communities of said games.
Suggested Testing Steps
Some games with .mdebug symbols (data types, functions, globals):
A game with SNDLL symbols (functions, globals):