-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Emit an error if -Zdwarf-version=1
is requested
#136746
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot has assigned @compiler-errors. Use |
This comment has been minimized.
This comment has been minimized.
Urgau
reviewed
Feb 8, 2025
513912d
to
2aca0a6
Compare
Urgau
approved these changes
Feb 8, 2025
Make sense to me. r? Urgau |
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this pull request
Feb 8, 2025
Emit an error if `-Zdwarf-version=1` is requested DWARF 1 is very different than DWARF 2+[^1] and LLVM does not really seem to support DWARF 1 as Clang does not offer a `-gdwarf-1` flag[^2] and `llc` will just generate DWARF 2 with the version set to 1[^3]. Since this isn't actually supported (and it's not clear it would be useful anyway), report that DWARF 1 is not supported if it is requested. Also add a help message to the error saying which versions are supported. cc rust-lang#103057 [^1]: https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html#index-gdwarf [^2]: https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-gdwarf [^3]: https://godbolt.org/z/s85d87n3a
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 8, 2025
…kingjubilee Rollup of 13 pull requests Successful merges: - rust-lang#134999 (Add cygwin target.) - rust-lang#135439 (Make `-O` mean `OptLevel::Aggressive`) - rust-lang#136397 (Add a comment pointing to ICE-136223) - rust-lang#136681 (resolve `llvm-config` path properly on cross builds) - rust-lang#136686 (Clean up `HashMap` and `HashSet` docs.) - rust-lang#136694 (Update minifier version to `0.3.4`) - rust-lang#136706 (compiler: mostly-finish `rustc_abi` updates) - rust-lang#136710 (Document `Sum::sum` returns additive identities for `[]`) - rust-lang#136724 (Make `AsyncFnOnce`, `AsyncFnMut`, `AsyncFn` non-`#[fundamental]`) - rust-lang#136727 (Have a break from review rotation) - rust-lang#136730 (transmutability: fix ICE when passing wrong ADT to ASSUME) - rust-lang#136736 (Small resolve refactor) - rust-lang#136746 (Emit an error if `-Zdwarf-version=1` is requested) r? `@ghost` `@rustbot` modify labels: rollup
@bors r- We should also error on |
2aca0a6
to
204078c
Compare
DWARF 1 is very different than DWARF 2+ (see the commentary in https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html#index-gdwarf) and LLVM does not really seem to support DWARF 1 as Clang does not offer a `-gdwarf-1` flag and `llc` will just generate DWARF 2 with the version set to 1: https://godbolt.org/z/s85d87n3a. Since this isn't actually supported (and it's not clear it would be useful anyway), report that DWARF 1 is not supported if it is requested. Also add a help message to the error saying which versions are supported.
204078c
to
eea8ce5
Compare
@bors r+ |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 10, 2025
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#135488 (Stabilize `vec_pop_if`) - rust-lang#136068 (crashes: more tests) - rust-lang#136694 (Update minifier version to `0.3.4`) - rust-lang#136722 (Visit all debug info in MIR Visitor) - rust-lang#136746 (Emit an error if `-Zdwarf-version=1` is requested) - rust-lang#136760 (Fix unwrap error in overflowing int literal) - rust-lang#136782 (Fix mistake in x86_64-unknown-freebsd platform description) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 10, 2025
Rollup merge of rust-lang#136746 - wesleywiser:err_dwarf1, r=Urgau Emit an error if `-Zdwarf-version=1` is requested DWARF 1 is very different than DWARF 2+[^1] and LLVM does not really seem to support DWARF 1 as Clang does not offer a `-gdwarf-1` flag[^2] and `llc` will just generate DWARF 2 with the version set to 1[^3]. Since this isn't actually supported (and it's not clear it would be useful anyway), report that DWARF 1 is not supported if it is requested. Also add a help message to the error saying which versions are supported. cc rust-lang#103057 [^1]: https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html#index-gdwarf [^2]: https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-gdwarf [^3]: https://godbolt.org/z/s85d87n3a
This was referenced Feb 12, 2025
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Apr 16, 2025
…n, r=petrochenkov Stabilize `-Zdwarf-version` as `-Cdwarf-version` I propose stabilizing `-Zdwarf-version` as `-Cdwarf-version`. This PR adds a new `-Cdwarf-version` flag, leaving the unstable `-Z` flag as is to ease the transition period. The `-Z` flag will be removed in the future. # `-Zdwarf-version` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC/MCP, this flag was added in rust-lang#98350 and was not deemed large enough to require additional process. The tracking issue for this feature is rust-lang#103057. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. None that has been extensively debated but there are a few questions that could have been chosen differently: 1. What should the flag name be? The current flag name is very specific to DWARF. Other debuginfo formats exist (msvc's CodeView format or https://en.wikipedia.org/wiki/Stabs) so we could have chosen to generalize the flag name (`-{C,Z} debuginfo-version=dwarf-5` for example). While this would extend cleanly to support formats other than DWARF, there are some downsides to this design. Neither CodeView nor Stabs have specification or format versions so it's not clear what values would be supported beyond `dwarf-{2,3,4,5}` or `codeview`. We would also need to take care to ensure the name does not lead users to think they can pick a format other than one supported by the target. For instance, what would `--target x86_64-pc-windows-msvc -Cdebuginfo-version=dwarf-5` do? 2. What is the behavior when flag is used on targets that do not support DWARF? 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). 3. Does the precompiled standard library potentially using a different version of DWARF a problem? I don't believe this is an issue as debuggers (and other such tools) already must deal with the possibility that an application uses different DWARF versions across its statically or dynamically linked libraries. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No extensions per se, although future DWARF versions could be considered as such. At present, we validate the requested DWARF version is between 2 and 5 (inclusive) so new DWARF versions will not automatically be supported until the validation logic is adjusted. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) - Targets define their preferred or default DWARF version: https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_target/src/spec/mod.rs#L2369 - We use the target default but this can be overriden by `-{C,Z} dwarf-version` https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L738 - The flag is validated https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L1253-L1258 - When debuginfo is generated, we tell LLVM to use the requested value or the target default https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs#L106 ## Summarize existing test coverage of this feature - Test that we actually generate the appropriate DWARF version - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf5.rs - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf4.rs - Test that LTO with different DWARF versions picks the highest version - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf-mixed-versions-lto.rs - Test DWARF versions 2-5 are valid while 0, 1 and 6 report an error - https://github.com/rust-lang/rust/blob/master/tests/ui/debuginfo/dwarf-versions.rs - Ensure LLVM does not report a warning when LTO'ing different DWARF versions together - https://github.com/rust-lang/rust/blob/master/tests/ui/lto/dwarf-mixed-versions-lto.rs ## Has a call-for-testing period been conducted? If so, what feedback was received? No call-for-testing has been conducted but Rust for Linux has been using this flag without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? All reported bugs have been resolved. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization - Initial implementation in rust-lang#98350 by `@pcwalton` - Stop emitting `.debug_pubnames` and `.debug_pubtypes` when using DWARF 5 in rust-lang#117962 by `@weihanglo.` - Refactoring & cleanups (rust-lang#135739), fix LLVM warning on LTO with different DWARF versions (rust-lang#136659) and argument validation (rust-lang#136746) by `@wesleywiser` ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? No FIXMEs related to this feature. ## What static checks are done that are needed to prevent undefined behavior? This feature cannot cause undefined behavior. We ensure the DWARF version is one of the supported values [here](https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L1255-L1257). ## In what way does this feature interact with the reference/specification, and are those edits prepared? No changes to reference/spec, unstable rustc docs are moved to the stable book as part of the stabilization PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? `-Zembed-source` requires use of DWARF 5 extensions but has its own feature gate. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup. Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors. cc-rs has support for detecting the presence of `-{C,Z} dwarf-version` in `RUSTFLAGS` and providing the corresponding flag to Clang/gcc (rust-lang/cc-rs#1395). --- Closes rust-lang#103057
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 16, 2025
… r=petrochenkov Stabilize `-Zdwarf-version` as `-Cdwarf-version` I propose stabilizing `-Zdwarf-version` as `-Cdwarf-version`. This PR adds a new `-Cdwarf-version` flag, leaving the unstable `-Z` flag as is to ease the transition period. The `-Z` flag will be removed in the future. # `-Zdwarf-version` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC/MCP, this flag was added in rust-lang#98350 and was not deemed large enough to require additional process. The tracking issue for this feature is rust-lang#103057. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. None that has been extensively debated but there are a few questions that could have been chosen differently: 1. What should the flag name be? The current flag name is very specific to DWARF. Other debuginfo formats exist (msvc's CodeView format or https://en.wikipedia.org/wiki/Stabs) so we could have chosen to generalize the flag name (`-{C,Z} debuginfo-version=dwarf-5` for example). While this would extend cleanly to support formats other than DWARF, there are some downsides to this design. Neither CodeView nor Stabs have specification or format versions so it's not clear what values would be supported beyond `dwarf-{2,3,4,5}` or `codeview`. We would also need to take care to ensure the name does not lead users to think they can pick a format other than one supported by the target. For instance, what would `--target x86_64-pc-windows-msvc -Cdebuginfo-version=dwarf-5` do? 2. What is the behavior when flag is used on targets that do not support DWARF? 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). 3. Does the precompiled standard library potentially using a different version of DWARF a problem? I don't believe this is an issue as debuggers (and other such tools) already must deal with the possibility that an application uses different DWARF versions across its statically or dynamically linked libraries. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No extensions per se, although future DWARF versions could be considered as such. At present, we validate the requested DWARF version is between 2 and 5 (inclusive) so new DWARF versions will not automatically be supported until the validation logic is adjusted. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) - Targets define their preferred or default DWARF version: https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_target/src/spec/mod.rs#L2369 - We use the target default but this can be overriden by `-{C,Z} dwarf-version` https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L738 - The flag is validated https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L1253-L1258 - When debuginfo is generated, we tell LLVM to use the requested value or the target default https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs#L106 ## Summarize existing test coverage of this feature - Test that we actually generate the appropriate DWARF version - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf5.rs - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf4.rs - Test that LTO with different DWARF versions picks the highest version - https://github.com/rust-lang/rust/blob/master/tests/assembly/dwarf-mixed-versions-lto.rs - Test DWARF versions 2-5 are valid while 0, 1 and 6 report an error - https://github.com/rust-lang/rust/blob/master/tests/ui/debuginfo/dwarf-versions.rs - Ensure LLVM does not report a warning when LTO'ing different DWARF versions together - https://github.com/rust-lang/rust/blob/master/tests/ui/lto/dwarf-mixed-versions-lto.rs ## Has a call-for-testing period been conducted? If so, what feedback was received? No call-for-testing has been conducted but Rust for Linux has been using this flag without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? All reported bugs have been resolved. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization - Initial implementation in rust-lang#98350 by `@pcwalton` - Stop emitting `.debug_pubnames` and `.debug_pubtypes` when using DWARF 5 in rust-lang#117962 by `@weihanglo.` - Refactoring & cleanups (rust-lang#135739), fix LLVM warning on LTO with different DWARF versions (rust-lang#136659) and argument validation (rust-lang#136746) by `@wesleywiser` ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? No FIXMEs related to this feature. ## What static checks are done that are needed to prevent undefined behavior? This feature cannot cause undefined behavior. We ensure the DWARF version is one of the supported values [here](https://github.com/rust-lang/rust/blob/34a5ea911c56e79bd451c63f04ea2f5023d7d1a3/compiler/rustc_session/src/session.rs#L1255-L1257). ## In what way does this feature interact with the reference/specification, and are those edits prepared? No changes to reference/spec, unstable rustc docs are moved to the stable book as part of the stabilization PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? `-Zembed-source` requires use of DWARF 5 extensions but has its own feature gate. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup. Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors. cc-rs has support for detecting the presence of `-{C,Z} dwarf-version` in `RUSTFLAGS` and providing the corresponding flag to Clang/gcc (rust-lang/cc-rs#1395). --- Closes rust-lang#103057
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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DWARF 1 is very different than DWARF 2+1 and LLVM does not really seem to support DWARF 1 as Clang does not offer a
-gdwarf-1
flag2 andllc
will just generate DWARF 2 with the version set to 13.Since this isn't actually supported (and it's not clear it would be useful anyway), report that DWARF 1 is not supported if it is requested.
Also add a help message to the error saying which versions are supported.
cc #103057
Footnotes
https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html#index-gdwarf ↩
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-gdwarf ↩
https://godbolt.org/z/s85d87n3a ↩