-
Notifications
You must be signed in to change notification settings - Fork 341
[lldb][swift] Prefer assembly unwind plans for async registers #9912
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
[lldb][swift] Prefer assembly unwind plans for async registers #9912
Conversation
var arr: [Int] = [] | ||
|
||
func ASYNC___1___() async -> Int { | ||
var a1 = 1, a2 = 2, a3 = 3, a4 = 4, a5 = 5 // BREAK HERE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var a1 = 1, a2 = 2, a3 = 3, a4 = 4, a5 = 5 // BREAK HERE | |
var a1 = 1, a2 = 2, a3 = 3, a4 = 4, a5 = 5 | |
print("BREAK HERE") |
Otherwise it's kind of ambiguous where exactly the breakpoint will be because the above line can result in vastly different code depending on the target.
5b78c0d
to
0053c67
Compare
Addressed feedback |
@swift-ci test |
LLDB assumes all functions are using the AAPCS64 ABI with two specific implementation details on Darwin, v. https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#machine-registers It is possible for a function to be marked to use a different ABI if it has DWARF via DW_AT_calling_convention, but of course you'd need to have debug info to recognize this, and lldb needs to operate correctly without that for all functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, I'm not positive that the test case will always result in an eh_frame unwind plan - I haven't looked at what eh_frame is generated for this function, but it seems like compact unwind can probably represent the unwind state. Maybe compact unwind isn't emitted because it's an alternate calling convention, I'm not sure.
It boils down to the specifics of the assembly generated. Right now, that example does cause the compiler to generate eh_frame; but, for example, if you reduce the size of that array, it will emit compact unwind instead. Sadly this is a best effort, just like having enough register pressure that it clobber x22 :/ |
The eh_frame section only describes registers that are callee saved; in other words, registers whose values will be restored before the function returns. Under this definition, the async register is not callee saved for the purposes of the swifttailcc ABI. Why? Because it's impossible to do so. When a funclet is about to tail call into something else, it will reuse the async register for the argument of the next funclet being tail called. However, the async register is guaranteed to be written to a specific stack slot as part of the frame formation; it is guaranteed to be written, *but not guaranteed to be restored*, as per the argument above. Because of this, we should not rely on a mechanism meant to describe registers that are restored (eh_frame). LLDB does not know about calling conventions, it just assumes they all follow the ARM calling convention as closely as possible. As such, when it tries to read information about the async register on eh_frame and finds no entry for that register, LLDB makes the assumption that "this register was not modified", which is correct for the "normal" ABI, but not for swifttailcc. The compiler did not add information for the async register in eh_frame because it is not a callee saved register. To address this, this commit makes a very targeted change in SwiftLanguageRuntime, preferring the assembly plans every time it attempts to recover the async register. This is a much smaller change than it looks: we're already using this same plan for the vast majority of cases, since compact unwinding is not valid at frame 0 and eh_frame is hardly ever produced in Apple platforms.
0053c67
to
486a032
Compare
@swift-ci test |
Turns out that codegen for x86 does indeed place a LOT of instructions before the first real debug location in the fuction, so my threshold of "make sure we run at least 90%" of the instructions was not lenient enough. |
The eh_frame section only describes registers that are callee saved; in other words, registers whose values will be restored before the function returns.
Under this definition, the async register is not callee saved for the purposes of the swifttailcc ABI. Why? Because it's impossible to do so. When a funclet is about to tail call into something else, it will reuse the async register for the argument of the next funclet being tail called.
However, the async register is guaranteed to be written to a specific stack slot as part of the frame formation; it is guaranteed to be written, but not guaranteed to be restored, as per the argument above. Because of this, we should not rely on a mechanism meant to describe registers that are restored (eh_frame).
LLDB does not know about calling conventions, it just assumes they all follow the ARM calling convention as closely as possible. As such, when it tries to read information about the async register on eh_frame and finds no entry for that register, LLDB makes the assumption that "this register was not modified", which is correct for the "normal" ABI, but not for swifttailcc. The compiler did not add information for the async register in eh_frame because it is not a callee saved register.
To address this, this commit makes a very targeted change in SwiftLanguageRuntime, preferring the assembly plans every time it attempts to recover the async register. This is a much smaller change than it looks: we're already using this same plan for the vast majority of cases, since compact unwinding is not valid at frame 0 and eh_frame is hardly ever produced in Apple platforms.
rdar://143399891