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

debuginfo: Don't emit DW_LANG_RUST unless it's explicitly demanded #33097

Closed
wants to merge 1 commit into from

Conversation

michaelwoerister
Copy link
Member

It turns out that LLDB can't handle DW_LANG_RUST and that GDB has problems with DW_LANG_C_PLUS_PLUS. Setting the language to 0x0 is the only value that will make both debuggers happy enough. I want proper debuginfo :(

This will hopefully take care of #33062 and #32520.

Heads up, @tromey! You'll still want the compiler to emit DW_LANG_RUST as the language, so you'll have to pass -Z rust-debuginfo to it.

@rust-highfive
Copy link
Collaborator

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

@michaelwoerister
Copy link
Member Author

r? @alexcrichton

@tromey
Copy link
Contributor

tromey commented Apr 19, 2016

If 0x9000 worked but the new value does not, then how about just reverting the patch that switched the value? I've already made my gdb work with both values. Setting it to 0 means I'll have to do producer sniffing to properly understand that the CU is Rust, which is pretty ugly.

@michaelwoerister
Copy link
Member Author

Unfortunately, newer versions of LLDB don't work with 0x9000 anymore. In older versions, it would always fall back to C/C++ behavior, not matter what.

But let me double check that...

@michaelwoerister
Copy link
Member Author

OK, I just tried using 0x9000 with LLDB and unfortunately that doesn't work.

@alexcrichton
Copy link
Member

alexcrichton commented Apr 19, 2016

@bors: r+

Also cc #32734 for posterity's sake

@bors
Copy link
Contributor

bors commented Apr 19, 2016

📌 Commit e8828e1 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 19, 2016

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Apr 19, 2016

📌 Commit e8828e1 has been approved by alexcrichton

@alexcrichton
Copy link
Member

Interesting, cc @Manishearth, it appears that if you edit a comment (I edited "stake" to "sake" in the above comment) that bors will receive two notifications and attempt to consider them both as approvals

@tromey
Copy link
Contributor

tromey commented Apr 19, 2016

Thanks for checking @michaelwoerister
I will fix my gdb to do producer sniffing. It would be good if someone could fix lldb here.

@vadimcn
Copy link
Contributor

vadimcn commented Apr 21, 2016

What was GDB's problem with DW_LANG_C_PLUS_PLUS ?

@Manishearth
Copy link
Member

Can we try #33062 (comment) and the other solutions in that thread before landing?

@michaelwoerister
Copy link
Member Author

@Manishearth It would be easy enough to make a pull request to LLDB that makes the change that @jasonmolenda suggests. However, that doesn't solve the problem that the current version of LLDB will not be usable with binaries that we produce.

@michaelwoerister
Copy link
Member Author

@vadimcn With DW_LANG_C_PLUS_PLUS the string ::(void) 0 started to show up all over the place (mostly within enum values, I think). I'm not sure right now if it also caused problems with the pretty printers.

@jasonmolenda
Copy link

jasonmolenda commented Apr 22, 2016

Yeah @michaelwoerister I don't think there's any way to handle this correctly for both lldb and gdb today (with the binaries that exist in distributions). I don't understand what you mean when you say that the string ::(void) 0 was showing up if you use DW_LANG_C_plus_plus for a rust debug info - is that from lldb or gdb or something involved with rust? It seems like having lldb/gdb treat rust binaries as if they were C++ is good enough for basic functionality, so marking them that way instead of defaulting to C/C++ would be good. And getting patches into gdb/lldb to handle DW_LANG_Rust correctly (treating it as c++ for now, until someone puts in the time to add more sophisticated support) so you can switch to using that language code once those debuggers are included in popular distributions.

(I think I'm just saying things that everyone here already knows - sorry if I'm just adding noise to this.)

@tromey
Copy link
Contributor

tromey commented Apr 22, 2016

I don't understand what you mean when you say that the string ::(void) 0 was showing up

For reasons unknown to me, gdb tries to demangle field names; tuple fields are emitted as __N, where N is some integer; and __0 demangles as ::(void) 0. So, if you print a tuple or ptype a tuple type, you get this nonsense.

@bors
Copy link
Contributor

bors commented Apr 22, 2016

⌛ Testing commit e8828e1 with merge 1b09c1b...

@bors
Copy link
Contributor

bors commented Apr 22, 2016

💔 Test failed - auto-win-msvc-64-opt-mir

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Apr 22, 2016 at 8:29 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-msvc-64-opt-mir
http://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-mir/builds/405


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#33097 (comment)

@bors
Copy link
Contributor

bors commented Apr 23, 2016

⌛ Testing commit e8828e1 with merge 4cf098b...

@bors
Copy link
Contributor

bors commented Apr 23, 2016

💔 Test failed - auto-linux-64-debug-opt

@sanxiyn
Copy link
Member

sanxiyn commented Apr 25, 2016

Buildbot tripped LLVM assertion checking for language tag.

rustc: /buildslave/rust-buildbot/slave/auto-linux-64-debug-opt/build/src/llvm/lib/IR/DIBuilder.cpp:143:
llvm::DICompileUnit* llvm::DIBuilder::createCompileUnit(unsigned int, llvm::StringRef, llvm::StringRef, llvm::StringRef, bool, llvm::StringRef, unsigned int, llvm::StringRef, llvm::DIBuilder::DebugEmissionKind, uint64_t, bool):
Assertion `((Lang <= dwarf::DW_LANG_Fortran08 && Lang >= dwarf::DW_LANG_C89) || (Lang <= dwarf::DW_LANG_hi_user && Lang >= dwarf::DW_LANG_lo_user)) && "Invalid Language tag"' failed.

@jasonmolenda
Copy link

Hm. From include/llvm/Support/Dwarf.def - are you emitting something other than 0x1c as your language code? This is the DWARF 5 (not yet finalized) spec, maybe you're emitting some vendor extension language code? But in that case you'd be using something between DW_LANG_hi_user and DW_LANG_lo_user.

// New in DWARF 5:
HANDLE_DW_LANG(0x0014, Python)
HANDLE_DW_LANG(0x0015, OpenCL)
HANDLE_DW_LANG(0x0016, Go)
HANDLE_DW_LANG(0x0017, Modula3)
HANDLE_DW_LANG(0x0018, Haskell)
HANDLE_DW_LANG(0x0019, C_plus_plus_03)
HANDLE_DW_LANG(0x001a, C_plus_plus_11)
HANDLE_DW_LANG(0x001b, OCaml)
HANDLE_DW_LANG(0x001c, Rust)
HANDLE_DW_LANG(0x001d, C11)
HANDLE_DW_LANG(0x001e, Swift)
HANDLE_DW_LANG(0x001f, Julia)
HANDLE_DW_LANG(0x0020, Dylan)
HANDLE_DW_LANG(0x0021, C_plus_plus_14)
HANDLE_DW_LANG(0x0022, Fortran03)
HANDLE_DW_LANG(0x0023, Fortran08)

@michaelwoerister
Copy link
Member Author

We are emitting 0x0, i.e. 'not specified'. I wonder why I didn't run into this assertion when when testing locally.

@michaelwoerister
Copy link
Member Author

OK, so since this doesn't really work the way I expected and an LLDB patch restoring the old behavior has landed, I'm going to close this PR.
I'll try to come up with a way of warning users of unsupported LLDB versions instead.

@vadimcn
Copy link
Contributor

vadimcn commented Jun 14, 2016

So the fix will be in LLDB 3.9?
LLDB currently bundled with XCode on OSX is unusable with Rust: can't step by line and can't see any variables. AFAIK, XCode 7.3 is based on LLVM 3.7. Looks like it'll be a while till they get to 3.9.

I wonder if we should reconsider this PR or something like it... How about a "debugger-tuning" switch, like clang has?

@jasonmolenda
Copy link

I cherry-picked the change on to the swift 3.0 branch for lldb, and it is included in the Xcode 8.0 preview release to apple developers that went public today.

@alexcrichton
Copy link
Member

Awesome, thanks @jasonmolenda!

@vadimcn
Copy link
Contributor

vadimcn commented Jun 14, 2016

Verified that lldb-360.1.25 (Xcode8) resolves this issue. Thanks @jasonmolenda!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants