libgkrust.a(webrender_bindings-ee4f1448dc823c6b.webrender_bindings.7qjxnz4e-cgu.0.rcgu.o) DWARF DIE at 0x001b2d28 (class closure) has a member variable 0x001b2d2f (__0) whose type is a forward declaration, not a complete definition. #57822
Comments
|
Here's the broken object file. |
|
@philipc is this something that's easy for ddbug to check for and give more information about? |
estebank
added
A-debuginfo
|
Maybe, but I don't understand what the problem is and what needs to be checked. The type of 0x001b2d2f is 0x001b2d3d, and I don't know why it claims 0x001b2d3d is a forward declaration since its members are defined too. |
|
So the crash doesn't happen with rust-lldb, so this probably pretty low to fix. Otherwise it seems like the next steps would be to figure out what's going on in lldb's DWARFFastParserClang and why it's sad about our info. |
|
I have this occurring with rust-lldb. Sample code: extern crate rexpect;
use rexpect::session::spawn_command;
use std::process::Command;
use tempfile::*;
type R<T> = Result<T, Box<std::error::Error>>;
type Session = rexpect::session::PtySession;
#[test]
fn test() -> R<()> {
let mut p = run(&["example.txt"])?;
p.exp_regex("Deleting example.txt")?;
p.exp_eof()?;
Ok(())
}
fn run(args: &[&str]) -> R<Session> {
let directory = tempdir()?;
let directory_path = directory.path();
let mut command = Command::new("./target/debug/rust-rm");
command.args(args);
command.current_dir(directory_path);
println!("Tmp folder: {:?}", directory_path);
let mut p = spawn_command(command, Some(2000))?;
Ok(p)
}And I'm stepping into the and am running: I'm on macOS 10.14.2 with just installed cargo and rust ( |
|
@michaelwoerister It seems like #36185 has resurfaced. From what I understand this is causing some issues for WebRender devs. |
|
I'm going to look into this asap. Thanks for the test case, @xixixao, that should be very helpful! |
|
I can reproduce this on macOS and Linux. On Linux I can reproduce with a recent debug build of LLDB. |
|
@rust-lang/compiler, maybe we should make this |
|
I've been able to make some progress here. I was not able to pinpoint why exactly LLDB fails to load the type info for fields in question but for the test cases I had, the bug occurred only when LLDB tries to get the type information of fields in a closure environment. I tried debugging further but with my limited understanding of LLDB's codebase makes this very time-consuming. So, for the time being I propose to implement a workaround: If we make |
|
@michaelwoerister I may be able to look into the LLDB side further. Do you have a simple testcase I can use? I wasn't able to reproduce with the testcase given earlier. At a guess, the problem may be because we use the same name ( |
|
That would be awesome, @philipc! Are you familiar with compiling Firefox? Then getting a test case is straightforward. It's also somewhat lengthy initially so bear with me
For me, the above repro worked on Fedora 30 as well as an up-to-date macOS. It also worked with the stock macOS LLDB version, LLDB 8 on Fedora and a self-compiled LLDB from a recent LLVM master.
Maybe yeah. Unfortunately, LLDB crashed on me while debugging itself and |
|
Thanks for the instructions, @michaelwoerister! Here's a reduced testcase: (in lldb: |
|
Hex editing the DWARF to change the names of the closure types stops the error from happening. |
|
Very interesting! Generating some kind of unique name for the closure types shouldn't be too hard. |
|
Alright, so I would suggest fixing this issue by doing both:
The same should probably done for generators (I have not really looked at those yet). Implementing item (1) should be straightforward: Go into librustc_codegen_llvm/debuginfo/metadata.rs and make closure types and their fields get rust/src/librustc_codegen_llvm/debuginfo/metadata.rs Lines 684 to 691 in 7858dc2 ... and implementing this will mean that create_struct_stub() needs to grow a Closure fields are created here (because the compiler treats them as tuple fields): rust/src/librustc_codegen_llvm/debuginfo/metadata.rs Lines 1184 to 1198 in 7858dc2 Similar to the case above, For examples, one can also look for other occurrences of Implementing item (2), i.e. generating a unique type name for closures, is even simpler: Debuginfo type names are generated in librustc_codegen_ssa/debuginfo/type_names.rs, and for closures specifically the name is generated here: rust/src/librustc_codegen_ssa/debuginfo/type_names.rs Lines 193 to 195 in 7858dc2 The first field of the One could also find a more human-readable unique name for closure types (e.g. by using the I'm gonna tag this issue as |
michaelwoerister
added
E-help-wanted
|
Further reduced: fn main() {
let f = |x: ()| x;
let g = move |y| f(y);
xxx();
}
fn xxx() {}In lldb: The requirements are to have the struct for one closure included as a member of the struct for another closure, and then get lldb to print that struct. I'll take a look at implementing the fix. |
|
Thanks, @philipc! This should also make for a good regression test. |
|
@michaelwoerister Should I be using Probably not something to do right now, but maybe we should move these types inside the functions where they are defined, and then use a simple disambiguation index (are we doing something like this for mangling closure symbols?). This would be similar to using |
|
Yes, I also think that we don't need that many characters for disambiguation. I'd probably do something like: ty::Closure(..) => {
use rustc_data_structures::base_n;
output.push_str("closure-"); // <-- could also use a short prefix here
let hash = tcx.def_path_hash(def_id).0.to_smaller_hash(); // 64 bits is enough
base_n::push_str(hash as u128, 62, output); // base62 only contains alphanumeric chars
}This should result in ~11 disambiguation chars which seems fine.
Do DWARF & LLVM allow for type DIEs inside of function DIEs? What would be the implications? I can't remember if we already do something like this (we definitely use namespaces though). |
|
I haven't checked clang, but here's some output from gcc which does that: int main() {
struct Foobar {
int i;
};
struct Foobar foobar = { .i = 1 };
return foobar.i;
} |
This whole thread had me confused because I just assumed we were using def-paths (like symbol names do, and we know those are unambiguous), but I thought maybe it was some subtlety everyone else was aware of. Thanks for asking this "out loud", everything's clear now! I wonder if there are other bugs/complications arising from debuginfo not (fully) relying on the disambiguating information that everything else is based on. |
|
You should be able to create similar situations to closures via this: let pair = (
{ struct S; S },
{ struct S; S },
);This should create two structs with the same def-path except for the disambiguator on the last component (since blocks don't introduce their own def-path components). |
|
Well, I don't think DWARF actually demands that types have different names. Each DIE has some kind of ID and the |
|
@philipc I remember now: We are building a parallel tree of fn main() {
struct XYZ {
a: u32
}
let b = XYZ { a: 8 };
println!("{:?}", b.a);
}DWARF It would be great to get rid of that hack |
|
Ok, but the closure types are all being put under the Not sure if this will cause problems, but one possible difference from other languages is that these types are visible outside the function by returning |
|
@philipc Pretty sure you can get the same in C++ with |
|
@philipc I'm pretty sure |
debuginfo: give unique names to closure and generator types Closure types have been moved to the namespace where they are defined, and both closure and generator type names now include the disambiguator. This fixes an exception when lldb prints nested closures. Fixes #57822 I haven't included the `DW_AT_artificial` changes discussed in #57822 because they make the output worse IMO, but I can easily add these if still required. For example, for the new test case the output is now: ``` (lldb) p g (issue_57822::main::closure-1) $1 = closure-1(closure(1)) ``` but adding `DW_AT_artificial` changes this to: ``` (lldb) p g (issue_57822::main::closure-1) $0 = closure-1 { } ``` Note that nested generators didn't cause the exception. I haven't determined why, but I think it makes sense to add the disambiguator for them too. It feels like we still don't really understand why closures were causing an error though. r? @michaelwoerister
Closure types have been moved to the namespace where they are defined, and both closure and generator type names now include the disambiguator. This fixes an exception when lldb prints nested closures. Fixes rust-lang#57822
Some idea as #36185
The text was updated successfully, but these errors were encountered: