Skip to content

Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF#91566

Merged
bors merged 8 commits intorust-lang:masterfrom
cbeuw:remap-dwo-name
Dec 18, 2021
Merged

Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF#91566
bors merged 8 commits intorust-lang:masterfrom
cbeuw:remap-dwo-name

Conversation

@cbeuw
Copy link
Contributor

@cbeuw cbeuw commented Dec 5, 2021 •
edited
Loading

--remap-path-prefix doesn't apply to paths to .o (in case of packed) or .dwo (in case of unpacked) files in DW_AT_GNU_dwo_name. GCC also has this bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91888

@rust-highfive
Copy link
Contributor

rust-highfive commented Dec 5, 2021

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Dec 5, 2021

cc @davidtwco I saw you mention dwp on zulip recently

@davidtwco
Copy link
Member

davidtwco commented Dec 5, 2021 •
edited
Loading

I don't have time to do a full review right now (will take a closer look tomorrow), but we could probably do this without breaking packaging:

As rustc invokes the dwarf packaging utility (rather than that being part of a separate build process), we can just always provide the paths to the object files directly rather than just rely on the path computed in the packaging utility from DW_AT_comp_dir and DW_AT_GNU_dwo_name (and for dependencies, we look inside the rlib).

@davidtwco
Copy link
Member

davidtwco commented Dec 5, 2021

r? @davidtwco

@rust-highfive rust-highfive assigned davidtwco and unassigned oli-obk Dec 5, 2021
davidtwco
davidtwco approved these changes Dec 6, 2021
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good to me, are there other changes you want to make (the PR is marked WIP)?

@cbeuw
Copy link
Contributor Author

cbeuw commented Dec 6, 2021

I marked it as draft because I didn't really know if this will work and I wanted to see the Linux CI run results (I'm on Windows so can't really test DWARF properly).

I'll remove WIP and I'm happy to merge as-is once bors is happy too

@cbeuw cbeuw changed the title [WIP] Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF Dec 6, 2021
@cbeuw cbeuw marked this pull request as ready for review December 6, 2021 14:03
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cbeuw
Copy link
Contributor Author

cbeuw commented Dec 6, 2021

Turns out my test actually wasn't doing anything and DW_AT_GNU_dwo_name still isn't properly remapped.

This acutally came up before #82074. Will look into it later

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cbeuw

This comment has been minimized.

Sign in to view
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2021
@cbeuw

This comment has been minimized.

Sign in to view
@cbeuw
Copy link
Contributor Author

cbeuw commented Dec 11, 2021

I misunderstood how MCOptions.SplitDwarfFile is used in LLVM. It does control what will be emitted into DW_AT_[GNU_]dwo_name, but it can be remapped because it's not used as a real path if pass in a separate a filestream when we create an emit file pass, which we do here:

rust/compiler/rustc_codegen_llvm/src/back/write.rs

Lines 913 to 924 in 4a66a70

with_codegen(tm, llmod, config.no_builtins, |cpm| {
write_output_file(
diag_handler,
tm,
cpm,
llmod,
&obj_out,
dwo_out,
llvm::FileType::ObjectFile,
&cgcx.prof,
)
})?;

And eventually

rust/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

Lines 644 to 656 in 4a66a70

if (DwoPath) {
raw_fd_ostream DOS(DwoPath, EC, sys::fs::OF_None);
EC.clear();
if (EC)
ErrorInfo = EC.message();
if (ErrorInfo != "") {
LLVMRustSetLastError(ErrorInfo.c_str());
return LLVMRustResult::Failure;
}
buffer_ostream DBOS(DOS);
unwrap(Target)->addPassesToEmitFile(*PM, BOS, &DBOS, FileType, false);
PM->run(*unwrap(M));
} else {
.

So the LLVM change is not needed and the current solution works

@cbeuw cbeuw requested a review from davidtwco December 11, 2021 11:32
davidtwco
davidtwco requested changes Dec 13, 2021
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, comment or two about some changes but broadly what I'd expect.

@davidtwco
Copy link
Member

davidtwco commented Dec 16, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 16, 2021

📌 Commit 5e481d0 has been approved by davidtwco

@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 Dec 16, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 17, 2021
@matthiaskrgr
Rollup merge of rust-lang#91566 - cbeuw:remap-dwo-name, r=davidtwco
84fbd22 
Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF

`--remap-path-prefix` doesn't apply to paths to `.o` (in case of packed) or `.dwo` (in case of unpacked) files in `DW_AT_GNU_dwo_name`. GCC also has this bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91888
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 17, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 17, 2021
@matthiaskrgr
Rollup merge of rust-lang#91566 - cbeuw:remap-dwo-name, r=davidtwco
b3f23e4 
Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF

`--remap-path-prefix` doesn't apply to paths to `.o` (in case of packed) or `.dwo` (in case of unpacked) files in `DW_AT_GNU_dwo_name`. GCC also has this bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91888
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 17, 2021
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 18, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2021
@bors
Auto merge of rust-lang#92065 - matthiaskrgr:rollup-qmpcsuj, r=matthi…
91a0600 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#91566 (Apply path remapping to DW_AT_GNU_dwo_name when producing split DWARF)
 - rust-lang#91926 (Remove `in_band_lifetimes` from `rustc_metadata`)
 - rust-lang#91931 (Remove `in_band_lifetimes` from `rustc_codegen_llvm`)
 - rust-lang#92024 (rustc_codegen_llvm: Give each codegen unit a unique DWARF name on all platforms, not just Apple ones.)
 - rust-lang#92037 (Use a const ParamEnv when in default_method_body_is_const)
 - rust-lang#92047 (Set `RUST_BACKTRACE=0` when running location-detail tests)
 - rust-lang#92050 (Add a space and 2 grave accents )

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1c42199 into rust-lang:master Dec 18, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 18, 2021
@cbeuw cbeuw deleted the remap-dwo-name branch December 18, 2021 21:19
@jieyouxu jieyouxu added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-reproducibility Area: Reproducible / deterministic builds labels Aug 14, 2024
@jieyouxu jieyouxu mentioned this pull request Aug 14, 2024
Open
81 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels

A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-reproducibility Area: Reproducible / deterministic builds 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

Milestone

1.59.0

Development

Successfully merging this pull request may close these issues.

9 participants

@cbeuw @rust-highfive @oli-obk @davidtwco @rust-log-analyzer @bors @apiraino @jieyouxu @rustbot