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

Pick the max DWARF version when LTO'ing modules with different versions #136659

Merged
merged 2 commits into from
Feb 9, 2025
Merged

Pick the max DWARF version when LTO'ing modules with different versions #136659

merged 2 commits into from
Feb 9, 2025
+49 −1

Conversation

wesleywiser
Copy link
Member

@wesleywiser wesleywiser commented Feb 6, 2025 •
edited
Loading

Currently, when rustc compiles code with -Clto enabled that was built
with different choices for -Zdwarf-version, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying -Clto -Zdwarf-version=5 since the
standard library is distributed with -Zdwarf-version=4.

This behavior isn't actually useful for a few reasons:

  • From observation, LLVM chooses to pick the highest DWARF version
    anyway after issuing the warning.
  • Clang specifies that in this case, the max version should be picked
    without a warning and as a general principle, we want to support
    x-lang LTO with Clang which implies using the same module flag merge
    behaviors.
  • Debuggers need to be able to handle a variety of versions within the
    same debugging session as you can easily have some parts of a binary
    (or some dynamic libraries within an application) all compiled with
    different DWARF versions.

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
adds a test to ensure no warning is printed.

Fixes #130041 which fails due to these warnings being printed

cc #103057

@rustbot
Copy link
Collaborator

rustbot commented Feb 6, 2025

r? @Noratrieb

rustbot has assigned @Noratrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Feb 6, 2025
@jieyouxu jieyouxu added the A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) label Feb 7, 2025
jieyouxu
jieyouxu approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, I think this behavior is strictly more sane.

@jieyouxu
Copy link
Member

jieyouxu commented Feb 7, 2025

r? jieyouxu @bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 7, 2025

📌 Commit 12fdab0 has been approved by jieyouxu

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 Feb 7, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2025
@matthiaskrgr
Rollup merge of rust-lang#136659 - wesleywiser:dwarf_version_lto_merg…
ae905d4 
…e_behavior, r=jieyouxu

Pick the max DWARF version when LTO'ing modules with different versions

Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- From observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning.
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors.
- Debuggers need to be able to handle a variety of versions within the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions.

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
adds a test to ensure no warning is printed.

Fixes rust-lang#130041 which fails due to these warnings being printed

cc rust-lang#103057
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 7, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2025
@bors
Auto merge of rust-lang#136712 - matthiaskrgr:rollup-fd5sw3d, r=matth…
d032cf0 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#136640 (Debuginfo for function ZSTs should have alignment of 8 bits, not 1 bit)
 - rust-lang#136648 (Add a missing `//@ needs-symlink` to `tests/run-make/libs-through-symlinks`)
 - rust-lang#136651 (Label mismatched parameters at the def site for foreign functions)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )
 - rust-lang#136691 (Remove Linkage::Private and Linkage::Appending)
 - rust-lang#136692 (add module level doc for bootstrap:utils:exec)
 - rust-lang#136700 (i686-unknown-hurd-gnu: bump baseline CPU to Pentium 4)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#136712 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 8, 2025
@wesleywiser
Add tests for -Zdwarf-version lto behavior
cd3dd5b
@wesleywiser
Pick the max DWARF version when LTO'ing modules with different versions
bbc40e7 
Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- from observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors
- Debuggers need to be able to handle a variety of versions withing the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
updates the test added in the previous commit to ensure no warning is
printed.
@wesleywiser wesleywiser force-pushed the dwarf_version_lto_merge_behavior branch from 12fdab0 to bbc40e7 Compare February 8, 2025 16:33
jieyouxu
jieyouxu approved these changes Feb 8, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks

@jieyouxu
Copy link
Member

jieyouxu commented Feb 8, 2025

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Feb 8, 2025

📌 Commit bbc40e7 has been approved by jieyouxu

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 8, 2025
Urgau added a commit to Urgau/rust that referenced this pull request Feb 8, 2025
@Urgau
Rollup merge of rust-lang#136659 - wesleywiser:dwarf_version_lto_merg…
1b81399 
…e_behavior, r=jieyouxu

Pick the max DWARF version when LTO'ing modules with different versions

Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- From observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning.
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors.
- Debuggers need to be able to handle a variety of versions within the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions.

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
adds a test to ensure no warning is printed.

Fixes rust-lang#130041 which fails due to these warnings being printed

cc rust-lang#103057
@Urgau Urgau mentioned this pull request Feb 8, 2025
Closed
@Urgau Urgau mentioned this pull request Feb 8, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
@bors
Auto merge of rust-lang#136754 - Urgau:rollup-qlkhjqr, r=Urgau
0edbadb 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#136213 (Allow Rust to use a number of libc filesystem calls)
 - rust-lang#136530 (Implement `x perf` directly in bootstrap)
 - rust-lang#136601 (Detect (non-raw) borrows of null ZST pointers in CheckNull)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
@bors
Auto merge of rust-lang#136754 - Urgau:rollup-qlkhjqr, r=Urgau
cd6ca8e 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#136213 (Allow Rust to use a number of libc filesystem calls)
 - rust-lang#136530 (Implement `x perf` directly in bootstrap)
 - rust-lang#136601 (Detect (non-raw) borrows of null ZST pointers in CheckNull)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
@bors
Auto merge of rust-lang#136754 - Urgau:rollup-qlkhjqr, r=Urgau
a26e97b 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#136213 (Allow Rust to use a number of libc filesystem calls)
 - rust-lang#136530 (Implement `x perf` directly in bootstrap)
 - rust-lang#136601 (Detect (non-raw) borrows of null ZST pointers in CheckNull)
 - rust-lang#136659 (Pick the max DWARF version when LTO'ing modules with different versions )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5ec56e5 into rust-lang:master Feb 9, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2025
@rust-timer
Unrolled build for rust-lang#136659
7919ff8 
Rollup merge of rust-lang#136659 - wesleywiser:dwarf_version_lto_merge_behavior, r=jieyouxu

Pick the max DWARF version when LTO'ing modules with different versions

Currently, when rustc compiles code with `-Clto` enabled that was built
with different choices for `-Zdwarf-version`, a warning will be
reported. It's very easy to observe this by compiling most anything (eg,
"hello world") and specifying `-Clto -Zdwarf-version=5` since the
standard library is distributed with `-Zdwarf-version=4`.

This behavior isn't actually useful for a few reasons:
- From observation, LLVM chooses to pick the highest DWARF version
  anyway after issuing the warning.
- Clang specifies that in this case, the max version should be picked
  without a warning and as a general principle, we want to support
  x-lang LTO with Clang which implies using the same module flag merge
  behaviors.
- Debuggers need to be able to handle a variety of versions within the
  same debugging session as you can easily have some parts of a binary
  (or some dynamic libraries within an application) all compiled with
  different DWARF versions.

This commit changes the module flag merge behavior to match Clang and
use the highest version of DWARF. It also adds a test to ensure this
behavior is respected in the case of two crates being LTO'd together and
adds a test to ensure no warning is printed.

Fixes rust-lang#130041 which fails due to these warnings being printed

cc rust-lang#103057
@rustbot rustbot added this to the 1.86.0 milestone Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) 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.86.0
Development

Successfully merging this pull request may close these issues.

Building with -Z dwarf-version=5 causes lto debuginfo test failures
6 participants
@wesleywiser @rustbot @jieyouxu @bors @matthiaskrgr @Noratrieb