Work around invalid DWARF bugs for fat LTO #89041

sticnarf · 2021-09-17T12:25:20Z

This PR applies the same workaround in #46772 to fat LTO.

It seems to fix the bug reported in #66118 (comment).

rust-highfive · 2021-09-17T12:25:23Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information.

oli-obk · 2021-09-17T14:12:12Z

r? rust-lang/wg-llvm

oli-obk · 2021-09-17T14:12:47Z

r? @rust-lang/wg-llvm can one of you take this?

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

nagisa · 2021-09-17T15:36:18Z

r? @nagisa

nagisa · 2021-09-17T16:15:55Z

Can you please add a regression test? If its too hard then its okay to r=me this.

sticnarf · 2021-09-17T16:45:13Z

Can you please add a regression test? If its too hard then its okay to r=me this.

Sorry but now I don't know how to construct a minimized case for reproducing the issue from Rust code (as you see the code I posted in the issue page has multiple dependencies, and the issue disappears after removing any of them). So I'm afraid it's really too hard for me to add a regression test for it.

nagisa · 2021-09-18T21:04:13Z

@bors r+

bors · 2021-09-18T21:04:14Z

📌 Commit d5de680 has been approved by nagisa

…nagisa Work around invalid DWARF bugs for fat LTO This PR applies the same workaround in rust-lang#46772 to fat LTO. It seems to fix the bug reported in rust-lang#66118 (comment).

Rollup of 8 pull requests Successful merges: - rust-lang#89036 (Fix missing `no_global_oom_handling` cfg-gating) - rust-lang#89041 (Work around invalid DWARF bugs for fat LTO) - rust-lang#89046 ("Fix" an overflow in byte position math) - rust-lang#89127 (Re-enable the `src/test/debuginfo/mutex.rs` test on Windows) - rust-lang#89133 (Fix ICE with `--cap-lints=allow` and `-Zfuel=...=0`) - rust-lang#89162 (rustc_index: Add some map-like APIs to `IndexVec`) - rust-lang#89164 (Document `--show-type-layout` in the rustdoc book) - rust-lang#89170 (Disable the leak sanitizer on Macos aarch64 for now) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

philipc · 2021-09-23T01:59:37Z

compiler/rustc_codegen_llvm/src/back/lto.rs

+        let mut cu1 = ptr::null_mut();
+        let mut cu2 = ptr::null_mut();
+        unsafe { llvm::LLVMRustLTOGetDICompileUnit(llmod, &mut cu1, &mut cu2) };
+        if !cu2.is_null() {


The Thin LTO use of this seems to have the opposite check? It only proceeds if cu2 is nul.

Oops... I didn't mean to do the opposite, actually.

However, after the patch, rustc seems to generate valid dwarf for binaries (such as ripgrep) while using full LTO. So I truly misunderstood something.

I am not sure how much missing debug info it will increase. But at least Linux perf does not show unknown userspace addresses in my practice...

I tried it out. It seems to be doing what I expect (puts everything in one huge CU), and this check makes sense to me (it's only worth trying to combine CUs if there is more than one CU), so it's probably just the comment that is wrong.

sticnarf · 2021-11-05T08:33:54Z

It seems quite some regressions are reported due to this PR. Better to revert this immature workaround.

cbiffle · 2022-04-04T17:50:50Z

It seems quite some regressions are reported due to this PR. Better to revert this immature workaround.

Can't really tell from the github output here, did this happen? This change without regression tests has been suggested as the culprit for broken DWARF output for static vars in LTO'd binaries over in #90357.

If this is still in-tree we can try reverting it on our side, I suppose.

Since September, the toolchain has not been generating reliable DWARF information for static variables when LTO is on. This has affected projects in the embedded space where the use of LTO is typical. In our case, it has kept us from bumping past the 2021-09-22 nightly toolchain lest our debugger break. This has been a pretty dramatic regression for people using debuggers and static variables. See rust-lang#90357 for more info and a repro case. This commit is a mechanical revert of d5de680 from PR rust-lang#89041, which caused the issue. (Note on that PR that the commit's author has requested it be reverted.) I have locally verified that this fixes rust-lang#90357 by restoring the functionality of both the repro case I posted on that bug, and debugger behavior on real programs. There do not appear to be test cases for this in the toolchain; if I've missed them, point me at 'em and I'll update them.

… r=pnkfelix Revert "Work around invalid DWARF bugs for fat LTO" Since September, the toolchain has not been generating reliable DWARF information for static variables when LTO is on. This has affected projects in the embedded space where the use of LTO is typical. In our case, it has kept us from bumping past the 2021-09-22 nightly toolchain lest our debugger break. This has been a pretty dramatic regression for people using debuggers and static variables. See rust-lang#90357 for more info and a repro case. This commit is a mechanical revert of d5de680 from PR rust-lang#89041, which caused the issue. (Note on that PR that the commit's author has requested it be reverted.) I have locally verified that this fixes rust-lang#90357 by restoring the functionality of both the repro case I posted on that bug, and debugger behavior on real programs. There do not appear to be test cases for this in the toolchain; if I've missed them, point me at 'em and I'll update them.

…=pnkfelix Revert "Work around invalid DWARF bugs for fat LTO" Since September, the toolchain has not been generating reliable DWARF information for static variables when LTO is on. This has affected projects in the embedded space where the use of LTO is typical. In our case, it has kept us from bumping past the 2021-09-22 nightly toolchain lest our debugger break. This has been a pretty dramatic regression for people using debuggers and static variables. See rust-lang#90357 for more info and a repro case. This commit is a mechanical revert of d5de680 from PR rust-lang#89041, which caused the issue. (Note on that PR that the commit's author has requested it be reverted.) I have locally verified that this fixes rust-lang#90357 by restoring the functionality of both the repro case I posted on that bug, and debugger behavior on real programs. There do not appear to be test cases for this in the toolchain; if I've missed them, point me at 'em and I'll update them.

rust-highfive assigned oli-obk Sep 17, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 17, 2021

This comment has been minimized.

Sign in to view

sticnarf force-pushed the sticnarf/fat-lto-dwarf branch from 594ac57 to ab02351 Compare September 17, 2021 13:02

This comment has been minimized.

Sign in to view

sticnarf force-pushed the sticnarf/fat-lto-dwarf branch 2 times, most recently from 1ee8d6f to 379a661 Compare September 17, 2021 13:14

Work around invalid DWARF bugs for fat LTO

d5de680

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

sticnarf force-pushed the sticnarf/fat-lto-dwarf branch from 379a661 to d5de680 Compare September 17, 2021 15:20

rust-highfive assigned nagisa and unassigned oli-obk Sep 17, 2021

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 Sep 18, 2021

the8472 mentioned this pull request Sep 22, 2021

Rollup of 7 pull requests #89172

Closed

the8472 mentioned this pull request Sep 22, 2021

Rollup of 8 pull requests #89179

Merged

bors merged commit 1deef1f into rust-lang:master Sep 22, 2021

rustbot added this to the 1.57.0 milestone Sep 22, 2021

philipc reviewed Sep 23, 2021

View reviewed changes

philipc mentioned this pull request Sep 23, 2021

[DWARF] Crate Debug Info incomplete? #88521

Closed

glandium mentioned this pull request Oct 19, 2021

DICompileUnit not listed in llvm.dbg.cu #90045

Closed

luqmana mentioned this pull request Oct 28, 2021

DWARF info for static vars in lib crates has stopped being produced reliably in LTO builds #90357

Closed

nabijaczleweli mentioned this pull request Nov 16, 2021

Revert "Cargo.toml: lto = "thin"" to = true systemd/zram-generator#131

Merged

cbiffle mentioned this pull request Apr 5, 2022

Revert "Work around invalid DWARF bugs for fat LTO" #95685

Merged

jieyouxu mentioned this pull request Aug 14, 2024

Tracking Issue for Reproducible Build bugs and challenges #129080

Open

81 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Work around invalid DWARF bugs for fat LTO #89041

Work around invalid DWARF bugs for fat LTO #89041

sticnarf commented Sep 17, 2021

rust-highfive commented Sep 17, 2021

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 17, 2021

oli-obk commented Sep 17, 2021

nagisa commented Sep 17, 2021

nagisa commented Sep 17, 2021 •

edited

Loading

sticnarf commented Sep 17, 2021

nagisa commented Sep 18, 2021

bors commented Sep 18, 2021

philipc Sep 23, 2021

sticnarf Sep 23, 2021

philipc Sep 23, 2021

sticnarf commented Nov 5, 2021

cbiffle commented Apr 4, 2022

Work around invalid DWARF bugs for fat LTO #89041

Work around invalid DWARF bugs for fat LTO #89041

Conversation

sticnarf commented Sep 17, 2021

rust-highfive commented Sep 17, 2021

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 17, 2021

oli-obk commented Sep 17, 2021

nagisa commented Sep 17, 2021

nagisa commented Sep 17, 2021 • edited Loading

sticnarf commented Sep 17, 2021

nagisa commented Sep 18, 2021

bors commented Sep 18, 2021

philipc Sep 23, 2021

Choose a reason for hiding this comment

sticnarf Sep 23, 2021

Choose a reason for hiding this comment

philipc Sep 23, 2021

Choose a reason for hiding this comment

sticnarf commented Nov 5, 2021

cbiffle commented Apr 4, 2022

nagisa commented Sep 17, 2021 •

edited

Loading