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

[IRGen][DebugInfo] split method declaration and definition. #67077

Merged
merged 1 commit into from
Sep 6, 2023
Merged

[IRGen][DebugInfo] split method declaration and definition. #67077

merged 1 commit into from
Sep 6, 2023

Conversation

DianQK
Copy link
Contributor

@DianQK DianQK commented Jul 1, 2023

When the function is a method, 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. Now Rust is too.

Related discussions:

Fixes #53940.

@DianQK
Copy link
Contributor Author

DianQK commented Jul 22, 2023

@swift-ci please test

@DianQK
Copy link
Contributor Author

DianQK commented Jul 27, 2023

@adrian-prantl Could you help me invoke swift-ci and merge changes? This is my first pull request.

@adrian-prantl
Copy link
Contributor

@swift-ci test and merge

2 similar comments
@adrian-prantl
Copy link
Contributor

@swift-ci test and merge

@adrian-prantl
Copy link
Contributor

@swift-ci test and merge

@DianQK
Copy link
Contributor Author

DianQK commented Jul 28, 2023

After adding the parameter --param backtracing, I can reproduce the test/Backtracing/Crash.swift error. Trying to solve it.

@DianQK DianQK requested a review from al45tair as a code owner July 28, 2023 15:53
@DianQK
Copy link
Contributor Author

DianQK commented Jul 28, 2023

After this PR, a DWARF in the following format may be generated:

0x00000d58:       DW_TAG_subprogram
                    DW_AT_linkage_name	("$s8CrashOpt0A0V5$mainyyFZ")
                    DW_AT_name	("$main")
                    DW_AT_type	(0x00000f65 "$sytD")
                    DW_AT_declaration	(true)
                    DW_AT_external	(true)
                    DW_AT_APPLE_optimized	(true)
...
0x00000d9d:         DW_TAG_inlined_subroutine
                      DW_AT_abstract_origin	(0x000011d2 "$s8CrashOpt0A0V5$mainyyFZ")
                      DW_AT_ranges	(0x00000210
                         [0x00000000000011e0, 0x00000000000012f3)
                         [0x00000000000012fb, 0x0000000000001307))
                      DW_AT_call_file	("/home/dianqk/Projects/swift-project/swift/test/Backtracing/Crash.swift")
                      DW_AT_call_line	(0)
...
0x000011d2:   DW_TAG_subprogram
                DW_AT_specification	(0x00000d58 "$s8CrashOpt0A0V5$mainyyFZ")
                DW_AT_inline	(DW_INL_inlined)

I put b00d1520f89bb7df1d2a3eeee57cae7682755634 to this PR to show on CI that this did work.

Opening a new PR should be a better option. #67592

This was referenced Jul 28, 2023
Merged
Closed
@DianQK
Copy link
Contributor Author

DianQK commented Jul 29, 2023

I also tried to solve the failure of lang/swift/async/stepping/step-in/task-switch/TestSwiftTaskSwitch.py on macOS.
The swiftlang/llvm-project#7114 is a change that can pass the test case. But I have no lldb related knowledge.
This might be something to ask @kastiglione to take a look at, thanks.

@al45tair
Copy link
Contributor

I've reviewed the backtracing change in #67592. TL/DR: It looks fine, though I think I'd like it to check the tag to make sure the DW_AT_specification is pointing at the kind of thing we expect.

al45tair
al45tair approved these changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, modulo my comment about checking the tag.

@al45tair
Copy link
Contributor

al45tair commented Aug 1, 2023

@swift-ci Please test

@DianQK
Copy link
Contributor Author

DianQK commented Aug 5, 2023

I figured out why the TestSwiftTaskSwitch.py test case was failing. I tried to submit the patch https://reviews.llvm.org/D157184.

@DianQK
[IRGen][DebugInfo] Split method declaration and definition.
81953ef 
When the function is a method, we want a DW_AT_declaration there.
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 when
doing LTO builds.
@DianQK
Copy link
Contributor Author

DianQK commented Aug 6, 2023

Rebase and update code comments.

@adrian-prantl
Copy link
Contributor

@swift-ci test

@DianQK DianQK mentioned this pull request Sep 1, 2023
Merged
@felipepiovezan
Copy link
Contributor

@swift-ci test

@felipepiovezan felipepiovezan merged commit 2c3c3c1 into swiftlang:main Sep 6, 2023
@DianQK DianQK deleted the type-decl-disubprogram branch September 6, 2023 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@al45tair al45tair al45tair approved these changes

@adrian-prantl adrian-prantl adrian-prantl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SR-11539] llvm-link: dsymutil prints warning: could not find referenced DIE
4 participants
@DianQK @adrian-prantl @al45tair @felipepiovezan