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

Implement support for DWARF version 5. #98350

Merged
merged 1 commit into from
Jul 9, 2022
Merged

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Jun 21, 2022

DWARF version 5 brings a number of improvements over version 4. Quoting from
the announcement 1:

Version 5 incorporates improvements in many areas: better data compression,
separation of debugging data from executable files, improved description of
macros and source files, faster searching for symbols, improved debugging
optimized code, as well as numerous improvements in functionality and
performance.

On platforms where DWARF version 5 is supported (Linux, primarily), this commit
adds support for it behind a new -Z dwarf-version=5 flag.

r? @michaelwoerister

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 21, 2022
@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 Jun 21, 2022
@rust-log-analyzer

This comment has been minimized.

@nikic
Copy link
Contributor

nikic commented Jun 21, 2022

This should probably be an unstable -Z dwarf-version flag, unless you intend this to be insta-stable.

Is the max dwarf version logic really needed? It seems odd that there is no way to force a newer version if a target has this option set. It's reasonable that some platforms default to an older version, but I don't really see a reason to hard forbid using a newer one (for example, if I bring my own toolchain).

@pcwalton pcwalton force-pushed the dwarf5 branch 2 times, most recently from 3e54d38 to 6c73840 Compare June 21, 2022 22:30
@pcwalton
Copy link
Contributor Author

OK, I've switched the -C flag to a -Z flag, removed the max_dwarf_version field from the target info, and moved the documentation over to the unstable book.

@nnethercote
Copy link
Contributor

Is this worth a perf run?

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

src/test/assembly/dwarf5.rs Outdated Show resolved Hide resolved
@michaelwoerister
Copy link
Member

Thanks, @pcwalton! Looks good to me.

@bors r+

Is this worth a perf run?

As it currently is, the PR does not actually change the DWARF version being used for anything. It just adds the -Z flag for explicitly setting it. So, we shouldn't see a perf difference. But once this is merged it would be interesting to see how switching to v5 affects performance.

@bors
Copy link
Contributor

bors commented Jun 22, 2022

📌 Commit 6c73840877fb5db8fd84c431d7078abe23cd8f87 has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 22, 2022

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@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 Jun 22, 2022
@pcwalton
Copy link
Contributor Author

Addressed comment. I think I need another review for bors to pick it up, r? @michaelwoerister

@nikic
Copy link
Contributor

nikic commented Jun 22, 2022

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 22, 2022

📌 Commit 42eeb58 has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 22, 2022

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@rust-log-analyzer

This comment has been minimized.

@pcwalton
Copy link
Contributor Author

Doesn't look related to this PR.

@nnethercote
Copy link
Contributor

That failure is the one that caused the tree closure. Should be resolved now.

@bors retry

@michaelwoerister
Copy link
Member

@bors retry

@michaelwoerister
Copy link
Member

@bors r+

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 8, 2022
@ehuss
Copy link
Contributor

ehuss commented Jul 8, 2022

@bors r=michaelwoerister

retry does not work when it has been unaccepted.

@bors
Copy link
Contributor

bors commented Jul 8, 2022

📌 Commit 1e0ad0c has been approved by michaelwoerister

It is now in the queue for this repository.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 9, 2022
Implement support for DWARF version 5.

DWARF version 5 brings a number of improvements over version 4. Quoting from
the announcement [1]:

> Version 5 incorporates improvements in many areas: better data compression,
> separation of debugging data from executable files, improved description of
> macros and source files, faster searching for symbols, improved debugging
> optimized code, as well as numerous improvements in functionality and
> performance.

On platforms where DWARF version 5 is supported (Linux, primarily), this commit
adds support for it behind a new `-Z dwarf-version=5` flag.

[1]: https://dwarfstd.org/Public_Review.php

r? `@michaelwoerister`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 9, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#98350 (Implement support for DWARF version 5.)
 - rust-lang#98915 (Clarify deriving code)
 - rust-lang#98980 (fix ICE in ConstProp)
 - rust-lang#99008 (Adding suggestion for E0530)
 - rust-lang#99043 (Collapse some weirdly-wrapping derives)
 - rust-lang#99048 (Remove a string comparison about types)
 - rust-lang#99070 (Update integer_atomics tracking issue)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fd4f11d into rust-lang:master Jul 9, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 9, 2022
glandium added a commit to glandium/cc-rs that referenced this pull request Jul 12, 2022
Rustc defaults to DWARF-2 on some targets, and DWARF-4 on others.
However using -g with the C compiler yields whatever default version the
C compiler prefers.

One side effect is that the DWARF debug info shipped in some libraries
with rustc itself (e.g. libcompiler_builtins and others) have recently
switched to DWARF-5 as a side effect of upgrading the clang version
used on rustc CI. (rust-lang/rust#98746)

Ideally, the preferred DWARF version would be given by the rust compiler
and/or cargo, but that's not the case at the moment, so the next best
thing is something that aligns with the current defaults, although
work in under way to add a rustc flag that would allow to pick the
preferred DWARF version (rust-lang/rust#98350)
glandium added a commit to glandium/cc-rs that referenced this pull request Jul 12, 2022
Rustc defaults to DWARF-2 on some targets, and DWARF-4 on others.
However using -g with the C compiler yields whatever default version the
C compiler prefers.

One side effect is that the DWARF debug info shipped in some libraries
with rustc itself (e.g. libcompiler_builtins and others) have recently
switched to DWARF-5 as a side effect of upgrading the clang version
used on rustc CI. (rust-lang/rust#98746)

Ideally, the preferred DWARF version would be given by the rust compiler
and/or cargo, but that's not the case at the moment, so the next best
thing is something that aligns with the current defaults, although
work in under way to add a rustc flag that would allow to pick the
preferred DWARF version (rust-lang/rust#98350)
Comment on lines -106 to +113
if let Some(version) = sess.target.dwarf_version {
llvm::LLVMRustAddModuleFlag(
self.llmod,
llvm::LLVMModFlagBehavior::Warning,
"Dwarf Version\0".as_ptr().cast(),
version,
)
}
let dwarf_version =
sess.opts.debugging_opts.dwarf_version.unwrap_or(sess.target.default_dwarf_version);
llvm::LLVMRustAddModuleFlag(
self.llmod,
llvm::LLVMModFlagBehavior::Warning,
"Dwarf Version\0".as_ptr().cast(),
dwarf_version,
);
Copy link
Member

Choose a reason for hiding this comment

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

Could this have caused #99143? Looks like before this PR, we only ever set Dwarf Version on targets with Some in their dwarf_version, which based on the diff in this PR, wasn't the case for e.g. Windows (at least, there's probably more targets not covered by the ${OS}_base.rs files).

Copy link
Member

Choose a reason for hiding this comment

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

Alright that was confusing but I'm pretty sure (#99143 (comment)) that what's going on is even with debuginfo disabled for a binary, the standard library is getting statically linked in and we ship it w/ debuginfo.

I'm not sure what the best approach here would be, maybe just relying on the is_like_msvc check to effectively switch between CodeView and Dwarf Version (and never set both)?

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 14, 2022
fix dwarf debuginfo being used in addition to CodeView on windows

Tackles the debuginfo size increase regression on windows to [unblock clippy](rust-lang#99143 (comment)) -- introduced by the DWARF5 support in rust-lang#98350 cc `@pcwalton.`

r? `@eddyb`
Fixes rust-lang#99143
@Wyvern
Copy link

Wyvern commented Jul 15, 2022

Does macOS support DWARF5 at the moment?

glandium added a commit to glandium/cc-rs that referenced this pull request Nov 10, 2022
Rustc defaults to DWARF-2 on some targets, and DWARF-4 on others.
However using -g with the C compiler yields whatever default version the
C compiler prefers.

One side effect is that the DWARF debug info shipped in some libraries
with rustc itself (e.g. libcompiler_builtins and others) have recently
switched to DWARF-5 as a side effect of upgrading the clang version
used on rustc CI. (rust-lang/rust#98746)

Ideally, the preferred DWARF version would be given by the rust compiler
and/or cargo, but that's not the case at the moment, so the next best
thing is something that aligns with the current defaults, although
work in under way to add a rustc flag that would allow to pick the
preferred DWARF version (rust-lang/rust#98350)
thomcc pushed a commit to rust-lang/cc-rs that referenced this pull request Nov 10, 2022
Rustc defaults to DWARF-2 on some targets, and DWARF-4 on others.
However using -g with the C compiler yields whatever default version the
C compiler prefers.

One side effect is that the DWARF debug info shipped in some libraries
with rustc itself (e.g. libcompiler_builtins and others) have recently
switched to DWARF-5 as a side effect of upgrading the clang version
used on rustc CI. (rust-lang/rust#98746)

Ideally, the preferred DWARF version would be given by the rust compiler
and/or cargo, but that's not the case at the moment, so the next best
thing is something that aligns with the current defaults, although
work in under way to add a rustc flag that would allow to pick the
preferred DWARF version (rust-lang/rust#98350)
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Feb 17, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Feb 19, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Mar 5, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Mar 25, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
ojeda added a commit to ojeda/linux that referenced this pull request Mar 29, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Mar 29, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Apr 2, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
sweettea pushed a commit to sweettea/btrfs-fscrypt that referenced this pull request Apr 3, 2024
Rust 1.64.0 introduced (unstable) support for the `-Zdwarf-version`
flag, which allows to select DWARFv5, thus use it.

Link: rust-lang/rust#103057
Link: rust-lang/rust#98350
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20240217002602.57270-1-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.