-
Notifications
You must be signed in to change notification settings - Fork 558
Add Debug Info section #2649
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
base: main
Are you sure you want to change the base?
Add Debug Info section #2649
Conversation
|
Thanks for the PR. If you have write access, feel free to merge this PR if it does not need reviews. You can request a review using |
|
This is amazing! May I suggest that when this is merged that you share it, maybe in the #t-compiler Zulip channel to let people know it exists? I imagine there will be people who will find it interesting (even if they aren't directly involved with it). |
|
r? rustc-dev-guide |
|
also probably @Kobzol |
|
This is really incredible, thank you very much for writing this down! Tbh I would maybe send this to #t-compiler even before it gets merged, to maybe even find more people that could take a look and review this 😅 |
madsmtm
left a comment
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.
Interesting read, thanks! Always in favour of more documentation!
Maybe CC @tromey, since you held the talk that most of the existing documentation comes from.
| optimizations. In short, LLDB attempts to cache the child-values of variables (e.g. struct fields, | ||
| array elements) when stepping through code. A heuristic is used to determine which values are safely | ||
| cache-able, and `const` is part of that heuristic. Research has not been done into how this would | ||
| interact with things like Rust's interrior mutability constructs. |
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.
| interact with things like Rust's interrior mutability constructs. | |
| interact with things like Rust's interior mutability constructs. |
src/debuginfo/rust-codegen.md
Outdated
| | `(T1, T2)` | `tuple$<T1, T2>`| | ||
| | `*const T` | `ptr_const$<T>` | | ||
| | `*mut T` | `ptr_mut$<T>` | | ||
| | `usize` | `size_t`** | |
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.
Suggestion: Maybe use footnotes here ([^footnotename]), I would confuse this for a double pointer to size_t. Or at least some other symbol than *.
| # Source Information | ||
|
|
||
| TODO No newline at end of file |
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.
Highlighting this, unsure if you intend to finish this?
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.
I'll do my best to get around to the incomplete sections eventually, but it'll probably be in the order of months since I have other priorities.
My major focus was more or less "document everything necessary to understand the visualizer scripts" since right now nobody feels confident reviewing changes to them, and we'll need people who are. These docs are a precursor to rewriting the debug info test suite, which will likely require moderate-to-substantial changes to the scripts.
Most of the WIP sections require a significant time investment for me to do more research before i'd feel comfortable writing about them. Having them marked also means if anyone does know about them, it's obvious it needs to be filled out (rather than it being omitted intentionally).
In this case, visualizers deal almost exclusively with type information, so the source mapping documentation isnt super important in the short term.
| @@ -0,0 +1,12 @@ | |||
| # (WIP) LLVM Codegen | |||
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.
Again, highlighting an incomplete section.
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.
Personally, I tend towards not having this section at all, and just folding it into rust-codegen.md as a small subsection? And then strongly recommend that people read https://llvm.org/docs/SourceLevelDebugging.html if they want to work with the LLVM parts?
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.
LLVM's docs mostly cover how the IR works, but once this section is complete, it'll ideally cover the actual structure of the relevant code sections. LLVM's code is pretty obtuse and doesnt have a ton of comments. Trying to figure out what happens between rust calling a DIBuilder and lldb handing you a deserialozed node from a SymbolFile is a real headache.
It's not often that we'd need to make changes to LLVM's codegen, but it does come up. It's also relevant for writing TypeSystemRust if we ever end up doing that.
It could be bundled with rustc-codegen, but it's so much less necessary for the day-to-day maintenance i figured it should be split
| @@ -0,0 +1,62 @@ | |||
| # Debugger Visualizers | |||
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.
Suggestion: I think I'd be interested in a high-level section here about how rust-lldb is configured with the visualizers, as well as a brief overview of how the #![debugger_visualizer] attribute works.
| Rust will almost always need to override `unsigned char`, `signed char`, `char`, `u8`, and `i8`, to | ||
| (unsigned) decimal format. | ||
|
|
||
| ## Synthetic Providers |
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.
Suggestion: I'll admit that I skimmed it, but it feels like most of the information in here isn't actually specific to Rust? Maybe it'd make sense to submit this upstream to LLVM instead (in some shape or form)?
(Excepting perhaps the Vec<T> example, but even that could probably be a useful real-life example to have in the LLVM docs.)
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.
To be completely honest, LLDB is in such deep documentation debt I don't really want to touch it. I am by no means an expert on LLDB. There's lots of weird behavior and huge sections of the codebase that I'm completely oblivious about. I don't think I have it in me right now to write/research with the kind of rigor and formality I'd deem necessary for official documentation.
This section also subtly serves as a "style guide" of sorts for our visualizer scripts going forward. I can make that more explicit though.
| workarounds can help, but at the end of the day Rust's needs are secondary compared to making sure | ||
| C and C++ compilation and debugging work correctly. | ||
|
|
||
| LLDB is receptive to adding a `TypeSystemRust`, but it is a massive undertaking. This section serves |
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.
Is this really true? I wrote one of these and my recollection is that they didn't want it.
If it is true then maybe that work can be resurrected. It's all still on a branch somewhere.
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.
AFAIK it is. I've heard second hand from Greg Clayton by way of @davidbarsky that they'd be interested in a TypeSystemRust, even if that means including a rust subdirectory in LLVM's repo to allow for direct compiler integration.
|
|
||
| The `TypeSystem` is typically written to have a counterpart that can handle expression parsing. It | ||
| requires implementing a few extra functions in the `TypeSystem` interface. The bulk of the | ||
| expression parsing code should live in [lldb/source/Plugins/ExpressionParser][expr]. |
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.
FWIW wrote one of these too: https://github.com/tromey/lldb/tree/a0fc10ce0dacb3038b7302fff9f6cb8cb34b37c6/source/Plugins/ExpressionParser/Rust
| ### Generics | ||
|
|
||
| Rust outputs generic *type* information (`T` in `ArrayVec<T, N: usize>`), but not generic *value* | ||
| information (`N` in `ArrayVec<T, N: usize>`). |
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.
Is there a Rust issue for this? DWARF can represent this so it seems like rustc ought to emit it.
|
Thank you for doing this. |
| # PDB/CodeView | ||
|
|
||
| The primary debug info format for `*-msvc` targets. PDB is a proprietary container format created by |
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.
Dunno if worth mentioning but PDB can be also created using for windows-gnu* targets when linking with LLD, by passing --pdb to LLD.
Adds a pretty substantial debug section. There are pages for GDB, but those contain mostly links to GDB's documentation because I don't know a ton about it at the moment. I'm currently working on the debuginfo test rewrite, which is starts with tests for LLDB. Once I get around to writing tests for GDB, I will end up sufficiently knowledgeable to fill out the GDB sections (if they haven't been filled out already by then).
There are 2 somewhat questionable changes:
Probably resolves #1661