cg_llvm: split dwarf support #77117

davidtwco · 2020-09-23T17:08:06Z

This PR adds initial support for Split DWARF to rustc, based on the implementation in Clang.

Current Status

This PR currently has functioning split-dwarf, running rustc with -Zsplit-dwarf=split when compiling a binary will produce a dwp alongside the binary, which contains the linked dwarf objects.

$ rustc -Cdebuginfo=2 -Zsplit-dwarf=split -C save-temps ./foo.rs
$ ls foo*
foo
foo.belfx9afw9cmv8.rcgu.dwo
foo.belfx9afw9cmv8.rcgu.o
foo.foo.7rcbfp3g-cgu.0.rcgu.dwo
foo.foo.7rcbfp3g-cgu.0.rcgu.o
foo.foo.7rcbfp3g-cgu.1.rcgu.dwo
foo.foo.7rcbfp3g-cgu.1.rcgu.o
foo.foo.7rcbfp3g-cgu.2.rcgu.dwo
foo.foo.7rcbfp3g-cgu.2.rcgu.o
foo.foo.7rcbfp3g-cgu.3.rcgu.dwo
foo.foo.7rcbfp3g-cgu.3.rcgu.o
foo.foo.7rcbfp3g-cgu.4.rcgu.dwo
foo.foo.7rcbfp3g-cgu.4.rcgu.o
foo.foo.7rcbfp3g-cgu.5.rcgu.dwo
foo.foo.7rcbfp3g-cgu.5.rcgu.o
foo.foo.7rcbfp3g-cgu.6.rcgu.dwo
foo.foo.7rcbfp3g-cgu.6.rcgu.o
foo.foo.7rcbfp3g-cgu.7.rcgu.dwo
foo.foo.7rcbfp3g-cgu.7.rcgu.o
foo.dwp
foo.rs
$ readelf -wi foo.foo.7rcbfp3g-cgu.0.rcgu.o
# ...
  Compilation Unit @ offset 0x90:
   Length:        0x2c (32-bit)
   Version:       4
   Abbrev Offset: 0x5b
   Pointer Size:  8
 <0><9b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <9c>   DW_AT_stmt_list   : 0xe8
    <a0>   DW_AT_comp_dir    : (indirect string, offset: 0x13b): /home/david/Projects/rust/rust0
    <a4>   DW_AT_GNU_dwo_name: (indirect string, offset: 0x15b): foo.foo.7rcbfp3g-cgu.0.rcgu.dwo
    <a8>   DW_AT_GNU_dwo_id  : 0x357472a2b032d7b9
    <b0>   DW_AT_low_pc      : 0x0
    <b8>   DW_AT_ranges      : 0x40
    <bc>   DW_AT_GNU_addr_base: 0x0
# ...

To-Do

I've opened this PR as a draft to get feedback and work out how we'd expect rustc to work when Split DWARF is requested. It might be easier to read the PR commit-by-commit.

Add error when Split DWARF is requested on platforms where it doesn't make sense.
Determine whether or not there should be a single dwo output from rustc, or one per codegen-unit as exists currently.
Add tests.
Fix single mode - currently single mode doesn't change the invocation of addPassesToEmitFile, which is correct, but it also needs to change the split dwarf path provided to createCompileUnit and createTargetMachine so that it's just the final binary (currently it is still a non-existent dwo file).

r? @nagisa
cc @michaelwoerister @eddyb @alexcrichton @rust-lang/wg-incr-comp

eddyb · 2020-09-24T04:11:17Z

Determine whether or not there should be a single dwo output from rustc, or one per codegen-unit as exists currently.

What happens when you compile a Cargo package in debug mode, with some dependencies? Debug mode should default to -Cdebuginfo=2 but will also enable -Cincremental. I expect you'll end up with hundreds of dwo files.

Would be nice if it were possible to combine the files cheaply, as managing one dwo file seems easier (for distros, or even rustup, if we e.g. want to add a rustc-debuginfo component that has at least -Cdebuginfo=1 for better backtraces).

Other than this, this PR is great! I remember how much better using split DWARF was when compiling LLVM in debug mode.

It also reminds me that I want to eventually look at perhaps deduplicating some of the DWARF definitions (which get duplicated through instantiation in separate CGUs), and being able to operate on the dwo separately from the rest of the binary should help.

nagisa

https://gcc.gnu.org/wiki/DebugFission and https://gcc.gnu.org/wiki/DebugFissionDWP have some discussion on merging the dwo files into a single file.

So far what I'm seeing looks good to me!

compiler/rustc_session/src/options.rs

nagisa · 2020-09-26T13:49:32Z

compiler/rustc_codegen_ssa/src/back/write.rs

-    }
+/// Configuration passed to the function returned by the `target_machine_factory`.
+pub struct TargetMachineFactoryConfig<'a> {
+    /// Split DWARF is enabled in LLVM by checking that `TM.MCOptions.SplitDwarfFile` isn't empty,


Hmm… this probably should not be LLVM-specific, but I don’t yet know how the alternative backends could handle this being set (error out? transparently ignore?)

I've added to this commit to note that non-LLVM backends can ignore this if they don't need it for their own Split DWARF support.

davidtwco · 2020-09-26T14:23:00Z

gcc.gnu.org/wiki/DebugFission and gcc.gnu.org/wiki/DebugFissionDWP have some discussion on merging the dwo files into a single file.

As far as I can tell (having just had a quick look at dwp's source), dwp doesn't support DWARF 5-flavoured Split DWARF, only pre-standard GNU-flavoured Split DWARF. According to #75890, LLVM enables DWARF 5 on some backends by default and it will emit DWARF 5-flavoured Split DWARF by default in those cases, so I'm not sure that we could rely on dwp always working.

davidtwco · 2020-10-14T18:20:52Z

I've revisited this today and made some good progress - I've discovered that llvm-dwp exists, and so I've just started using that. First, I change bootstrap so that, like rust-lld, we ship rust-llvm-dwp in the sysroot. Next, I use rust-llvm-dwp after linking objects to link together the dwo files into a dwp file. And that's basically it.

There's still some work to be done on this PR but this is a reasonable step forward.

cc @Mark-Simulacrum for some bootstrap changes (see e83d81b at time of writing)

davidtwco · 2020-11-08T18:05:37Z

Pushed another update - added a compare mode that makes debuginfo tests also run with Split DWARF - which identified an issue that I've resolved. That might not be the long-term approach we want for testing Split DWARF, but it worked for now and I'm open to ideas.

Only thing I identified initially that I've yet to do is add an error when this is used on platforms where it wouldn't make sense, but I'm not sure if that's actually something we want - would appreciate feedback on that point.

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

src/tools/compiletest/src/runtest.rs

nagisa

@bors r+

src/bootstrap/test.rs

nagisa · 2020-11-15T00:16:47Z

@bors rollup=iffy

davidtwco · 2020-11-15T01:13:07Z

Should the flag for this be moved to -C before this lands?

I'd also like @Mark-Simulacrum to review the changes to bootstrap. I'm not sure if there's any downsides to adding a new tool to the sysroot.

With that said, Split DWARF in this PR isn't enabled by default or accessible outside of nightly - so we can still make changes (except for the dwp tool in the sysroot, I suppose).

nagisa · 2020-11-15T01:17:57Z

Nah, move from -Z to -C flag should happen as part of a separate stabilization effort.

With that said, Split DWARF in this PR is enabled by default or accessible outside of nightly

Did you mean not enabled by default?

Mark-Simulacrum · 2020-11-15T01:56:43Z

I think the main downside to adding the extra tool is that it increases our dependence on LLVM a bit further - I don't think it is a sufficient concern to block this, though. When we get closer to stabilization we can consider this and perhaps look at alternatives (e.g. a rust alternative to dwp). Otherwise the bootstrap changes seem fine.

This commit makes minor changes to the cranelift backend so that it can build given changes in cg_ssa for Split DWARF. Signed-off-by: David Wood <david@davidtw.co>

This commit adds a run-make-fulldeps test which checks that a DWARF package file is emitted. Signed-off-by: David Wood <david@davidtw.co>

This commit adds a Split DWARF compare mode to compiletest so that debuginfo tests are also tested using Split DWARF in split mode (and manually in single mode). Signed-off-by: David Wood <david@davidtw.co>

llvm-dwp concatenates `DW_AT_comp_dir` with `DW_AT_GNU_dwo_name` (only when `DW_AT_comp_dir` exists), which can result in it failing to find the DWARF object files. In earlier testing, `DW_AT_comp_dir` wasn't present in the final object and the current directory was the output directory. When running tests through compiletest, the working directory of the compilation is different from output directory and that resulted in `DW_AT_comp_dir` being in the object file (and set to the current working directory, rather than the output directory), and `DW_AT_GNU_dwo_name` being set to the full path (rather than just the filename), so llvm-dwp was failing. This commit changes the compilation directory provided to LLVM to match the output directory, where DWARF objects are output; and ensures that only the filename is used for `DW_AT_GNU_dwo_name`. Signed-off-by: David Wood <david@davidtw.co>

davidtwco · 2020-12-16T11:17:43Z

@bors r=nagisa

bors · 2020-12-16T11:17:44Z

📌 Commit ee073b5 has been approved by nagisa

bors · 2020-12-16T12:51:49Z

⌛ Testing commit ee073b5 with merge 2ba7ca2...

bors · 2020-12-16T15:39:28Z

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 2ba7ca2 to master...

This commit includes the `llvm-dwp` tool in the CI LLVM (which rustc developers can download instead of building LLVM locally) - `llvm-dwp` is required by Split DWARF which landed in PR rust-lang#77117. Signed-off-by: David Wood <david@davidtw.co>

…llvm, r=Mark-Simulacrum bootstrap: include llvm-dwp in CI LLVM Fixes rust-lang#80086. This PR includes the `llvm-dwp` tool in the CI LLVM (which rustc developers can download instead of building LLVM locally) - `llvm-dwp` is required by Split DWARF which landed in PR rust-lang#77117. r? `@Mark-Simulacrum`

philipc · 2020-12-31T07:13:56Z

@davidtwco Is there a reason this requires -C save-temps? I thought it should automatically save the files if needed. I get errors such as this without it:

$ RUSTFLAGS="-Z split-dwarf=split" cargo +nightly test
...
error: linking dwarf objects with `rust-llvm-dwp` failed: exit code: 1
  |
  = note: "rust-llvm-dwp" "-e" "/home/philipc/code/rust/backtrace-rs/target/debug/build/miniz_oxide-f8a4dfce48798ca5/build_script_build-f8a4dfce48798ca5" "-o" "/home/philipc/code/rust/backtrace-rs/target/debug/build/miniz_oxide-f8a4dfce48798ca5/build_script_build-f8a4dfce48798ca5.dwp"
  = note: 
  = note: error: No such file or directory

davidtwco · 2020-12-31T12:44:31Z

@philipc -C save-temps shouldn't be required, when llvm-dwp is invoked then it should have all of the .o and .dwo files still available. There could be some strange behaviour when this is used with dependencies or build scripts that I didn't discover. Alternatively, there could be some scenario whereby DW_AT_comp_dir concatenated with DW_AT_GNU_dwo_name isn't a valid path to the dwo file (I ran into this in some circumstances while working on this, but thought that I had resolved the issue). Is it possible for you to share the commands you ran to produce this failure?

philipc · 2020-12-31T13:04:08Z

The command is as given above (RUSTFLAGS="-Z split-dwarf=split" cargo +nightly test) and I am running this in a clone of the backtrace-rs crate. The nightly I was using was the first one that had this PR in it, but I could try a newer nightly if you think that might help. (By the way, that backtrace-rs test will also fail without rust-lang/backtrace-rs#401.)

Alternatively, there could be some scenario whereby DW_AT_comp_dir concatenated with DW_AT_GNU_dwo_name isn't a valid path to the dwo file

I wouldn't expect a problem like that to be fixed by using -C save-temps though?

davidtwco · 2020-12-31T15:23:00Z

I wouldn't expect a problem like that to be fixed by using -C save-temps though?

I wouldn't either. I'll take a look at this and see if I can work out what is happening as soon as I can.

rust-highfive assigned nagisa Sep 23, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 23, 2020

nagisa reviewed Sep 26, 2020

View reviewed changes

davidtwco added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2020

This comment has been minimized.

Sign in to view

davidtwco force-pushed the issue-34651-split-dwarf branch 3 times, most recently from 0d82e49 to 39cb612 Compare October 14, 2020 18:17

davidtwco requested a review from nagisa October 14, 2020 18:20

This comment has been minimized.

Sign in to view

davidtwco force-pushed the issue-34651-split-dwarf branch from 39cb612 to ff83683 Compare November 3, 2020 17:12

davidtwco added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 3, 2020

davidtwco force-pushed the issue-34651-split-dwarf branch from ff83683 to c367af4 Compare November 8, 2020 18:03

davidtwco marked this pull request as ready for review November 8, 2020 18:05

davidtwco commented Nov 8, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs Show resolved Hide resolved

bjorn3 reviewed Nov 8, 2020

View reviewed changes

src/tools/compiletest/src/runtest.rs Outdated Show resolved Hide resolved

davidtwco force-pushed the issue-34651-split-dwarf branch from c367af4 to 8ad0de9 Compare November 8, 2020 18:20

nagisa approved these changes Nov 15, 2020

View reviewed changes

src/bootstrap/test.rs Show resolved Hide resolved

nagisa added relnotes Marks issues that should be documented in the release notes of the next release. and removed relnotes Marks issues that should be documented in the release notes of the next release. labels Nov 15, 2020

davidtwco added 4 commits December 16, 2020 10:33

cg_clif: fix build with split dwarf

6c4350d

This commit makes minor changes to the cranelift backend so that it can build given changes in cg_ssa for Split DWARF. Signed-off-by: David Wood <david@davidtw.co>

tests: add run-make-fulldeps split-dwarf test

2b22670

This commit adds a run-make-fulldeps test which checks that a DWARF package file is emitted. Signed-off-by: David Wood <david@davidtw.co>

compiletest: add split dwarf compare mode

99ad915

This commit adds a Split DWARF compare mode to compiletest so that debuginfo tests are also tested using Split DWARF in split mode (and manually in single mode). Signed-off-by: David Wood <david@davidtw.co>

davidtwco force-pushed the issue-34651-split-dwarf branch from e6bb400 to ee073b5 Compare December 16, 2020 10:36

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 16, 2020

bors merged commit 2ba7ca2 into rust-lang:master Dec 16, 2020

rustbot added this to the 1.50.0 milestone Dec 16, 2020

This was referenced Dec 16, 2020

[WIP] Content hash support. (See also cargo changes) #75594

Closed

rustc: Stabilize -Zrun-dsymutil as -Csplit-debuginfo #79570

Merged

davidtwco deleted the issue-34651-split-dwarf branch December 16, 2020 16:12

jyn514 mentioned this pull request Dec 16, 2020

x.py build fails with download-ci-llvm = true #80086

Closed

davidtwco mentioned this pull request Dec 16, 2020

bootstrap: include llvm-dwp in CI LLVM #80087

Merged

alexcrichton mentioned this pull request Jan 6, 2021

debuginfo: Add support for split-debuginfo on platforms that allow it #34651

Closed

davidtwco mentioned this pull request Jan 14, 2021

split dwarf doesn't work with crate dependencies #81024

Closed

Alexendoo mentioned this pull request Feb 12, 2021

CentOS 7's rpmbuild can't handle debuginfo generated by Rust 1.50.0 #82006

Closed

davidtwco mentioned this pull request Oct 12, 2021

cg: split dwarf for crate dependencies #89819

Merged

davidtwco mentioned this pull request Jun 13, 2022

session: stabilize split debuginfo on linux #98051

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cg_llvm: split dwarf support #77117

cg_llvm: split dwarf support #77117

davidtwco commented Sep 23, 2020 •

edited

Loading

eddyb commented Sep 24, 2020 •

edited

Loading

nagisa left a comment

nagisa Sep 26, 2020

davidtwco Nov 8, 2020

davidtwco commented Sep 26, 2020 •

edited

Loading

This comment has been minimized.

davidtwco commented Oct 14, 2020 •

edited

Loading

This comment has been minimized.

davidtwco commented Nov 8, 2020 •

edited

Loading

nagisa left a comment

nagisa commented Nov 15, 2020

davidtwco commented Nov 15, 2020 •

edited

Loading

nagisa commented Nov 15, 2020 •

edited

Loading

Mark-Simulacrum commented Nov 15, 2020

davidtwco commented Dec 16, 2020

bors commented Dec 16, 2020

bors commented Dec 16, 2020

bors commented Dec 16, 2020

philipc commented Dec 31, 2020

davidtwco commented Dec 31, 2020

philipc commented Dec 31, 2020

davidtwco commented Dec 31, 2020

cg_llvm: split dwarf support #77117

cg_llvm: split dwarf support #77117

Conversation

davidtwco commented Sep 23, 2020 • edited Loading

Current Status

To-Do

eddyb commented Sep 24, 2020 • edited Loading

nagisa left a comment

Choose a reason for hiding this comment

nagisa Sep 26, 2020

Choose a reason for hiding this comment

davidtwco Nov 8, 2020

Choose a reason for hiding this comment

davidtwco commented Sep 26, 2020 • edited Loading

This comment has been minimized.

davidtwco commented Oct 14, 2020 • edited Loading

This comment has been minimized.

davidtwco commented Nov 8, 2020 • edited Loading

nagisa left a comment

Choose a reason for hiding this comment

nagisa commented Nov 15, 2020

davidtwco commented Nov 15, 2020 • edited Loading

nagisa commented Nov 15, 2020 • edited Loading

Mark-Simulacrum commented Nov 15, 2020

davidtwco commented Dec 16, 2020

bors commented Dec 16, 2020

bors commented Dec 16, 2020

bors commented Dec 16, 2020

philipc commented Dec 31, 2020

davidtwco commented Dec 31, 2020

philipc commented Dec 31, 2020

davidtwco commented Dec 31, 2020

davidtwco commented Sep 23, 2020 •

edited

Loading

eddyb commented Sep 24, 2020 •

edited

Loading

davidtwco commented Sep 26, 2020 •

edited

Loading

davidtwco commented Oct 14, 2020 •

edited

Loading

davidtwco commented Nov 8, 2020 •

edited

Loading

davidtwco commented Nov 15, 2020 •

edited

Loading

nagisa commented Nov 15, 2020 •

edited

Loading