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.

Sign up for GitHub

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

Jump to bottom

Preserve split DWARF files when building archives. #106904

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Preserve split DWARF files when building archives. #106904

merged 1 commit into from
Jan 26, 2023

Conversation

khuey
Copy link
Contributor

@khuey khuey commented Jan 15, 2023

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 15, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@khuey
Preserve split DWARF files when building archives.
2b99b9f 
The optimization that removes artifacts when building libraries is correct
from the compiler's perspective but not from a debugger's perspective.
Unpacked split debuginfo is referred to by filename and debuggers need
the artifact that contains debuginfo to continue to exist at that path.

Ironically the test expects the correct behavior but it was not running.
@khuey khuey force-pushed the preserve_debuginfo_for_rlibs branch from d4025ad to 2b99b9f Compare January 15, 2023 17:53
@khuey khuey mentioned this pull request Jan 23, 2023
Merged
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 25, 2023

📌 Commit 2b99b9f has been approved by davidtwco

It is now in the queue for this repository.

@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 25, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
@matthiaskrgr
Rollup merge of rust-lang#106904 - khuey:preserve_debuginfo_for_rlibs…
a8f5028 
…, r=davidtwco

Preserve split DWARF files when building archives.

r? `@davidtwco`
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 26, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
@matthiaskrgr
Rollup merge of rust-lang#106904 - khuey:preserve_debuginfo_for_rlibs…
e328198 
…, r=davidtwco

Preserve split DWARF files when building archives.

r? ``@davidtwco``
This was referenced Jan 26, 2023
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2023
@bors
Auto merge of rust-lang#107328 - matthiaskrgr:rollup-lfqwo0o, r=matth…
c62665e 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#106904 (Preserve split DWARF files when building archives.)
 - rust-lang#106971 (Handle diagnostics customization on the fluent side (for one specific diagnostic))
 - rust-lang#106978 (Migrate mir_build's borrow conflicts)
 - rust-lang#107150 (`ty::tls` cleanups)
 - rust-lang#107168 (Use a type-alias-impl-trait in `ObligationForest`)
 - rust-lang#107189 (Encode info for Adt in a single place.)
 - rust-lang#107322 (Custom mir: Add support for some remaining, easy to support constructs)
 - rust-lang#107323 (Disable ConstGoto opt in cleanup blocks)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1b442be into rust-lang:master Jan 26, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 26, 2023
@ehuss
Copy link
Contributor

ehuss commented Feb 14, 2023

Can you say more about what kinds of issues this was causing? On macOS, I haven't noticed any problems with lldb finding debuginfo for rlib dependencies.

@khuey
Copy link
Contributor Author

khuey commented Feb 14, 2023

With -Csplit-debuginfo=unpacked the DWO files with the debug info were being thrown away.

@ehuss
Copy link
Contributor

ehuss commented Feb 14, 2023

Yes, but how does that manifest as a problem? Does a debugger stepping into a dependency just show assembly output? Why would macOS be different here? My understanding is that macOS just dumps a bunch of .o files, but only for the executable. Does macOS store the debuginfo for the dependencies inside the .o files for the binary? If so, perhaps this change isn't needed for macOS?

@khuey
Copy link
Contributor Author

khuey commented Feb 14, 2023

It shows as there being no debugging information for the relevant crate, no source, no function names, etc.

Are you using split debuginfo on Mac? You haven't stated anything about your configuration yet.

@ehuss
Copy link
Contributor

ehuss commented Feb 14, 2023

I'm using all defaults with Cargo, whose default is unpacked.

@khuey
Copy link
Contributor Author

khuey commented Feb 14, 2023

Ok, then cargo should take the

(SplitDebuginfo::Unpacked, _) if !sess.target_can_use_split_dwarf() => (true, false)

branch in order to leave the object files on the file system where the debugger can find them.

@ehuss
Copy link
Contributor

ehuss commented Feb 14, 2023

Sorry, I'm not sure I follow. The debugger was finding the debuginfo fine before this PR.

Historically, split-debuginfo=unpacked on macOS only left unpacked .o files for the final binary. It did not leave unpacked .o files for rlibs. lldb worked fine and was able to find debug information for dependencies. I don't know enough about DWARF or how lldb finds debuginfo to say how that worked.

This PR changed it so that rlibs also leave .o files. My question is: is that necessary?

@khuey
Copy link
Contributor Author

khuey commented Feb 14, 2023

My question is: is that necessary?

I don't know, but I don't see how things would have worked on Mac before this change.

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
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.69.0
Development

Successfully merging this pull request may close these issues.
6 participants
@khuey @rust-log-analyzer @davidtwco @bors @ehuss @rustbot