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

Stabilize -Zdwarf-version as -Cdwarf-version #136926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+42 −32
Open

Stabilize -Zdwarf-version as -Cdwarf-version #136926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ fn test_codegen_options_tracking_hash() {
tracked!(control_flow_guard, CFGuard::Checks);
tracked!(debug_assertions, Some(true));
tracked!(debuginfo, DebugInfo::Limited);
tracked!(dwarf_version, Some(5));
tracked!(embed_bitcode, false);
tracked!(force_frame_pointers, FramePointer::Always);
tracked!(force_unwind_tables, Some(true));
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,9 @@ options! {
"allow the linker to link its default libraries (default: no)"),
dlltool: Option<PathBuf> = (None, parse_opt_pathbuf, [UNTRACKED],
"import library generation tool (ignored except when targeting windows-gnu)"),
#[rustc_lint_opt_deny_field_access("use `Session::dwarf_version` instead of this field")]
dwarf_version: Option<u32> = (None, parse_opt_number, [TRACKED],
"version of DWARF debug information to emit (default: 2 or 4, depending on platform)"),
embed_bitcode: bool = (true, parse_bool, [TRACKED],
"emit bitcode in rlibs (default: yes)"),
extra_filename: String = (String::new(), parse_string, [UNTRACKED],
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_session/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,11 @@ impl Session {

/// Returns the DWARF version passed on the CLI or the default for the target.
pub fn dwarf_version(&self) -> u32 {
self.opts.unstable_opts.dwarf_version.unwrap_or(self.target.default_dwarf_version)
self.opts
.cg
.dwarf_version
.or(self.opts.unstable_opts.dwarf_version)
.unwrap_or(self.target.default_dwarf_version)
}

pub fn stack_protector(&self) -> StackProtector {
Expand Down Expand Up @@ -1250,7 +1254,9 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
sess.dcx().emit_err(errors::BranchProtectionRequiresAArch64);
}

if let Some(dwarf_version) = sess.opts.unstable_opts.dwarf_version {
if let Some(dwarf_version) =
sess.opts.cg.dwarf_version.or(sess.opts.unstable_opts.dwarf_version)
{
// DWARF 1 is not supported by LLVM and DWARF 6 is not yet finalized.
if dwarf_version < 2 || dwarf_version > 5 {
sess.dcx().emit_err(errors::UnsupportedDwarfVersion { dwarf_version });
Expand Down
13 changes: 13 additions & 0 deletions src/doc/rustc/src/codegen-options/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@ It takes a path to [the dlltool executable](https://sourceware.org/binutils/docs
If this flag is not specified, a dlltool executable will be inferred based on
the host environment and target.

## dwarf-version

This option controls the version of DWARF that the compiler emits, on platforms
that use DWARF to encode debug information. It takes one of the following
values:
Comment on lines +115 to +117
Copy link
Member

@jieyouxu jieyouxu Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Discussion: should Pick the max DWARF version when LTO'ing modules with different versions #136659 be described here, now that this is being proposed for stabilization?

For DWARF experts maybe, is there ever a reason to not want to pick the max DWARF version when LTO-ing modules with different DWARF versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidtwco, @nikic, @workingjubilee Do any of you have opinions on the LLVM module merge behavior for the DWARF version flag?

Copy link
Member

@workingjubilee workingjubilee Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

trying to generate debuginfo while doing significant LTO seems to have exciting results anyways, so perhaps we should consider heavily caveating our behavior when this is combined with LTO: https://github.com/llvm/llvm-project/issues?q=is%3Aissue%20state%3Aopen%20%20label%3ALTO%20label%3Adebuginfo

that would not be because the choice for the module versions made is bad, though. indeed, "pick highest" seems like the only reasonable option.


* `2`: DWARF version 2 (the default on certain platforms, like Android).
* `3`: DWARF version 3 (the default on certain platforms, like AIX).
* `4`: DWARF version 4 (the default on most platforms, like Linux & macOS).
* `5`: DWARF version 5.

DWARF version 1 is not supported.

## embed-bitcode

This flag controls whether or not the compiler embeds LLVM bitcode into object
Expand Down
13 changes: 0 additions & 13 deletions src/doc/unstable-book/src/compiler-flags/dwarf-version.md
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion tests/assembly/auxiliary/dwarf-mixed-versions-lto-aux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ compile-flags: -g --crate-type=rlib -Zdwarf-version=4
//@ compile-flags: -g --crate-type=rlib -Cdwarf-version=4

pub fn check_is_even(number: &u64) -> bool {
number % 2 == 0
Expand Down
2 changes: 1 addition & 1 deletion tests/assembly/dwarf-mixed-versions-lto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

//@ only-linux
//@ aux-build:dwarf-mixed-versions-lto-aux.rs
//@ compile-flags: -C lto -g -Zdwarf-version=5
//@ compile-flags: -C lto -g -Cdwarf-version=5
//@ assembly-output: emit-asm
//@ no-prefer-dynamic

Expand Down
4 changes: 2 additions & 2 deletions tests/assembly/dwarf4.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Makes sure that `-Z dwarf-version=4` causes `rustc` to emit DWARF version 4.
// Makes sure that `-C dwarf-version=4` causes `rustc` to emit DWARF version 4.
//@ assembly-output: emit-asm
//@ compile-flags: -g --target x86_64-unknown-linux-gnu -Z dwarf-version=4 -Copt-level=0
//@ compile-flags: -g --target x86_64-unknown-linux-gnu -C dwarf-version=4 -Copt-level=0
//@ needs-llvm-components: x86

Comment on lines +3 to 4
Copy link
Member

Choose a reason for hiding this comment

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

Discussion (re. no indication if DWARF is unsupported):

Currently, passing -{C,Z} dwarf-version on targets like *-windows-msvc does not do anything. It may be preferable to emit a warning alerting the user that the flag has no effect on the target platform. Alternatively, we could emit an error but this could be annoying since it would require the use of target specific RUSTFLAGS to use the flag correctly (and there isn't a way to target "any platform that uses DWARF" using cfgs).

Could we maybe emit an allow-able warn-by-default if -{C,Z} dwarf-version is a no-op? It might be surprising or misleading if flag just silently does nothing on unsupported platforms.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it could make sense to generate a warning or informational message in that case but I could also see it just being annoying on Windows/MSVC where this simply doesn't matter. If the user really needs DWARF for some reason, they're going to find out very quickly that DWARF is not used on those targets. For most other deprecated no-opt -C flags, we generate an unconditional warning (not a lint).

Copy link
Member

@jieyouxu jieyouxu Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah. It can't be an unconditional warning because you can't silence that and it would be very annoying. I think maybe a remark about this being no-op in the docs would suffice (at the risk of it becoming outdated, but still)?

#![feature(no_core, lang_items)]
Expand Down
4 changes: 2 additions & 2 deletions tests/assembly/dwarf5.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Makes sure that `-Z dwarf-version=5` causes `rustc` to emit DWARF version 5.
// Makes sure that `-C dwarf-version=5` causes `rustc` to emit DWARF version 5.
//@ assembly-output: emit-asm
//@ compile-flags: -g --target x86_64-unknown-linux-gnu -Z dwarf-version=5 -Copt-level=0
//@ compile-flags: -g --target x86_64-unknown-linux-gnu -C dwarf-version=5 -Copt-level=0
//@ needs-llvm-components: x86

#![feature(no_core, lang_items)]
Expand Down
2 changes: 1 addition & 1 deletion tests/run-make/embed-source-dwarf/rmake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn main() {
.output(&output)
.arg("-g")
.arg("-Zembed-source=yes")
.arg("-Zdwarf-version=5")
.arg("-Cdwarf-version=5")
.run();
let output = rfs::read(output);
let obj = object::File::parse(output.as_slice()).unwrap();
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/debuginfo/dwarf-versions.rs
View file
Open in desktop
Copy link
Member

@jieyouxu jieyouxu Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Question (not for this test but just to have an inline comment somewhere instead of having a ton of top-level comments): does -Cdwarf-version have any interesting interactions with -Csplit-dwarf?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I understand it, most DWARF features are implemented as extensions on top of the base specification describing the format itself. Split DWARF is technically a DWARF 5 feature but it's just an agreed-upon extension to DWARF itself. So -Cdwarf-version=2 -Csplit-dwarf will generate DWARF 2 but with the Split DWARF extension "enabled".

As such, I don't think there's any interesting interactions (@davidtwco please correct me if I'm wrong here) other than the fact you can target a lower DWARF standard version and still have this feature enabled (which is the case right now on stable Rust as we target DWARF 4 by default but still support Split DWARF).

Copy link
Member

@workingjubilee workingjubilee Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That is effectively correct. As DWARF moves forward, it generally is extended in a way that a debugger that doesn't support it will just be less able to read the debuginfo and produce less useful feedback, not experience a critical error. But I'm only really somewhat familiar with DWARFv4 and DWARFv5. Maybe @tromey, @khuey, or @philipc have more thoughts on how that works out in practice?

Copy link
Contributor

Choose a reason for hiding this comment

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

DWARF 5 rearranged the compilation unit header so I think any reader had to adapt to even try to scan the contents.

Often a lot of things are shared or overlap though. And DWARF readers are expected to ignore some things they don't understand, like extension tags or unusual attributes or whatnot.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not correct. There is an interaction between the DWARF version and split-debuginfo=[un]packed.

Without -Zdwarf-version=5 rustc/llvm emit the pre-DWARF 5 GNU extension documented at https://gcc.gnu.org/wiki/DebugFission. With -Zdwarf-version=5 rust/llvm emit the standardized DWARF 5 split DWARF from the specification. These two differ in the attributes used, forms, and even some of the ELF section names, and are not directly compatible. Generally tools support both, though.

As far as what the compatibility story is that depends on what you consider a "critical error". Debugging symbols are of course always optional and the debugger can operate without them at all. Since DWARF is versioned on a per-compilation-unit basis, in a binary with heterogeneous DWARF versions in principle a debugger could simply skip the compilation units with the version it doesn't understand while continuing to provide a full debugging experience for the compilation units it does understand.

Copy link
Member

@workingjubilee workingjubilee Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, I wasn't aware of the GNU extension! (I mean, I was aware there was a predecessor but not that we actually already emitted it, huh!)

Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
// This test verifies the expected behavior of various options passed to
// `-Zdwarf-version`: 2 - 5 (valid) with all other options being invalid.
// `-Cdwarf-version`: 2 - 5 (valid) with all other options being invalid.

//@ revisions: zero one two three four five six

//@[zero] compile-flags: -Zdwarf-version=0
//@[zero] compile-flags: -Cdwarf-version=0
//@[zero] error-pattern: requested DWARF version 0 is not supported

//@[one] compile-flags: -Zdwarf-version=1
//@[one] compile-flags: -Cdwarf-version=1
//@[one] error-pattern: requested DWARF version 1 is not supported

//@[two] compile-flags: -Zdwarf-version=2
//@[two] compile-flags: -Cdwarf-version=2
//@[two] check-pass

//@[three] compile-flags: -Zdwarf-version=3
//@[three] compile-flags: -Cdwarf-version=3
//@[three] check-pass

//@[four] compile-flags: -Zdwarf-version=4
//@[four] compile-flags: -Cdwarf-version=4
//@[four] check-pass

//@[five] compile-flags: -Zdwarf-version=5
//@[five] compile-flags: -Cdwarf-version=5
//@[five] check-pass

//@[six] compile-flags: -Zdwarf-version=6
//@[six] compile-flags: -Cdwarf-version=6
//@[six] error-pattern: requested DWARF version 6 is not supported

//@ compile-flags: -g --target x86_64-unknown-linux-gnu --crate-type cdylib
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lto/auxiliary/dwarf-mixed-versions-lto-aux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ compile-flags: -g --crate-type=rlib -Zdwarf-version=4
//@ compile-flags: -g --crate-type=rlib -Cdwarf-version=4

pub fn say_hi() {
println!("hello there")
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lto/dwarf-mixed-versions-lto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

//@ ignore-msvc Platform must use DWARF
//@ aux-build:dwarf-mixed-versions-lto-aux.rs
//@ compile-flags: -C lto -g -Zdwarf-version=5
//@ compile-flags: -C lto -g -Cdwarf-version=5
Copy link
Member

Choose a reason for hiding this comment

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

Discussion (re. call-for-testing period): should we still explicitly have a call-for-testing period, to invite users to explicitly try to break this flag somehow? It seems not too high risk, but still.

No call-for-testing has been conducted but Rust for Linux has been using this flag without issue.

RfL is an important user group, but they may have their specific ways of exercising this flag. Would we benefit from having more diverse user groups try to break this?

Copy link
Member Author

Choose a reason for hiding this comment

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

No objection to a call-for-testing period but I think we've gotten most of the interesting user testing done by RfL here as the Linux kernel developer community cares quite a lot about things like the various new compact forms in DWARF 5.

Copy link
Member

@jieyouxu jieyouxu Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That's good enough for me. I'll leave this unresolved for a brief moment, in case someone else has concerns. But feel free to resolve this (non-blocking).

//@ no-prefer-dynamic
//@ build-pass

Expand Down
Loading