llvm-project
llvm-project copied to clipboard
apple
[lldb][swift] Filter await-resume funclets when setting breakpoints
These funclets only serve to task_dealloc previously allocated tasks when returning from an async call, and immediately task_switch to the next await-suspend funclet (which contains real user code).
By not filtering out these funclets, any breakpoint on a line with an async call will cause execution to pause 3 times: once before the call, twice when "returning" from the call, which makes for a confusing experience.
The patch does the filtering on BreakpointResolver::SetSCMatchesByLine, which is the common code between BreakpointResolverFileLine and BreakpointResolverFileRegex.
We also considered changing the debug line information in any of the many different lowering stages the swift compiler, but this turned out to be very complex to do in a targeted way; more often than not, a handful of early-IR coroutine instructions get expanded into multiple function clones, all inheriting the same debug line information. The current approach also has the advantaged of being easily reversible if we decide to do so.
I'm very open to other ways of accomplishing this, in case anyone has suggestions
Filtering them in the File & Line resolver seems fine to me.
However, there needs to be a virtual Language::RemoveInvalidSourceLineMatches or something like that which does nothing. Then your function needs to implement that in the Swift Language. I'm not wedded to the name, but you have to make this generic. We really want to keep from adding more new #ifdef LLDB_ENABLE_SWIFT in generic files.
You should probably also do the generic stuff on the llvm.org version, cherry-pick that and add your implementation so we don't end up with more diffs on the swift side.
You should probably also do the generic stuff on the llvm.org version, cherry-pick that and add your implementation so we don't end up with more diffs on the swift side.
I actually had that on my first iteration of this patch, but then realized we would be putting virtual methods with no real implementation upstream, which is generally something that sees push back against (it would basically be dead code, since Swift is not an upstreamed language). If you believe this won't be a problem, I can go the language route (still need to figure out if we have access to a Language object from this point)
This is a general feature, where the compiler is emitting some debug information that for its own internal reasons it wishes it could manage not to emit, but can't. OTOH, the debugger after the fact can. There's nothing swift specific about that, except currently the swift compiler is the only one that does it. So I don't think it will be an issue putting in an API here - and that's much preferable to more #ifdef LLDB_ENABLE_SWIFT.
The breakpoints offer an option to filter by language, so you should be able to get your hands on the language of the function you are setting the breakpoint in.
Sounds good, I will convert this to a draft while I propose the first commit of this PR upstream
I've re-implemented the swift side of things with the suggestions and marked this patch as ready for review.
Note that the first commit here is identical to https://github.com/llvm/llvm-project/pull/83908/files, and I'll convert it to an actual cherry-pick once it gets merged upstream (won't merge this PR until that's done)
@adrian-prantl I've addressed your comments on the upstream pr https://github.com/llvm/llvm-project/pull/83908