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

Addition of debug info to official builds #36452

Closed
larsbergstrom opened this issue Sep 13, 2016 · 24 comments
Closed

Addition of debug info to official builds #36452

larsbergstrom opened this issue Sep 13, 2016 · 24 comments

Comments

@larsbergstrom
Copy link
Contributor

Currently, it's hard to debug some Rust code in Firefox because the standard library does not have symbol information included. That means that all of the code that gets inlined from the stdlib has no source location info. It would be great to generate official builds with symbol info and package that in the official builds.

cc @alexcrichton @froydnj @luser

@luser
Copy link
Contributor

luser commented Sep 13, 2016

There are two related issues here:

  1. This makes our crash reports worse, since many frames in a crash report from libstd will be missing function names / source line info. (The exception here is generics.)
  2. This makes the debugging story worse. I was trying to debug an issue with Rust panics in Firefox on Win64 and it got incomprehensible very quickly, especially since we build with lto, so your simple Rust function winds up with a ton of inlined bits of libstd.

Neither of these are huge issues yet, since we don't have much Rust code. As we pull things like Stylo in, they're going to quickly become a problem.

Rust is in a unique situation here compared to C or C++. With those languages you typically dynamically link the standard library, so you can always get debug symbols for the one you're using after-the-fact. Since Rust always statically links in libstd if you don't have debug symbols at build time you are out of luck.

@luser
Copy link
Contributor

luser commented Sep 13, 2016

Related, Rust omits frame pointers by default, so not having debug symbols in libstd likely means not getting good backtraces through libstd in a debugger. (I suspect this works OK on Linux because rustc does emit .eh_frame sections for the object files in libstd, even on x86.)

@retep998
Copy link
Member

This would really improve the debugging experience on Windows, in particular with msvc. If a function was generated with debuginfo, then VS's debugger is able to display the unmangled name, and can even do step by step debugging through the source code if it is available. When stepping into a function from libstd, all it sees is an opaque mangled symbol which makes it hard to tell what is going on.

@larsbergstrom
Copy link
Contributor Author

cc @vvuk, re: the thread in #servo on name mangling and MSVC.

@vvuk
Copy link
Contributor

vvuk commented Oct 13, 2016

So I think to do this, we need:

  • build with debuginfo=true in a config.toml for a rustbuild
  • when installing the various bits into the stage2/ dest dir, make sure the pdb files get copied along
  • when packaging, either package the pdb files directly, or put them in a separate -debuginfo component for rustup

If someone can give me a pointer on where to look for each of these three things, I can create PRs for this.

@luser
Copy link
Contributor

luser commented Oct 18, 2016

We discussed this on IRC, but I think all we actually need is:

  • build with debuginfo=true in a config.toml for a rustbuild

Since the bits of lib{core,std} that people will actually be linking against are rlibs, which should have the debug info embedded in the object files contained within.

@retep998
Copy link
Member

Copying along and packaging pdb files is useful for debugging when you're linking against the dylib crates (which only plugins should ever do), or for debugging rustc itself. For applications that statically link libstd, just enabling debuginfo in config.toml is sufficient.

@cuviper
Copy link
Member

cuviper commented Oct 18, 2016

@retep998 Do you mean enabling debuginfo in your own Config.toml? That only helps for generic or inline-able code from libstd. Anything that was actually compiled to object code in the rlib won't have debuginfo that way. (At least for ELF+DWARF -- I'm not sure how Windows pdbs work with rlib.)

@brson
Copy link
Contributor

brson commented Oct 18, 2016

@vvuk It looks to me like we could just pass --enable-debuginfo to the configure script on the release builders and that would work fine (or we could just turn that option on by default), though I'm not sure what that will do with pdb files (I suspect they will be packaged along with the libs if they are generated correctly).

If we could persuade rustc to emit elf debuginfo in a separate file then I'd definitely be in favor of making a separate debuginfo package. I don't know if LLVM/rustc can generate such files though.

It's probably worth making the simple change to the configure script and seeing how it affects both total tarball size and build times. If it's a big drag on build times then we might not want to turn it on by default when in the "dev" channel.

@brson
Copy link
Contributor

brson commented Oct 18, 2016

@cuviper
Copy link
Member

cuviper commented Oct 18, 2016

If we could persuade rustc to emit debuginfo in a separate file

The Linux way is usually a separate step afterward, something like strip -g -o foo.debug foo

@retep998
Copy link
Member

@cuviper I mean the config.toml that rustbuild reads settings from, which is the modern way to build rustc that doesn't involve configure or make.

@brson
Copy link
Contributor

brson commented Oct 18, 2016

FWIW I don't think we can get away from configure+make as an interface to the build system. It's a standard that unixes expect.

@alexcrichton
Copy link
Member

I believe @brson is correct here where we just need to pass --enable-debuginfo on the MSVC nightly builders. Unfortunately rustbuild (e.g. the Cargo.toml files) do not generate the MSVC distribution right now, so changing them wouldn't actually fix the problem here.

@brson I think we may prefer though to change buildbot config rather than our build script here, right?

I'd also prefer to only do this for MSVC right now as it'd be more of a PITA to run strip or other tools for Unixes.

@brson
Copy link
Contributor

brson commented Oct 18, 2016

@alexcrichton why only for windows? I'd prefer to keep feature parity between platforms.

The reason to change configure is that others packaging rust would get debuginfo as well. If that's the experience we want our users to have then doing it by default helps distros get it right.

Why would we need to run strip on the unixes? I was assuming we would just include the debuginfo inside the binaries for now, to get the feature out. If someday somebody teaches rustc to put the debuginfo beside the bins then we can turn that on.

@alexcrichton
Copy link
Member

Hm are we 100% sure that this doesn't affect code generation? Or otherwise no negative effects on Unix? I assumed that the story there was somewhat up in the air so we've avoided it in the past.

You can ignore my comment about strip.

@cuviper
Copy link
Member

cuviper commented Oct 18, 2016

GCC does have the option -gsplit-dwarf creating separate .dwo files, but not as many tools support it. A similar option for rustc could be nice, but I'd be careful about making it the default.

You need strip if you want the main rust download to stay small, and ship a separate debug component.

I think it does affect codegen though, sadly. I still do opt+debug for Fedora builds, because this is our policy, but I wish I could rely on it having no overhead, even if that meant worse debuginfo.

@luser
Copy link
Contributor

luser commented Oct 19, 2016

I don't think stripping or shipping separate debug info files is feasible for Rust without a bit of extra work. Since rustc is going to statically link the prebuilt object files from the rlibs, the debug info needs to be in the object files. The alternative would mean teaching rustc to know how to find and link a standalone debug info file. That seems like a sensible future direction, but I don't think it's a thing that exists right now.

@cuviper
Copy link
Member

cuviper commented Oct 19, 2016

The most obvious codegen change is that debuginfo explicitly forces frame pointers. It's controlled by set_frame_pointer_elimination, referencing #11906, which calls must_not_eliminate_frame_pointers to determine the debuginfo state.

@alexcrichton
Copy link
Member

I've sent a PR to enable at least line number information in releases, so we can evaluate the impact from that and see if it works for Gecko.

@luser
Copy link
Contributor

luser commented Oct 19, 2016

If that at least gets us function names and source line info that should be sufficient for crash reporting purposes. We'll likely eventually want full debug info for developers doing actual debugging, but maybe we can figure out something smarter before then.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Oct 19, 2016
This commit enables by default passing the `-C debuginfo=1` argument to the
compiler for the stable, beta, and nightly release channels. A new configure
option was also added, `--enable-debuginfo-lines`, to enable this behavior in
developer builds as well.

Closes rust-lang#36452
bors added a commit that referenced this issue Oct 21, 2016
Enable line number debuginfo in releases

This commit enables by default passing the `-C debuginfo=1` argument to the
compiler for the stable, beta, and nightly release channels. A new configure
option was also added, `--enable-debuginfo-lines`, to enable this behavior in
developer builds as well.

Closes #36452
@vvuk
Copy link
Contributor

vvuk commented Oct 21, 2016

#37280 isn't sufficient for Windows -- with an Oct 20 nightly build, source information is missing from any standard rust libs. Additionally, for Windows, we need more than just line numbers unfortunately. rustc always uses gnu-style mangling, so we need debug info for the symbols to make sense in the debugger (you get _ZN4test17run_tests_console17h07b8e649031d5e08E when you want test::run_tests_console::07b8e...).

@alexcrichton
Copy link
Member

@vvuk were you using this nightly?

rustc 1.14.0-nightly (f09420685 2016-10-20)

If so that doesn't include e470827, the merge commit with #37280

@vvuk
Copy link
Contributor

vvuk commented Oct 22, 2016

Indeed! Okay, will check with the next one and report again.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 11, 2017
In rust-lang#37280 we enabled line number debugging information in release artifacts,
primarily to close out rust-lang#36452 where debugging information was critical for MSVC
builds of Rust to be useful in production. This commit, however, apparently had
some unfortunate side effects.

Namely it was noticed in rust-lang#37477 that if `RUST_BACKTRACE=1` was set then any
compiler error would take a very long time for the compiler to exit. The cause
of the problem here was somewhat deep:

* For all compiler errors, the compiler will `panic!` with a known value. This
  tears down the main compiler thread and allows cleaning up all the various
  resources. By default, however, this panic output is suppressed for "normal"
  compiler errors.
* When `RUST_BACKTRACE=1` was set this caused every compiler error to generate a
  backtrace.
* The libbacktrace library hits a pathological case where it spends a very long
  time in its custom allocation function, `backtrace_alloc`, because the
  compiler has so much debugging information. More information about this can be
  found in rust-lang#29293 with a summary at the end of rust-lang#37477.

To solve this problem this commit simply removes debuginfo from the compiler but
not from the standard library. This should allow us to keep rust-lang#36452 closed while
also closing rust-lang#37477. I've measured the difference to be orders of magnitude
faster than it was before, so we should see a much quicker time-to-exit after a
compile error when `RUST_BACKTRACE=1` is set.

Closes rust-lang#37477
Closes rust-lang#37571
bors added a commit that referenced this issue Jan 11, 2017
rustbuild: Don't enable debuginfo in rustc

In #37280 we enabled line number debugging information in release artifacts,
primarily to close out #36452 where debugging information was critical for MSVC
builds of Rust to be useful in production. This commit, however, apparently had
some unfortunate side effects.

Namely it was noticed in #37477 that if `RUST_BACKTRACE=1` was set then any
compiler error would take a very long time for the compiler to exit. The cause
of the problem here was somewhat deep:

* For all compiler errors, the compiler will `panic!` with a known value. This
  tears down the main compiler thread and allows cleaning up all the various
  resources. By default, however, this panic output is suppressed for "normal"
  compiler errors.
* When `RUST_BACKTRACE=1` was set this caused every compiler error to generate a
  backtrace.
* The libbacktrace library hits a pathological case where it spends a very long
  time in its custom allocation function, `backtrace_alloc`, because the
  compiler has so much debugging information. More information about this can be
  found in #29293 with a summary at the end of #37477.

To solve this problem this commit simply removes debuginfo from the compiler but
not from the standard library. This should allow us to keep #36452 closed while
also closing #37477. I've measured the difference to be orders of magnitude
faster than it was before, so we should see a much quicker time-to-exit after a
compile error when `RUST_BACKTRACE=1` is set.

Closes #37477
Closes #37571
brson pushed a commit to brson/rust that referenced this issue Jan 17, 2017
In rust-lang#37280 we enabled line number debugging information in release artifacts,
primarily to close out rust-lang#36452 where debugging information was critical for MSVC
builds of Rust to be useful in production. This commit, however, apparently had
some unfortunate side effects.

Namely it was noticed in rust-lang#37477 that if `RUST_BACKTRACE=1` was set then any
compiler error would take a very long time for the compiler to exit. The cause
of the problem here was somewhat deep:

* For all compiler errors, the compiler will `panic!` with a known value. This
  tears down the main compiler thread and allows cleaning up all the various
  resources. By default, however, this panic output is suppressed for "normal"
  compiler errors.
* When `RUST_BACKTRACE=1` was set this caused every compiler error to generate a
  backtrace.
* The libbacktrace library hits a pathological case where it spends a very long
  time in its custom allocation function, `backtrace_alloc`, because the
  compiler has so much debugging information. More information about this can be
  found in rust-lang#29293 with a summary at the end of rust-lang#37477.

To solve this problem this commit simply removes debuginfo from the compiler but
not from the standard library. This should allow us to keep rust-lang#36452 closed while
also closing rust-lang#37477. I've measured the difference to be orders of magnitude
faster than it was before, so we should see a much quicker time-to-exit after a
compile error when `RUST_BACKTRACE=1` is set.

Closes rust-lang#37477
Closes rust-lang#37571
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

No branches or pull requests

7 participants