Fix swift async frames unwinding unwinding #9025
Conversation
|
|
There was a problem hiding this comment.
lldb functions start with an Upper case letter.
There was a problem hiding this comment.
Fixed in the new version
felipepiovezan
force-pushed
the
felipe/unwinding_pc_patches_to_release
branch
from
9964bc1 to
317e4a6
Compare
There was a problem hiding this comment.
I don't understand this comment. Yes, it's 128-8, but sleb128 is a variable-length encoding, so I'm confused.
There was a problem hiding this comment.
I also understand this is not new in your patch.
There was a problem hiding this comment.
Can you elaborate? -8 is 0x78 in sleb128 encoding, and it takes a single byte to represent it.
The comment is explaining where the magic 0x78 comes from.
There was a problem hiding this comment.
This should be a doxygen comment on the declaration.
There was a problem hiding this comment.
is there a generic name for x22 (since this is arch specific?)
There was a problem hiding this comment.
context_reg or some thing like that?
There was a problem hiding this comment.
calling is "async_reg" to match other comments in this file which refer to it as "asynchronous register"
There was a problem hiding this comment.
We should comment that this is true on all ABIs, otherwise this screams for being an ABI callback.
There was a problem hiding this comment.
Can you add the missing detail to this comment that explains why the indirection follows from it being a virtual frame?
There was a problem hiding this comment.
I've reworded this entire paragraph to remove this part of the explanation that requires a lot of background information.
There was a problem hiding this comment.
Target &target = process.GetTarget(); ?
felipepiovezan
force-pushed
the
felipe/unwinding_pc_patches_to_release
branch
from
317e4a6 to
1aa6a85
Compare
felipepiovezan
left a comment
felipepiovezan
left a comment
There was a problem hiding this comment.
Address review comments
There was a problem hiding this comment.
Can you elaborate? -8 is 0x78 in sleb128 encoding, and it takes a single byte to represent it.
The comment is explaining where the magic 0x78 comes from.
There was a problem hiding this comment.
calling is "async_reg" to match other comments in this file which refer to it as "asynchronous register"
There was a problem hiding this comment.
I've reworded this entire paragraph to remove this part of the explanation that requires a lot of background information.
This will enable subsequent commits to introduce calls to other member functions from that method.
felipepiovezan
force-pushed
the
felipe/unwinding_pc_patches_to_release
branch
from
1aa6a85 to
8e0cfd2
Compare
felipepiovezan
changed the base branch from
stable/20230725
to
swift/release/6.0
|
Removed the last two commits, which dealt with Q funclets as frame 0 and required compiler changes. |
|
@swift-ci test |
|
|
||
| // Compute the CFA of this frame. | ||
| addr_t cfa = async_reg_val; | ||
| while (num_indirections--) { |
There was a problem hiding this comment.
The fact that this underflows on the last iteration bothers me a bit.
for (; num_indirections != 0; --num_indirections) ?
|
|
||
| func ASYNC___2___() async -> Int { | ||
| var myvar = 222; | ||
| let result = await ASYNC___1___() //breakpoint1 |
Variables are never available in the prologue of a function. By unwinding the pc of the virtual frames as the first instruction of the continuation function, LLDB is unable to display any variable information. This patch addresses the issue by computing the continuation pointer from the context of the function being unwound, and then querying the continuation's SymbolContext to determine its prologue size.
felipepiovezan
force-pushed
the
felipe/unwinding_pc_patches_to_release
branch
from
8e0cfd2 to
85069c8
Compare
felipepiovezan
left a comment
felipepiovezan
left a comment
There was a problem hiding this comment.
Addressed review comments
|
|
||
| func ASYNC___2___() async -> Int { | ||
| var myvar = 222; | ||
| let result = await ASYNC___1___() //breakpoint1 |
|
|
||
| // Compute the CFA of this frame. | ||
| addr_t cfa = async_reg_val; | ||
| while (num_indirections--) { |
|
@swift-ci test |
|
@swift-ci test platform windows |
|
@swift-ci test windows platform |
|
@swift-ci test windows platform |
3 participants
This series of patches aims to solve the problem where LLDB is unable to print variables in backtraces involving swift async functions. The only frame where this works today is the first async frame, which is a real frame and is physically on the stack. We fail to do so for all virtual frames.
The underlying problem is simple: when unwinding the PC, we use the first instruction of the continuation funclets. This instruction is part of the prologue, where no variables are in scope.
The solution is similarly simple: we just offset the PC by the size of the prologue of the continuation funclet.
These commits are meant to be reviewed independently, please the commit messages too.
The first commit is a cherry-pick from: llvm#100624