Make cargo aware of dwp files. by khuey · Pull Request #11572 · rust-lang/cargo

khuey · 2023-01-13T01:53:03Z

When using -Csplit-debuginfo=packed on Linux, rustc will produce a dwp file. Unlike the dwo files, whose paths are embedded into the binary, there's no information in the binary to help a debugger locate a dwp file. By convention, the dwp file for <EXE> is given the name <EXE>.dwp and placed next to <EXE>.

When cargo hardlinks the executable file rustc put in target/debug/deps into target/debug, it also needs to hardlink the dwp file along with it. Failing to do this prevents the debugger from finding the dwp file when the binary is executed from target/debug, as there's no way for the debugger to know to look in the deps subdirectory.

The split_debuginfo option is passed down into file_types to make this possible. For cargo clean manual handling is added to match the other split_debuginfo files. bin_link_for_target also passes in None because it won't care about the dwp file.

rustbot · 2023-01-13T01:53:08Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information.

khuey · 2023-01-13T01:54:00Z

cc @davidtwco

khuey · 2023-01-13T01:56:57Z

CI seems broken.

weihanglo · 2023-01-13T07:58:26Z

Thank you! Looks like a duplicate of #11384? I would probably ping them to see if they want to proceed. There is also a discussion regarding artifact name conflict on Windows which is worth considering.

weihanglo · 2023-01-13T07:59:58Z

r? @weihanglo

khuey · 2023-01-13T16:10:03Z

Thanks for the pointer to the other PR. I hadn't seen that.

The mingw issue they ran across is actually a rustc bug: rust-lang/rust#106811. .dwp should be added to the filename; it should not replace the existing extension. On Linux this isn't noticed on regular binaries but it is visible if you build a dylib. I've fixed the cargo side of that in the updated commit here.

I've also added a test along the lines you suggested in the other PR. It would be good to test the dylib behavior too but I don't know how cargo handles varying rustc versions for tests. Any test would need a version of rustc with the rustc PR above merged to pass.

I think the main difference between the PRs at this point is whether the split_debuginfo value is passed into file_types (and thus whether the DWP entry is emitted only when relevant or all the time).

khuey · 2023-01-13T16:14:01Z

And the value of should_replace_hyphens has to match the value the binary itself had. Setting it to false unconditionally as in the other PR is wrong.

ehuss · 2023-01-13T16:37:40Z

I think the main difference between the PRs at this point is whether the split_debuginfo value is passed into file_types (and thus whether the DWP entry is emitted only when relevant or all the time).

So far cargo has generated the FileType list with all potential files that rustc might generate, and doesn't necessarily differentiate based on anything like profile settings. It is generally safe to list extra files since cargo will ignore them if rustc didn't generate them. I can't offhand think of a particular problem of differentiating, but by adding it here for just dwp that makes the code inconsistent with other platforms.

khuey · 2023-01-14T00:14:57Z

Alright, done.

When using -Csplit-debuginfo=packed on Linux, rustc will produce a dwp file. Unlike the dwo files, whose paths are embedded into the binary, there's no information in the binary to help a debugger locate a dwp file. By convention, the dwp file for <EXE> is given the name <EXE>.dwp and placed next to <EXE>. When cargo hardlinks the executable file rustc put in target/debug/deps into target/debug, it also needs to hardlink the dwp file along with it. Failing to do this prevents the debugger from finding the dwp file when the binary is executed from target/debug, as there's no way for the debugger to know to look in the deps subdirectory.

khuey · 2023-01-15T07:58:28Z

Yay the tests finally pass.

src/cargo/core/compiler/build_context/target_info.rs

khuey · 2023-01-16T02:01:11Z

Also even once you are satisfied with this you probably shouldn't merge it before rust-lang/rust#106811 is merged.

weihanglo · 2023-01-16T16:12:10Z

Also even once you are satisfied with this you probably shouldn't merge it before rust-lang/rust#106811 is merged.

Fair enough. Let's label it as blocked until that PR gets merged.

Gankra · 2023-01-16T16:39:26Z

Oh, thanks so much for picking this up and doing it right!

khuey · 2023-01-27T17:58:20Z

The rustc PRs this depends on have been merged.

weihanglo

Looks good. Thanks!

weihanglo · 2023-01-30T12:00:13Z

@bors r+

bors · 2023-01-30T12:00:15Z

📌 Commit a393267 has been approved by weihanglo

It is now in the queue for this repository.

bors · 2023-01-30T12:00:22Z

⌛ Testing commit a393267 with merge 99b1369...

bors · 2023-01-30T12:49:26Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 99b1369 to master...

18 commits in 3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c..e84a7928d93a31f284b497c214a2ece69b4d7719 2023-01-24 15:48:15 +0000 to 2023-01-31 22:18:09 +0000 - chore: Add autolabel for `Command-*` labels (rust-lang/cargo#11664) - Update cross test instructions for aarch64-apple-darwin (rust-lang/cargo#11663) - Make cargo install report needed features (rust-lang/cargo#11647) - docs(contrib): Remove out-of-date process step (rust-lang/cargo#11662) - Do not error for `auth-required: true` without `-Z sparse-registry` (rust-lang/cargo#11661) - Warn on commits to non-default branches. (rust-lang/cargo#11655) - Avoid saving the same future_incompat warning multiple times (rust-lang/cargo#11648) - Mention current default value in `publish.timeout` docs (rust-lang/cargo#11652) - Make cargo aware of dwp files. (rust-lang/cargo#11572) - Reduce target info rustc query calls (rust-lang/cargo#11633) - Bump to 0.70.0; update changelog (rust-lang/cargo#11640) - Enable sparse protocol in CI (rust-lang/cargo#11632) - Fix split-debuginfo support detection (rust-lang/cargo#11347) - refactor(toml): Move `TomlWorkspaceDependency` out of `TomlDependency` (rust-lang/cargo#11565) - book: describe how the current resolver sometimes duplicates deps (rust-lang/cargo#11604) - `cargo add` check `[dependencies]` order without considering the dotted item (rust-lang/cargo#11612) - Link CoC to www.rust-lang.org/conduct.html (rust-lang/cargo#11622) - Add more labels to triagebot (rust-lang/cargo#11621)

Update cargo 18 commits in 3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c..e84a7928d93a31f284b497c214a2ece69b4d7719 2023-01-24 15:48:15 +0000 to 2023-01-31 22:18:09 +0000 - chore: Add autolabel for `Command-*` labels (rust-lang/cargo#11664) - Update cross test instructions for aarch64-apple-darwin (rust-lang/cargo#11663) - Make cargo install report needed features (rust-lang/cargo#11647) - docs(contrib): Remove out-of-date process step (rust-lang/cargo#11662) - Do not error for `auth-required: true` without `-Z sparse-registry` (rust-lang/cargo#11661) - Warn on commits to non-default branches. (rust-lang/cargo#11655) - Avoid saving the same future_incompat warning multiple times (rust-lang/cargo#11648) - Mention current default value in `publish.timeout` docs (rust-lang/cargo#11652) - Make cargo aware of dwp files. (rust-lang/cargo#11572) - Reduce target info rustc query calls (rust-lang/cargo#11633) - Bump to 0.70.0; update changelog (rust-lang/cargo#11640) - Enable sparse protocol in CI (rust-lang/cargo#11632) - Fix split-debuginfo support detection (rust-lang/cargo#11347) - refactor(toml): Move `TomlWorkspaceDependency` out of `TomlDependency` (rust-lang/cargo#11565) - book: describe how the current resolver sometimes duplicates deps (rust-lang/cargo#11604) - `cargo add` check `[dependencies]` order without considering the dotted item (rust-lang/cargo#11612) - Link CoC to www.rust-lang.org/conduct.html (rust-lang/cargo#11622) - Add more labels to triagebot (rust-lang/cargo#11621) r? `@ghost`

rustbot assigned epage Jan 13, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 13, 2023

khuey mentioned this pull request Jan 13, 2023

Split DWARF inlining should be disabled by default rust-lang/rust#106592

Closed

rustbot assigned weihanglo and unassigned epage Jan 13, 2023

khuey force-pushed the dwp branch from 1230a76 to 0b9c54b Compare January 13, 2023 15:12

khuey force-pushed the dwp branch from 0b9c54b to db7a250 Compare January 13, 2023 16:13

khuey force-pushed the dwp branch from db7a250 to f21f2b4 Compare January 14, 2023 00:14

khuey force-pushed the dwp branch from f21f2b4 to 847dc6e Compare January 15, 2023 07:12

weihanglo reviewed Jan 15, 2023

View reviewed changes

src/cargo/core/compiler/build_context/target_info.rs Show resolved Hide resolved

src/cargo/core/compiler/build_context/target_info.rs Outdated Show resolved Hide resolved

address review comments

a393267

weihanglo added S-blocked and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2023

weihanglo mentioned this pull request Jan 16, 2023

Register .dwp files as debuginfo artifacts so they get uplifted #11384

Closed

NobodyXu mentioned this pull request Jan 17, 2023

When cargo#11384 lands: use uplifted dwp in release builds cargo-bins/cargo-binstall#664

Open

Gankra mentioned this pull request Jan 19, 2023

Splitting Debuginfo axodotdev/cargo-dist#30

Open

khuey mentioned this pull request Jan 23, 2023

Support split DWARF gimli-rs/addr2line#260

Merged

khuey requested a review from weihanglo January 30, 2023 04:44

weihanglo approved these changes Jan 30, 2023

View reviewed changes

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-blocked labels Jan 30, 2023

bors merged commit 99b1369 into rust-lang:master Jan 30, 2023

weihanglo mentioned this pull request Feb 1, 2023

Update cargo rust-lang/rust#107541

Merged

ehuss added this to the 1.69.0 milestone Feb 2, 2023

khuey mentioned this pull request Feb 13, 2023

Are dwp files the right choice for split-debuginfo=packed on Linux? rust-lang/rust#105991

Open

khuey deleted the dwp branch August 4, 2024 05:03

Comments

Conversation

khuey commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 13, 2023

Uh oh!

khuey commented Jan 13, 2023

Uh oh!

khuey commented Jan 13, 2023

Uh oh!

weihanglo commented Jan 13, 2023

Uh oh!

weihanglo commented Jan 13, 2023

Uh oh!

khuey commented Jan 13, 2023

Uh oh!

khuey commented Jan 13, 2023

Uh oh!

ehuss commented Jan 13, 2023

Uh oh!

khuey commented Jan 14, 2023

Uh oh!

khuey commented Jan 15, 2023

Uh oh!

Uh oh!

Uh oh!

khuey commented Jan 16, 2023

Uh oh!

weihanglo commented Jan 16, 2023

Uh oh!

Gankra commented Jan 16, 2023

Uh oh!

khuey commented Jan 27, 2023

Uh oh!

weihanglo left a comment

Choose a reason for hiding this comment

Uh oh!

weihanglo commented Jan 30, 2023

Uh oh!

bors commented Jan 30, 2023

Uh oh!

bors commented Jan 30, 2023

Uh oh!

bors commented Jan 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

khuey commented Jan 13, 2023 •

edited

Loading