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

cg: split dwarf for crate dependencies #89819

Merged

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Oct 12, 2021

Fixes #81024.

  • In rustc: Stabilize -Zrun-dsymutil as -Csplit-debuginfo #79570, -Z split-dwarf-kind={none,single,split} was replaced by -C split-debuginfo={off,packed,unpacked}. -C split-debuginfo's packed and unpacked aren't exact parallels to single and split, respectively.

    On Unix, -C split-debuginfo=packed will put debuginfo in object files and package debuginfo into a DWARF package file (.dwp) and -C split-debuginfo=unpacked will put debuginfo in dwarf object files and won't package it.

    In the initial implementation of Split DWARF, split mode wrote sections which did not require relocation into a DWARF object (.dwo) file which was ignored by the linker and then packaged those DWARF objects into DWARF packages (.dwp). In single mode, sections which did not require relocation were written into object files but ignored by the linker and were not packaged. However, both split and single modes could be packaged or not, the primary difference in behaviour was where the debuginfo sections that did not require link-time relocation were written (in a DWARF object or the object file).

    In the first commit of this PR, I re-introduce a -Z split-dwarf-kind flag, which can be used to pick between split and single modes when -C split-debuginfo is used to enable Split DWARF (either packed or unpacked).

  • Split DWARF packaging requires all of the object files to exist, including those in dependencies. Therefore, the second commit of this PR makes rustc keep all objects or dwarf objects for unpacked mode and if the crate is a dependency in packed mode (determined by heuristic: if no linking is taking place), then objects or dwarf objects are kept. Objects are kept if -Z split-dwarf-kind is SplitDwarfKind::Single, and dwarf objects if SplitDwarfKind::Split.

    There are other approaches that could be taken to supporting packed Split DWARF with crate dependencies but this seemed like the least complicated and was contained to only rustc. Other potential approaches are described in split dwarf doesn't work with crate dependencies #81024 (comment), I'm happy to change the approach I've taken here if it isn't what we're looking for. See cg: split dwarf for crate dependencies #89819 (comment) for the current approach.

  • There's still a dependency on llvm-dwp after this change, which we probably want to move away from but that seems out-of-scope for this PR. Ideally, Split DWARF (in packed or unpacked modes) will be usable on nightly after this lands. If there aren't any bugs reported then it's possible we could allow Split DWARF to be used on stable after this change, it depends whether or not switching away from llvm-dwp later would break any guarantees, or whether we'd want to change how we handle this cross-crate case in future. See cg: split dwarf for crate dependencies #89819 (comment).

r? @nagisa
cc @alexcrichton

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2021
@alexcrichton
Copy link
Member

I think I'm probably missing something, but could you reiterate why a new compiler option is needed? Dealing with the matrix of -Csplit-debuginfo and -Zsplit-dwarf-kind seems hairy and difficult to manage, and the discussion for -Csplit-debuginfo was in theory all about not requiring a separate option for split-dwarf on Unix.

@davidtwco
Copy link
Member Author

I think I'm probably missing something, but could you reiterate why a new compiler option is needed? Dealing with the matrix of -Csplit-debuginfo and -Zsplit-dwarf-kind seems hairy and difficult to manage, and the discussion for -Csplit-debuginfo was in theory all about not requiring a separate option for split-dwarf on Unix.

C++ compilers provide both split and single modes for their split dwarf implementations (and don't provide packed/unpacked option equivalents as it doesn't make as much sense for the compiler to do that for those languages), but I'm not sure if there are distinct use cases that each of these variants cater to, or particular reasons why you'd want one over the other. I don't think that it's too complicated to maintain both -Csplit-debuginfo and -Csplit-dwarf-kind though, -Csplit-debuginfo is still the only option you need for enabling and disabling split debuginfo regardless of platform, -Csplit-dwarf-kind just lets you tweak the specifics of how we emit that on Unix.

All of that said, it isn't strictly necessary, and it's unlikely to be an option that most users of split debuginfo will need. I don't think it makes sense or is intuitive for packed/unpacked to affect where the split debuginfo goes for split dwarf though (as it does now), that option feels like it should only change the packed-ness of the debuginfo, so we could alternatively just use split mode in both packed/unpacked.

@alexcrichton
Copy link
Member

Hm so I was under the impression that the 4 options for dwarf were:

  • Nothing, no debug info
  • Debuginfo in *.o files themselves, then also linked into final executable
  • Debuginfo split out into *.dwo files and nothing else
  • Debuginfo "linked" into *.dwp from all the *.dwo files

That corresponds to -Cdebuginfo=0, -Csplit-debuginfo={off,unpacked,packed}. Does split-dwarf have more options we need to encompass though? (sorry I feel like I'm missing something obvious you've probably already explained but on rereading I keep getting confused...)

@davidtwco
Copy link
Member Author

davidtwco commented Oct 12, 2021

Hm so I was under the impression that the 4 options for dwarf were:

  • Nothing, no debug info
  • Debuginfo in *.o files themselves, then also linked into final executable
  • Debuginfo split out into *.dwo files and nothing else
  • Debuginfo "linked" into *.dwp from all the *.dwo files

That corresponds to -Cdebuginfo=0, -Csplit-debuginfo={off,unpacked,packed}. Does split-dwarf have more options we need to encompass though? (sorry I feel like I'm missing something obvious you've probably already explained but on rereading I keep getting confused...)

As I understand it, it would be:

  • Nothing, no debuginfo.
    • -Cdebuginfo=0 -Csplit-debuginfo=off
  • Debuginfo in object files, linked into the final executable.
    • -Cdebuginfo=2 -Csplit-debuginfo=off
  • Debuginfo in object files, but in such a way that the linker skips it, not in the final executable - single mode
    • -Cdebuginfo=2 -Csplit-debuginfo=unpacked -Zsplit-dwarf-kind=single
  • Debuginfo in object files, but in such a way that the linker skips it, not in the final executable, debuginfo from object files linked into dwarf package - single mode, with packaging
    • -Cdebuginfo=2 -Csplit-debuginfo=packed -Zsplit-dwarf-kind=single
  • Debuginfo in dwarf object files, not in the final executable - split mode
    • -Cdebuginfo=2 -Csplit-debuginfo=unpacked (uses the default of -Zsplit-dwarf-kind=split)
  • Debuginfo in dwarf object files, not in the final executable, debuginfo from dwarf object files linked into dwarf package - split mode, with packaging
    • -Cdebuginfo=2 -Csplit-debuginfo=packed (uses the default of -Zsplit-dwarf-kind=split)

I've described this in an earlier comment which phrases it differently, that might help. I don't know what the specific use-cases for single vs split mode are for split dwarf, I just noticed that these were the variants provided by C++ compilers and so implemented both in rustc.

@alexcrichton
Copy link
Member

Oof that's a lot of modes. Well in that case seems like we do need a flag or more options!

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021
@davidtwco
Copy link
Member Author

Noticed there's still a bug in this somewhere, trying to build rustc with packed split dwarf will fail with an error about a duplicate DWO ID, should work fine for smaller crates, haven't been able to produce a small reproduction but I didn't spend too much time on it.

@nagisa
Copy link
Member

nagisa commented Oct 17, 2021

This seems reasonable to me, though I would like to see a description of a use-case before we stabilize the option. r=me with or without the nit about exhaustive matches and whatever other bugs that you see fixed.

@davidtwco davidtwco force-pushed the issue-81024-multiple-crates-multiple-dwarves branch from 260b924 to 2935c95 Compare October 22, 2021 10:54
@davidtwco
Copy link
Member Author

This seems reasonable to me, though I would like to see a description of a use-case before we stabilize the option. r=me with or without the nit about exhaustive matches and whatever other bugs that you see fixed.

Looks like the motivation is primarily to retain compatability with tools that only understand object files (source):

To summarise:

  • It shares most of the benefits with the .dwo solution.
  • Unlike .dwo, there is no need to split the file and it is friendlier to other tools (ccache, distcc, etc)
  • But unlike .dwo a distributed build system has to copy larger .o files.

@rust-log-analyzer

This comment has been minimized.

@davidtwco davidtwco force-pushed the issue-81024-multiple-crates-multiple-dwarves branch from 2935c95 to f05a938 Compare October 22, 2021 12:36
@davidtwco
Copy link
Member Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Oct 22, 2021

📌 Commit f05a938581b609c0636474b2c8f8e6f0d89d42a4 has been approved by nagisa

@bors 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 Oct 22, 2021
@bors

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2021
@davidtwco 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 29, 2021
@matthiaskrgr

This comment has been minimized.

@lqd
Copy link
Member

lqd commented Jan 5, 2022

Looks like an apple runner issue ? let's @bors retry

@bors 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 Jan 5, 2022
@bors

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 5, 2022
In rust-lang#79570, `-Z split-dwarf-kind={none,single,split}` was replaced by `-C
split-debuginfo={off,packed,unpacked}`. `-C split-debuginfo`'s packed
and unpacked aren't exact parallels to single and split, respectively.

On Unix, `-C split-debuginfo=packed` will put debuginfo into object
files and package debuginfo into a DWARF package file (`.dwp`) and
`-C split-debuginfo=unpacked` will put debuginfo into dwarf object files
and won't package it.

In the initial implementation of Split DWARF, split mode wrote sections
which did not require relocation into a DWARF object (`.dwo`) file which
was ignored by the linker and then packaged those DWARF objects into
DWARF packages (`.dwp`). In single mode, sections which did not require
relocation were written into object files but ignored by the linker and
were not packaged. However, both split and single modes could be
packaged or not, the primary difference in behaviour was where the
debuginfo sections that did not require link-time relocation were
written (in a DWARF object or the object file).

This commit re-introduces a `-Z split-dwarf-kind` flag, which can be
used to pick between split and single modes when `-C split-debuginfo` is
used to enable Split DWARF (either packed or unpacked).

Signed-off-by: David Wood <david.wood@huawei.com>
`thorin` is a Rust implementation of a DWARF packaging utility that
supports reading DWARF objects from archive files (i.e. rlibs) and
therefore is better suited for integration into rustc.

Signed-off-by: David Wood <david.wood@huawei.com>
@davidtwco davidtwco force-pushed the issue-81024-multiple-crates-multiple-dwarves branch from f88572b to 7ecdc89 Compare January 6, 2022 09:33
@davidtwco
Copy link
Member Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Jan 6, 2022

📌 Commit 7ecdc89 has been approved by nagisa

@bors 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 Jan 6, 2022
@davidtwco
Copy link
Member Author

@bors rollup=never

@bors
Copy link
Contributor

bors commented Jan 6, 2022

⌛ Testing commit 7ecdc89 with merge a77cc64...

@bors
Copy link
Contributor

bors commented Jan 6, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 6, 2022
@bors bors merged commit a77cc64 into rust-lang:master Jan 6, 2022
@rustbot rustbot added this to the 1.59.0 milestone Jan 6, 2022
@davidtwco davidtwco deleted the issue-81024-multiple-crates-multiple-dwarves branch January 6, 2022 15:33
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a77cc64): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

split dwarf doesn't work with crate dependencies