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

debuginfo: split method declaration and definition #111167

Merged
merged 1 commit into from
May 6, 2023

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented May 3, 2023

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.

Both GCC and Clang write debuginfo this way for C++ class methods.

Fixes #109730.
Fixes #109934.

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.
@rustbot
Copy link
Collaborator

rustbot commented May 3, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@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 May 3, 2023
@michaelwoerister
Copy link
Member

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Thanks for the PR, @cuviper! The change looks good to me.
@bors r+

@bors
Copy link
Contributor

bors commented May 5, 2023

📌 Commit 10b69dd has been approved by michaelwoerister

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 May 5, 2023
@cuviper
Copy link
Member Author

cuviper commented May 5, 2023

Nominating for 1.70-beta, because in Fedora we ran into the error: Cannot represent a difference across sections building Firefox with Rust 1.69 + LLVM 16, and upstream here we'll be releasing with LLVM 16 in 1.70.0.

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 5, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 6, 2023
…michaelwoerister

debuginfo: split method declaration and definition

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.

Both GCC and Clang write debuginfo this way for C++ class methods.

Fixes rust-lang#109730.
Fixes rust-lang#109934.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 6, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#110577 (Use fulfillment to check `Drop` impl compatibility)
 - rust-lang#110610 (Add Terminator conversion from MIR to SMIR, part #1)
 - rust-lang#110985 (Fix spans in LLVM-generated inline asm errors)
 - rust-lang#110989 (Make the BUG_REPORT_URL configurable by tools )
 - rust-lang#111167 (debuginfo: split method declaration and definition)
 - rust-lang#111230 (add hint for =< as <=)
 - rust-lang#111279 (More robust debug assertions for `Instance::resolve` on built-in traits with non-standard trait items)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f440999 into rust-lang:master May 6, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 6, 2023
@fundawang
Copy link

Will rust 1.69.1 release soon? Because this patch cannot be applied to 1.69.0 tarball without modification.

@cuviper
Copy link
Member Author

cuviper commented May 7, 2023

I wasn't thinking of proposing this for 1.69-stable, since the official release using LLVM 15 didn't suffer that "cannot represent a difference across sections" error. You can use the backport patch I applied in Fedora though:
https://src.fedoraproject.org/rpms/rust/blob/777115da9e2892eb47502c8afcbe2b7742b14879/f/0001-debuginfo-split-method-declaration-and-definition.patch

cuviper added a commit to cuviper/rust that referenced this pull request May 8, 2023
This reverts rust-lang#46722, commit e0ab5d5.

Since rust-lang#111167, commit 10b69dd, we are
generating DWARF subprograms in a way that is meant to be more compatible
with LLVM's expectations, so hopefully we don't need this workaround
rewriting CUs anymore.
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 11, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2023
Remove the ThinLTO CU hack

This reverts rust-lang#46722, commit e0ab5d5.

Since rust-lang#111167, commit 10b69dd, we are
generating DWARF subprograms in a way that is meant to be more compatible
with LLVM's expectations, so hopefully we don't need this workaround
rewriting CUs anymore.
@cuviper cuviper mentioned this pull request May 20, 2023
@cuviper cuviper modified the milestones: 1.71.0, 1.70.0 May 20, 2023
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 20, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2023
[beta] backport

- debuginfo: split method declaration and definition rust-lang#111167
- Encode VariantIdx so we can decode ADT variants in the right order rust-lang#111494
- Simplify find_width_of_character_at_span. rust-lang#111560

r? cuviper
@cuviper cuviper deleted the type-decl-disubprogram branch May 21, 2023 03:49
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2023
…isubprogram, r=cuviper

DebugInfo: Updates test cases that add method declarations.

We've investigated one reason why debugging information often goes wrong at https://reviews.llvm.org/D152095.
> LLVM can't handle IR where subprogram definitions are nested within DICompositeType when doing LTO builds, because there's no good way to cross the CU boundary to insert a nested DISubprogram definition in one CU into a type defined in another CU.

In rust-lang#111167, we added a declaration for the DISubprogram for the method. This PR completes this test case.

stream history: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/Dwarf.20CUs/near/384269475.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.

LTO, alloc and debug compilation problem Cannot represent a difference across sections
7 participants