-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add support for Disassembly Source for GDB #1237
base: main
Are you sure you want to change the base?
Conversation
47c32b4
to
31f3dac
Compare
Do you understand this condition? I would have thought it would be Refers to: src/MIDebugEngine/AD7.Impl/AD7Disassembly.cs:122 in d63c422. [](commit_id = d63c422, deletion_comment = False) |
Need to handle skipping the first instruction in this path.
Refers to: src/MIDebugEngine/AD7.Impl/AD7Disassembly.cs:125 in d63c422. [](commit_id = d63c422, deletion_comment = False) |
@@ -118,52 +136,114 @@ public int Read(uint dwInstructions, enum_DISASSEMBLY_STREAM_FIELDS dwFields, ou | |||
prgDisassembly[iOp] = FetchBadInstruction(dwFields); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need a similar change here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we could probably combine the two code paths together.
// If we have a new line and the current line is greater than the previously seen source line. | ||
// Try to grab the last seen source line + 1 and show a group of source code. | ||
// Else, just show the single line. | ||
uint startLine = isNewLine && currentLine > _dwLastSourceLine ? _dwLastSourceLine + 1 : currentLine; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uint startLine = isNewLine && currentLine > _dwLastSourceLine ? _dwLastSourceLine + 1 : currentLine; | |
uint startLine = isNewLine && !isNewDocument && currentLine > _dwLastSourceLine ? _dwLastSourceLine + 1 : currentLine; |
This PR adds Location and Line to Disassembly Response.
2f7a529
to
b7870f5
Compare
Based on godbolt, it looks like lines out of order is intended. |
If it is alright, I can do a Pull Request for interleaving source in assembly. I had to do it for my own debugger and made similar changes to AD7. There are some unexpected gotchas in the parsing of the gdb MI results for mixed source/asm. My changes are on https://github.com/haneefdm/MIEngine Will work with gdb version 8+ (Jun 4 2017) where they revamped (and deprecated) some of the ways disassembly is produced. |
I can't do a PR for MIEngine with this PR being outstanding. I looked at the files in this PR. There are a couple of things jump out at me. My changes are much simpler and not sure we have to worry about source lines being in order, etc. One thing missing (and should be fixed) in my changes however are calls to ConvertDebuggerLineToClient and ConvertDebuggerColumnToClient My own debugger is here https://github.com/Marus/cortex-debug/blob/master/src/backend/disasm.ts I have two disassemblers in there and hoping to get rid of the old one and replace it with the new DAP protocol version so it can work nicely with the VSCode debug infrastructure. |
@haneefdm One issue that came up with this feature was when requesting address ranges outside of the user's space or a huge size request, GDB would return with no source information, which required us to find each range per symbol and re-request disassembly from GDB. It seemed like GDB would only return source information if it actually had matching symbol information, but at any point if it was missing, it would remove it from the whole data-disassemble response. |
*/ | ||
foreach (TupleValue src_and_asm_line in src_and_asm_lines) | ||
{ | ||
uint line = src_and_asm_line.FindUint("line"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Watch out, this can fail (Exception) because "line" can be missing. When "line" is missing so will the "file" or "filename". I did it backwards. First I check for the file and if it exists and the line exists, then I call FindUnit("line")
. It happens when a few instructions have source info but a few don't in the same addr-rane. If all don't have source-info, then there will be no src_and_asm_line
either. My code looks like.
string file = process.GetMappedFileFromTuple(item);
uint line = !String.IsNullOrEmpty(file) && item.Contains("line") ? item.FindUint("line") : 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see you are disassembling twice. I did not find that necessary, but you may have seen things I haven't. You can ignore my previous comment but I would still guard against not having file/source info in the middle of normal looking stuff.
From what I saw, the request for source info (mode 4 or 5) degenerates into a request of request for mode 2. At least, the results will look like that. This just baffled me at first. This happens especially with bad/unknown memory regions and happened even within a proper memory region when source information was missing. Yup, address ranges are a big problem. I am still experimenting with this, btw. I am reading gdb source code to confirm what I guessed -- lot left to read though. Also want to see what Ecliipse does. Here is what I plan to do in the short term for my debugger
The following does not work for native programs because technically the entire memory is the same and super large. So, no regions are defined.
The above is what is possible even if very little is used (actual boundaries). For the same executable, using objdump, I get the following which is what this executable is using. In your case, you won't know which memory region got mapped to which virtual one.
|
This PR adds Location and Line to Disassembly Response.
CTRL+F
main.cpp
forSource
andLine
filled out.Example Response:
Disassemble Response