jimingham wrote: > > I quickly looked at the private parts of the change. I had a slightly > > different design in mind, which I pitched to Ismail in person this morning. > > I created a quick sequence diagram to show it in action. > > Thanks for that updated design! > > Afaict, two of my points (lazy unwinding, script-controlled merging > strategies) are already addressed in the latest version of this PR and we are > also aligned on my third point (global registration) by holding the stack > frame provider in the target. > > With that high-level alignment out of the way, I now wonder about the > detailed design of the interface. If I understand the updated design > correctly, Python authors would now have to implement a > `get_stack_frames(real_threads, provided_frames, stack_depth)` function. Or > is that only an internal implementation detail on the C++ level, and we would > expose this differently to the Python layer? > > I think a generator-based Python interface would be more user-friendly, as > shown by comparing the following two examples (loosely based on the [C++ > coroutine unwinding > script](https://clang.llvm.org/docs/DebuggingCoroutines.html#gdb-debugger-script)): > > **generator / `yield`-based** > > ```python > def _create_coroutine_frames(coro_frame): > while coro_frame is not None: > yield CoroutineFrame(coro_frame) > coro_frame = _get_continuation(coro_frame.promise_ptr) > > class CppCoroutineFrameProvider: > def get_stack_frames(self, input_frames): > for frame in input_frames: > # forward physical frame > yield frame > # Check if we are in a coroutine by looking for `__promise` > try: > promise_ptr = frame.read_var("__promise") > except Exception: > continue > parent_coro = _get_continuation(promise_ptr) > # Add all the async stack frames > if parent_coro is not None: > yield from _create_coroutine_frames(parent_coro) > ``` > > **index-based** > > Two downsides: > > 1. quite a bit of manual book-keeping around `frame_idx` > 2. I don't have a way to store state between `get_stack_frames` calls and as > such I have to back off until the previous physical frame and redo some stack > frames from there > > ```python > def _create_coroutine_frames(coro_frame, output_frames, frame_idx, > frame_count): > while coro_frame is not None and frame_idx < frame_count: > output_frames.SetFrameAt(frame_idx, CoroutineFrame(coro_frame, > inferior_frame)) > coro_frame = _get_continuation(coro_frame.promise_ptr) > return frame_idx > > class CppCoroutineFrameProvider: > def get_stack_frames(self, input_frames, output_frames, frame_count): > # Don't redo the work for frames which we already unrolled > frame_idx = len(output_frames) > # Backtrack until the last physical frame, as we cannot not store the > # `coro_frame ` intermediate state and hence can't resume in the > middle > # of `_create_coroutine_frames` > while not output_frames.GetFrameAt(frame_idx).IsPhysicalThread(): > --frame_idx > > while frame_idx < frame_count > # forward physical frame > output_frames.SetFrameAt(frame_idx, > input_frames.GetFrameAt(start_frame_idx) > frame_idx += 1; > # Check if we are in a coroutine by looking for `__promise` > inferior_frame = frame.inferior_frame() > try: > promise_ptr = inferior_frame.read_var("__promise") > except Exception: > continue > parent_coro = _get_continuation(promise_ptr) > # Add all the async stack frames > if parent_coro is not None: > frame_idx = _create_coroutine_frames(parent_coro, > output_frames, frame_idx, frame_count) > ```
I don't see why you say "I don't have a way to store state between get_stack_frames calls". The way this will work is that when we decide to attach a stack frame provider to a Thread, we'll create an instance of your Python class and attach it to the thread. Then we'll keep querying that object every time we need more stack frames. Since that object persists between requests it can easily store whatever state it needs. Note, the object will have to clear its state every time the process resumes. We could do that by creating a new object on each stop, which is closer to how lldb handles stack frame lists internally. or we could have a "will_resume" (and maybe "did_stop") callback in the class that the object can use to reset its state. I kind of like the latter approach better because it might be useful for the provider to remember what it had produced the last time round. Also, at base, in Python `some_thread.GetFrameAtIndex(5)` has got to work to produce whatever the current view of that thread's stack frame list wants to present for that frame. It seems easier to treat that as the primitive and wrap a generator approach around that if you like it better. https://github.com/llvm/llvm-project/pull/161870 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
