Page MenuHomePhabricator

Add a callout to the LanguageRuntime to override the normal UnwindPlan used for a frame
ClosedPublic

Authored by jasonmolenda on Feb 16 2021, 10:54 PM.

Details

Summary

Swift is gaining asynchronous functions, which may not be executing when a function they call is executing. In the backtrace, we want to show the logical caller of this currently-executing function even though it is not present on the stack like a real frame. The LanguageRuntime can provide an UnwindPlan that will provide that function.

When the async function is actually executing, we'll use its normal UnwindPlans. But when it is not executing, we'll use the LanguageRuntime's UnwindPlan to show it in the stack.

The way this is actually implemented by the swift runtime is that the stack has a chain of these async-not-running functions and pointers to a data block with saved context for them, and as we walk the chain we will get back to real stack functions again. The swift language runtime uses a flag in the spilled fp register to tell when it should provide its special unwind plan.

This patch adds code to LanguageRuntime to see if any LanguageRuntime plugins can provide an UnwindPlan given a RegisterContext. It adds code to RegisterContextUnwind to see if the LanguageRuntime can provide an UnwindPlan before we start looking at the function's normal UnwindPlans, and uses the async unwind plan if it was provided. Pretty simple hooks out to the LanguageRuntime where the details are encapsulated.

Without interjecting a LanguageRuntime plugin in a live debug session, this isn't testable in standard lldb; in the swift branch of lldb I can have an API test that has some async calls on the stack and do a stack walk. But I need the LanguageRuntimeSwift support for one of these to get that to work.

Open to any comments or suggestions, not sure if anyone is really into the unwinder enough to spend time reviewing it.

Diff Detail

Event Timeline

jasonmolenda created this revision.Feb 16 2021, 10:54 PM
jasonmolenda requested review of this revision.Feb 16 2021, 10:54 PM
kastiglione accepted this revision.Feb 17 2021, 8:15 AM

"Asynced" sounds funny to me, but I don't have suggestions other than "Async".

Looks good to me.

lldb/source/Target/RegisterContextUnwind.cpp
556

Looks like active_row will always be valid here, as the code is currently constructed.

This revision is now accepted and ready to land.Feb 17 2021, 8:15 AM
JDevlieghere added inline comments.Feb 17 2021, 8:56 AM
lldb/include/lldb/Target/LanguageRuntime.h
176

Please use /// so this becomes a Doxygen comment.

186

s/Asynced/Async/ maybe?

lldb/source/Target/RegisterContextUnwind.cpp
294

nit: This sounds like is supposed to be generic, I'd drop the Swift here, you already gave that as an example above.

jingham added inline comments.
lldb/include/lldb/Target/LanguageRuntime.h
186

Maybe "Pending"? From the standpoint of the API, it doesn't really matter why there are some not-currently executing stacks, just that they are not currently executing. Pending describes the state, not the why, so might be more appropriate.

jasonmolenda added inline comments.Feb 17 2021, 12:09 PM
lldb/include/lldb/Target/LanguageRuntime.h
186

Thanks for the comments all. Yeah the wording Asynced is a little weird, but I was trying to get across the idea that a function is executing normally on the stack, then it is descheduled (or the function it calls is executed on another thread) and it has been async'ified from its previous running state, and will later revivify. But it's probably more trouble than it's worth and I should just use Async here.

Can each real stack frame inject N frames in the middle of a stack frame for these asynchronous functions? Or is this like the libdispatch stuff that show more frames at the end of a normal stack trace? Can you attach some sample backtraces with some annotations?

lldb/include/lldb/Target/LanguageRuntime.h
21

I don't think we need this in the header file as we only use pointers to a RegisterContext. So move this to the .cpp file?

187

Any reason we are passing in the register context and the thread instead of a stack frame? a stack frame has access to the register context and to the thread it belongs to. Or is this register context just a raw register context that was extracted from somewhere in memory?

192

What does "ThisLanguage" this mean? Is it the language of the stack frame that owns "regctx"? Should we pass in the current stack frame instead of a register context?

jasonmolenda added inline comments.Feb 17 2021, 1:56 PM
lldb/include/lldb/Target/LanguageRuntime.h
187

I'll rethink this though, but I don't think we have a StackFrame yet. We're constructing the register context and we'll use that in creating the stack frame in a bit. I might be off-by-one, these parts of the unwinder can take some mental gymnastics to model properly when you're changing things.

192

Yeah it's not a great method name. The LanguageRuntime base class has a list of registered LanguageRuntime plugins. I want to iterate over them, seeing if any LanguageRuntime can provide one of these special one-off UnwindPlans. Tbh the top-level method GetAsyncedFrameForThisLanguage could have gone in Process and it could call a method of the same name in all the LanguageRuntime plugins, would end up working the same.

This revision was landed with ongoing or failed builds.Feb 18 2021, 11:23 PM
This revision was automatically updated to reflect the committed changes.