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

Investigate replacing most of the debugger pretty-printing scripts with traits. #65564

Open
eddyb opened this issue Oct 18, 2019 · 25 comments
Open
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Oct 18, 2019

In the case of Vec<T> and String, it would be possible to have e.g.:

impl<'a, T> DebuggerView<'a> for Vec<T> {
    type View = &'a [T];
    fn debugger_view(&'a self) -> Self::View { &self[..] }
}
impl<'a> DebuggerView<'a> for String {
    type View = &'a str;
    fn debugger_view(&'a self) -> Self::View { &self[..] }
}

We would then need:

  • support for pretty-printing &[T] and &str
  • a way to encode <X as DebuggerView>::debugger_view in X's debuginfo
    • i.e. a symbol name, or maybe we can make symbol names based on DWARF type IDs?
  • support for going through <X as DebuggerView>::debugger_view for pretty-printing
    • the method signature above should be relatively simple to call

I suspect there are better APIs that could accommodate more complex data structures, but I'm not sure what the limitation of the pretty-printer scripts are, around being able to call arbitrary functions.

There's also the possibility that we could encode this sort of information as data, instead of compiling it into functions, but that's probably a lot more work.

cc @michaelwoerister

@eddyb eddyb added the A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) label Oct 18, 2019
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 18, 2019
@jonas-schievink
Copy link
Contributor

How does this work when cross-compiling? Is it possible to easily strip this code from the executable to reduce its size while still being able to debug the binary (important for embedded)?

@eddyb
Copy link
Member Author

eddyb commented Oct 18, 2019

@jonas-schievink You mean with remote debugging? As long as the gdb server (or however you're controlling the target) lets you call functions, this should work.

Also, for embedded, would you have dynamic allocation? Pretty printers are mostly needed in that case. And even if you do, each of these impls would be a couple instructions (at least the above ones).

@jonas-schievink
Copy link
Contributor

You mean with remote debugging? As long as the gdb server (or however you're controlling the target) lets you call functions, this should work.

Ah, I see. Though this sounds like it would disrupt the target state more than the current solution. Is anything similar to this implemented by other languages?

Also, for embedded, would you have dynamic allocation?

You can, yes. I suppose in that case (if you can afford a heap) the overhead of a few instructions isn't terrible but it's still a regression nonetheless.

How would this be controlled? With -Cdebuginfo? Does that currently have any impact on generated code or is everything it adds removable with strip?

@matklad
Copy link
Member

matklad commented Oct 23, 2019

👍 on the approach, this is essentially what KotlinNative is doing: calling into runtime to "reflect" values

https://github.com/JetBrains/kotlin-native/blob/838cccf275b121878003d9c8c2f7600d78b10989/llvmDebugInfoC/src/scripts/konan_lldb.py#L178

https://github.com/JetBrains/kotlin-native/blob/838cccf275b121878003d9c8c2f7600d78b10989/llvmDebugInfoC/src/scripts/konan_lldb.py#L249

Production-grade DebugerView should be richer and express children.

The MVP here can be much simpler though:

  • add fn debug<T: std::fmt::Debug>(value: &T, buf: *const u8, buf_len: usize)
  • make sure that code, compiled with debug-info, contains instantiations of debug for all T's
  • make sure that debug info is rich enough for the debugger to get symbol name for debug::<T>

@matklad
Copy link
Member

matklad commented Oct 23, 2019

Hm, maybe even crazier:

  • expose non-generic fn debug(value: *const T, buf: *const u8, buf_len: usize)
  • make that function itself look at the debug info associated with value (this might need some additional info like a frame pointer passed it)
  • after figuring out the runtime-type of value, debug calls into fmt::Debug using the appropriate mangled name

That way, debugger can blindly call debug on any pointer, without containing logic for figuring out symbol name from a type.

Note that this actually can be prototyped as a crate! The only thing that the crate can't do is forcing eager instantiation of fmt::Debug, but, for a POC, it would be good enough to per-instantiate some stdlib types and provide generate_debug_printers!(HashMap<String, MyType>) macro.

@eddyb
Copy link
Member Author

eddyb commented Oct 23, 2019

The problem with using fmt::Debug is that I don't know the extent of the power of Python scripts.

make that function itself look at the debug info associated with value (this might need some additional info like a frame pointer passed it)

That seems... extremely complicated, compared to the debugger which already has it.

Also, if DWARF type IDs are exposed in the Python scripts, then it should be trivial to get a per-type symbol name that way.

@bjorn3
Copy link
Member

bjorn3 commented Oct 23, 2019

While I like the idea, I see two problems:

  1. Do we want to run arbitrary code during debugging? It may cause an infinite loop or stack overflow. It may also just be plain slow.
  2. Sometimes Debug instances hide sensitive information (eg encryption keys) to prevent accidentially leaking it. However while debugging you may want to be able to inspect it though.

@eddyb
Copy link
Member Author

eddyb commented Oct 23, 2019

Oh, I would prefer if this wasn't based on fmt::Debug but rather provided access, through collections, to elements, for debug purposes. So more like &_: IntoIterator for debugging.

That is, I want the debugger to do inside, what it was doing outside, e.g. Vec.

@bjorn3
Copy link
Member

bjorn3 commented Oct 23, 2019

Just a random idea: Is is possible to store &mut [MaybeUninit<T>] inside Vec<T> instead of ptr + cap or &mut [T] instead of ptr + len? That would make special casing Vec<T> inside the debugger pretty printer unnecessary.

@eddyb
Copy link
Member Author

eddyb commented Oct 23, 2019

That wouldn't work for, say, HashMap. And there's no nice way to fit both capacity and length into one slice.

@michaelwoerister
Copy link
Member

I'm not sure if there is a reliable way for calling trait methods from inside debuggers yet.

@eddyb
Copy link
Member Author

eddyb commented Oct 25, 2019

@michaelwoerister We would be exposing it as a regular function with a predictable symbol name, so that we can find it from the DWARF type information (via type ID perhaps?).

@bjorn3
Copy link
Member

bjorn3 commented Oct 25, 2019

I think we need to add the type ID as custom DWARF attribute for a type (maybe call it DW_AT_rust_type_id) to be able to access the type id from inside a debugger.

@eddyb
Copy link
Member Author

eddyb commented Oct 25, 2019

I'm not talking about our TypeId, but the code in the compiler led me to believe that "Type ID" is a DWARF concept - however, if we can invent custom DWARF attributes, we can do better than an ID, we can have an entire mangled symbol presumably.

@bjorn3
Copy link
Member

bjorn3 commented Oct 25, 2019

I dont think that DWARF has a type id concept. I believe it only cares about offsets within a debug section.

however, if we can invent custom DWARF attributes, we can do better than an ID, we can have an entire mangled symbol presumably.

Yes, that would be nicer than storing a type id.

@eddyb
Copy link
Member Author

eddyb commented Oct 25, 2019

Okay so I dug into that stuff and I found how it works:

  1. rustc_codegen_llvm tracks something it calls unique_type_id / UniqueTypeId:
    debug_context(cx).type_map.borrow().get_unique_type_id_as_string(unique_type_id)
    );
    let metadata_stub = unsafe {
    // LLVMRustDIBuilderCreateStructType() wants an empty array. A null
    // pointer will lead to hard to trace and debug LLVM assertions
    // later on in llvm/lib/IR/Value.cpp.
    let empty_array = create_DIArray(DIB(cx), &[]);
    llvm::LLVMRustDIBuilderCreateStructType(
    DIB(cx),
    containing_scope,
    name.as_ptr(),
    unknown_file_metadata(cx),
    UNKNOWN_LINE_NUMBER,
    struct_size.bits(),
    struct_align.bits() as u32,
    DIFlags::FlagZero,
    None,
    empty_array,
    0,
    None,
    unique_type_id.as_ptr())
  2. this trickles down through LLVM until it's hashed with MD5:
    https://github.com/llvm/llvm-project/blob/abd89c243a42da490dfd32368e25c896a7111a40/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L2865-L2874
  3. the resulting 64-bit part of the MD5 hash ends up in DW_FORM_ref_sig8:
    https://github.com/llvm/llvm-project/blob/abd89c243a42da490dfd32368e25c896a7111a40/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp#L370-L379

@bjorn3
Copy link
Member

bjorn3 commented Oct 25, 2019

That is only for DWARF version 4 and higher. Only those versions support type units, which contain that value as the type_signature header field.

@uberjay
Copy link

uberjay commented Nov 4, 2019

I didn't see this mentioned yet -- apologies if it has: doesn't this only work if you've got an active process? When debugging a core file, you're not going to be able to call anything. It'd be unfortunate to lose that ability.

(granted, I have never actually debugged a rust process, so I don't know the current state of the type inspector script support.)

EDIT: I mean, granted, debugging a coredump is probably 10-100x more likely in C/C++ land, but still... 😂

@eddyb
Copy link
Member Author

eddyb commented Nov 6, 2019

@uberjay I don't this it was mentioned, that's a very good point!
That would suggest it might be better to encode structural information in some data format (a bytecode? DWARF already has some features like that), as opposed to using machine code.

This would also alleviate @jonas-schievink's concern with regards to remote debugging, I think, even if it may be harder to implement overall.

pnkfelix added a commit to pnkfelix/rust that referenced this issue Nov 20, 2019
…ed vec.

I based this solution on my reading of:

https://rethinkdb.com/blog/make-debugging-easier-with-custom-pretty-printers#what-is-still-to-be-done

That post claims that there is no clean way to check for garbage pointers, and
so this PR adopts the same solution of tentatively attempting to convert a
dererence to a string, which throws a clean exception on garbage that we can
catch and recover from.

I only made the change to vec and not the other pretty printers because I wanted
to focus my effort on the simplest thing that would resolve issue rust-lang#64343. In
particular, I *considered* generalizing this fix to work on the other datatypes
in the pretty-printing support library, but I don't want to invest effort in
that until after we resolve our overall debugging support strategy; see also
issues rust-lang#60826 and rust-lang#65564.
pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 22, 2019
…r, r=alexcrichton

made gdb pretty-printing more robust when printing uninitialized vec

made gdb pretty-printing more robust when printing uninitialized vec

I based this solution on my reading of:

https://rethinkdb.com/blog/make-debugging-easier-with-custom-pretty-printers#what-is-still-to-be-done

That post claims that there is no clean way to check for garbage pointers, and
so this PR adopts the same solution of tentatively attempting to convert a
dererence to a string, which throws a clean exception on garbage that we can
catch and recover from.

I only made the change to vec and not the other pretty printers because I wanted
to focus my effort on the simplest thing that would resolve issue rust-lang#64343. In
particular, I *considered* generalizing this fix to work on the other datatypes
in the pretty-printing support library, but I don't want to invest effort in
that until after we resolve our overall debugging support strategy; see also
issues rust-lang#60826 and rust-lang#65564.

Fix rust-lang#64343
Centril added a commit to Centril/rust that referenced this issue Nov 23, 2019
…r, r=alexcrichton

made gdb pretty-printing more robust when printing uninitialized vec

made gdb pretty-printing more robust when printing uninitialized vec

I based this solution on my reading of:

https://rethinkdb.com/blog/make-debugging-easier-with-custom-pretty-printers#what-is-still-to-be-done

That post claims that there is no clean way to check for garbage pointers, and
so this PR adopts the same solution of tentatively attempting to convert a
dererence to a string, which throws a clean exception on garbage that we can
catch and recover from.

I only made the change to vec and not the other pretty printers because I wanted
to focus my effort on the simplest thing that would resolve issue rust-lang#64343. In
particular, I *considered* generalizing this fix to work on the other datatypes
in the pretty-printing support library, but I don't want to invest effort in
that until after we resolve our overall debugging support strategy; see also
issues rust-lang#60826 and rust-lang#65564.

Fix rust-lang#64343
@eddyb
Copy link
Member Author

eddyb commented Nov 27, 2019

Something I wasn't aware of, that looks very useful: Windows natvis!
Spotted new natvis definitions being added in #66597 (cc @MaulingMonkey)

This is, for example, Vec's natvis definition:

<Type Name="alloc::vec::Vec&lt;*&gt;">
<DisplayString>{{ size={len} }}</DisplayString>
<Expand>
<Item Name="[size]" ExcludeView="simple">len</Item>
<Item Name="[capacity]" ExcludeView="simple">buf.cap</Item>
<ArrayItems>
<Size>len</Size>
<ValuePointer>buf.ptr.pointer</ValuePointer>
</ArrayItems>
</Expand>
</Type>

This looks decently declarative to me, and the more advanced features are pretty interesting. I definitely prefer it over Python code, even if it's wrapped in XML.

Here's a more advanced example (HashMap from #66597):

<Type Name="std::collections::hash::map::HashMap&lt;*,*,*&gt;">
<DisplayString>{{ size={base.table.items} }}</DisplayString>
<Expand>
<Item Name="[size]">base.table.items</Item>
<Item Name="[capacity]">base.table.items + base.table.growth_left</Item>
<CustomListItems>
<Variable Name="i" InitialValue="0" />
<Variable Name="n" InitialValue="base.table.items" />
<Size>base.table.items</Size>
<Loop>
<Break Condition="n == 0" />
<If Condition="(base.table.ctrl.pointer[i] &amp; 0x80) == 0">
<!-- Bucket is populated -->
<Exec>n--</Exec>
<Item Name="{base.table.data.pointer[i].__0}">base.table.data.pointer[i].__1</Item>
</If>
<Exec>i++</Exec>
</Loop>
</CustomListItems>
</Expand>
</Type>


I could see this being integrated as either an unstable attribute or an associated const - but we could go further and generate the C expressions necessary (and/or some sort of DWARF ops) from a subset of MIR, for example, so that it has to fully go through type-checking first.


In terms of support, VS Code even supports (a subset of) natvis for gdb/lldb, but there are also other projects, like gdb-natvis and natvis4gdb.

And if we come up with our own solution that can be lowered to natvis, then we can have our own pretty-printers and also interface with all of the above as well.

@MaulingMonkey
Copy link
Contributor

natvis is indeed pretty useful! They're used by VS/VCS/WinDbg/CDB as the main means of visualizing the C++ stdlib containers, smart pointers, etc. - and they can be embedded into pdbs (as rustc does via undocumented link.exe flags) where they're automatically picked up by debuggers. They work fine on minidumps, VS can hot-reload .natvis files if they're part of your project, you can use them on third party types you don't even have source code for... there's even a non-Microsoft game console out there with natvis support in their debug engine.

Cons: Using XML for a programming language is admittedly kinda horrific, and CustomListItems amounts to that. Fortunately it's typically only needed for hash/tree containers. It's also very C++ oriented - my initial PR did a bit of mangling to make Rust debug info look C++y enough for natvis to work with slices and strs (although not reliably... something to do with fat pointer types perhaps?)

My own wild fantasies include auto-generating natvis files for rust enums, and teaching rust to gather and embed natvis files found in dependency crates (or maybe using build scripts to emit link args from said crates would be sufficient? I should experiment with that...)

@eddyb
Copy link
Member Author

eddyb commented Jan 24, 2020

Interestingly, DWARF is more flexible than I thought, and it looks like at the very least Vec<T> and String could be implemented natively in it - see #37504 (comment).

I could maybe see this scaling to HashMap (to get something akin to [(K, V)] being shown in the debugger), but for LinkedList and BTreeMap you would have some nesting wherever there's recursion through indirection.

@tromey
Copy link
Contributor

tromey commented Feb 26, 2020

When debugging a core file, you're not going to be able to call anything. It'd be unfortunate to lose that ability.

In addition to this, in gdb we normally recommend that pretty-printers not make inferior calls because it involves letting the inferior run; this can be very confusing when other threads run when you are trying to print something. Also, these calls involve stack manipulations, which is unpleasant when also trying to unwind -- gdb calls into these things when printing the stack trace.

@eddyb
Copy link
Member Author

eddyb commented Feb 26, 2020

Overall that makes me want to go for something more like "Vec<T> is a dynamic array where ptr is in this field and the length is in this other field".
Maybe offsets instead if field indices (using offset_of!?)

We can do this with a trait and an associated const, and generate DWARF from it, maybe even NatVis (if we can rely on embedding it into pdbs).

@wesleywiser
Copy link
Member

Visited during wg-debugging triage. We think building debugger visualizations in "regular" Rust code is definitely a good idea but we agree with many of the points raised about the problems of the debugger calling into the target to render values. Additionally, users compiling with debuginfo enabled in release mode for production workloads would probably not accept needing all of this extra code in the binary which is only used at debug time.

There have been various discussions about embedding (S)MIR in debuginfo which can be split from the binary and building debugger plugins that can host a proper evaluation environment. We think that is a better but more difficult route which we would like to see more fully explored.

@wesleywiser wesleywiser added the P-medium Medium priority label May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants