"previous frame inner to this frame" error when unwinding fibers

"previous frame inner to this frame" error when unwinding fibers

[Andrey Turkin]

Hi all,

I'm trying to write a custom unwinder in Python to unwind through
boost coroutine (so, follow coroutine stack with its caller's stack).
It generally works pretty well except for occasional errors about the
inner frame. Obviously, coroutine switches stack so comparing frame
pointers makes no sense; so, when caller stack happens to be allocated
above coroutine's, the bogus error occurs.
I can see in gdb sources that it knows about gcc's split-stacks and
apparently arch-specific frames, however I found no way for the
unwinder to do anything about it. Is there any way to fix this error
with the current gdb, or do I need to patch the sources?

Thanks,
  Andrey Turkin

Re: "previous frame inner to this frame" error when unwinding fibers

[Tom Tromey]

>>>>> "Andrey" == Andrey Turkin via Gdb <gdb@sourceware.org> writes:

Andrey> I'm trying to write a custom unwinder in Python to unwind through
Andrey> boost coroutine (so, follow coroutine stack with its caller's stack).
Andrey> It generally works pretty well except for occasional errors about the
Andrey> inner frame. Obviously, coroutine switches stack so comparing frame
Andrey> pointers makes no sense; so, when caller stack happens to be allocated
Andrey> above coroutine's, the bogus error occurs.
Andrey> I can see in gdb sources that it knows about gcc's split-stacks and
Andrey> apparently arch-specific frames, however I found no way for the
Andrey> unwinder to do anything about it. Is there any way to fix this error
Andrey> with the current gdb, or do I need to patch the sources?

You'll need to patch gdb.  The current code for handling this sort of
thing is totally ad hoc and can't be tweaked by the user or from Python.

Letting Python unwinders affect this seems like a nice feature.  Like
maybe a method on gdb.UnwindInfo that sets a flag, then pipe this
through to the check in get_prev_frame_always_1.

Maybe some other way is better, I don't know.  I'm definitely open to
ideas and I think this is worth solving -- I feel it's been asked for
before.

Tom

Re: "previous frame inner to this frame" error when unwinding fibers

[Andrey Turkin]

пн, 11 дек. 2023 г. в 01:30, Tom Tromey <tom@tromey.com>:
>
> >>>>> "Andrey" == Andrey Turkin via Gdb <gdb@sourceware.org> writes:
>
> Andrey> I'm trying to write a custom unwinder in Python to unwind through
> Andrey> boost coroutine (so, follow coroutine stack with its caller's stack).
> Andrey> It generally works pretty well except for occasional errors about the
> Andrey> inner frame. Obviously, coroutine switches stack so comparing frame
> Andrey> pointers makes no sense; so, when caller stack happens to be allocated
> Andrey> above coroutine's, the bogus error occurs.
> Andrey> I can see in gdb sources that it knows about gcc's split-stacks and
> Andrey> apparently arch-specific frames, however I found no way for the
> Andrey> unwinder to do anything about it. Is there any way to fix this error
> Andrey> with the current gdb, or do I need to patch the sources?
>
> You'll need to patch gdb.  The current code for handling this sort of
> thing is totally ad hoc and can't be tweaked by the user or from Python.
>
> Letting Python unwinders affect this seems like a nice feature.  Like
> maybe a method on gdb.UnwindInfo that sets a flag, then pipe this
> through to the check in get_prev_frame_always_1.
>
> Maybe some other way is better, I don't know.  I'm definitely open to
> ideas and I think this is worth solving -- I feel it's been asked for
> before.
>
> Tom

One possible solution that comes to my mind is to allow unwinders to
specify the type of the frame (ideally that would be the frame being
unwinded, i.e. one from PendingFrame; UnwindInfo I think is all about
the next frame though). This would be enough to solve this issue since
the inner-frame checking code only works with normal caller-callee
pairs. Not sure which type it would be; sigtrap is the one most
closely resembling it I think but not quite it.

PS: One other thing that is needed for the fiber/coroutine use case is
an ability to perform backtraces from a random starting point.
Backtrace through the switch point is what's needed for active
asymmetric coroutines like generators and such; however it would be
nice to be able to see the current stack of suspended asymmetric
coroutines, or to see the state of symmetric coroutines. This is
something that can be done currently through the abuse of the
backtrace/unwind system, and it sort of works fine, ca. inner frame
thing (see e.g. folly's fiber gdb helper for an example of this). Gdb
can show basic frame information for any frame given sp/ip but I don't
think it is possible to do the unwinding. Would be great if that was
possible to do in some semi-clean manner (to start at any given sp/ip,
or maybe to start from the dummy frame with some registers filled in).

Re: "previous frame inner to this frame" error when unwinding fibers

[Tom Tromey]

Andrey> One possible solution that comes to my mind is to allow unwinders to
Andrey> specify the type of the frame (ideally that would be the frame being
Andrey> unwinded, i.e. one from PendingFrame; UnwindInfo I think is all about
Andrey> the next frame though). This would be enough to solve this issue since
Andrey> the inner-frame checking code only works with normal caller-callee
Andrey> pairs. Not sure which type it would be; sigtrap is the one most
Andrey> closely resembling it I think but not quite it.

SIGTRAMP_FRAME has some special handling in stack.c, so stack traces
would be formatted differently.  I guess we could introduce a new
constant if we wanted to go this route.

Andrey> PS: One other thing that is needed for the fiber/coroutine use case is
Andrey> an ability to perform backtraces from a random starting point.
Andrey> Backtrace through the switch point is what's needed for active
Andrey> asymmetric coroutines like generators and such; however it would be
Andrey> nice to be able to see the current stack of suspended asymmetric
Andrey> coroutines, or to see the state of symmetric coroutines.

A while ago I wrote some initial support for "green threads" by
extending the Python API.  See this thread:

https://inbox.sourceware.org/gdb/YiCk+NNtAGQPhyK5@stefanha-x1.localdomain/

Your situation sounds somewhat similar -- the basic idea is to model
user-space threads, letting Python code replace the sentinel frame.
Then, unlike with 'select-frame', backtraces will work ok.

However maybe your case isn't really identical to this.  Like, do
coroutines store registers?  Maybe some more abstract approach is needed.

Tom

Re: "previous frame inner to this frame" error when unwinding fibers

[Andrey Turkin]

пн, 11 дек. 2023 г. в 20:46, Tom Tromey <tom@tromey.com>:
>
> Andrey> One possible solution that comes to my mind is to allow unwinders to
> Andrey> specify the type of the frame (ideally that would be the frame being
> Andrey> unwinded, i.e. one from PendingFrame; UnwindInfo I think is all about
> Andrey> the next frame though). This would be enough to solve this issue since
> Andrey> the inner-frame checking code only works with normal caller-callee
> Andrey> pairs. Not sure which type it would be; sigtrap is the one most
> Andrey> closely resembling it I think but not quite it.
>
> SIGTRAMP_FRAME has some special handling in stack.c, so stack traces
> would be formatted differently.  I guess we could introduce a new
> constant if we wanted to go this route.
>
> Andrey> PS: One other thing that is needed for the fiber/coroutine use case is
> Andrey> an ability to perform backtraces from a random starting point.
> Andrey> Backtrace through the switch point is what's needed for active
> Andrey> asymmetric coroutines like generators and such; however it would be
> Andrey> nice to be able to see the current stack of suspended asymmetric
> Andrey> coroutines, or to see the state of symmetric coroutines.
>
> A while ago I wrote some initial support for "green threads" by
> extending the Python API.  See this thread:
>
> https://inbox.sourceware.org/gdb/YiCk+NNtAGQPhyK5@stefanha-x1.localdomain/
>
> Your situation sounds somewhat similar -- the basic idea is to model
> user-space threads, letting Python code replace the sentinel frame.
> Then, unlike with 'select-frame', backtraces will work ok.
>
> However maybe your case isn't really identical to this.  Like, do
> coroutines store registers?  Maybe some more abstract approach is needed.
>
> Tom

Stackful coroutines I've ever encountered are very close to
cooperative green threads - minus an underlying scheduler - in their
implementation (separate stack + ucontext to store ABI-saved
registers); a mental model is a bit different. I like the idea you
proposed in that thread, with a caveat that the green threads would be
more ephemeral. E.g. if I want to see the state of a coroutine, I'd
spin up a green thread for it, switch to it and do whatever I'd
usually do with a thread of execution; and then when I'm done I'd
switch over to another thread and wind the green thread down so it is
no more (or maybe I'd cache those threads and destroy them in bulk
upon debuggee resume). As long as there is a Python API to manage them
and as long as their model is similar enough to the OS threads' API,
I'm sure we'd be able to code whatever solution suits one's specific
needs.

Regards, Andrey

Re: "previous frame inner to this frame" error when unwinding fibers

[Tom Tromey]

Andrey> Stackful coroutines I've ever encountered are very close to
Andrey> cooperative green threads - minus an underlying scheduler - in their
Andrey> implementation (separate stack + ucontext to store ABI-saved
Andrey> registers); a mental model is a bit different. I like the idea you
Andrey> proposed in that thread, with a caveat that the green threads would be
Andrey> more ephemeral. E.g. if I want to see the state of a coroutine, I'd
Andrey> spin up a green thread for it, switch to it and do whatever I'd
Andrey> usually do with a thread of execution; and then when I'm done I'd
Andrey> switch over to another thread and wind the green thread down so it is
Andrey> no more (or maybe I'd cache those threads and destroy them in bulk
Andrey> upon debuggee resume). As long as there is a Python API to manage them
Andrey> and as long as their model is similar enough to the OS threads' API,
Andrey> I'm sure we'd be able to code whatever solution suits one's specific
Andrey> needs.

Yeah, the API on the branch is very simple.  You can create a
GreenThread object, and this object supplies a few details: basically
registers and the thread name.  There's a way to indicate that the
thread has exited as well.  (I didn't get around to implementing a way
for 'bt' in a green thread to know when it should terminate.)

So, things like "frame view" could be rewritten in these terms, with the
idea being that the new green thread would simply be marked as exited
when the user switched away from it.  Coroutines could do the same, I
think.

Tom

Re: "previous frame inner to this frame" error when unwinding fibers

[Tom Tromey]

Tom> Yeah, the API on the branch is very simple.

I made a new branch and reworked it a bit, dropping a few stale parts.
It's in my github as "green-threads-v2"; you can try it if you want.

I've appended a sample .py that works with the "vireo" user-space
threads package so you can see how it works.

I still haven't implemented a way for green threads to know when to stop
unwinding.  And, I haven't added a way to indicate that the "inner than"
requirement should be lifted.

If you have ideas for how those ought to work, that would be good to
hear.  We talked about doing this in an unwinder but it seems kind of
heavy to require an unwinder to supplement green threads; though maybe
in your case that's ok.

One thought I had was to have a callback on the green thread object that
could be passed a frame to see if it is the outermost frame.  This way
green threads, at least, could stop unwinding at their scheduler; and
users wanting to debug a scheduler could switch to a "real" thread to
see that.

Maybe inner-than could just be an optional flag passed to
create_green_thread.

Tom


import gdb

thread_map = {}

main_thread = None

# From glibc/sysdeps/unix/sysv/linux/x86/sys/ucontext.h
x8664_regs = [ 'r8', 'r9', 'r10', 'r11', 'r12', 'r13', 'r14',
               'r15', 'rdi', 'rsi', 'rbp', 'rbx', 'rdx', 'rax',
               'rcx', 'rsp', 'rip', 'efl', 'csgsfs', 'err',
               'trapno', 'oldmask', 'cr2' ]

def vireo_current():
    return int(gdb.parse_and_eval('curenv')) + 1

class VireoGreenThread:
    def __init__(self, tid):
        self.tid = tid

    def _get_state(self):
        return gdb.parse_and_eval('envs')[self.tid]['state']

    def fetch(self, reg):
        """Fetch REG from memory."""
        global x8664_regs
        global thread_map
        thread = thread[self.tid]
        state = self._get_state()
        gregs = state['uc_mcontext']['gregs']
        for i in range(0, len(x8664_regs)):
            if reg is None or reg == x8664_regs[i]:
                thread.write_register(x8664_regs[i], gregs[i])

    def store(self, reg):
        global x8664_regs
        global thread_map
        thread = thread[self.tid]
        state = self._get_state()
        gregs = state['uc_mcontext']['gregs']
        for i in range(0, len(x8664_regs)):
            if reg is None or reg == x8664_regs[i]:
                gregs[i] = thread.read_register(x8664_regs[i])

    def name(self):
        return "Vireo Thread " + str(self.tid)

    def underlying_thread(self):
        if vireo_current() == self.tid:
            global main_thread
            return main_thread
        return None

class VFinish(gdb.FinishBreakpoint):
    def stop(self):
        tid = int(self.return_value) + 1
        global thread_map
        thread_map[tid] = gdb.create_green_thread(tid, VireoGreenThread(tid))
        return False

class VCreate(gdb.Breakpoint):
    def stop(self):
        VFinish(gdb.newest_frame(), True)
        return False

class VExit(gdb.Breakpoint):
    def stop(self):
        global main_thread
        if main_thread is None:
            main_thread = gdb.selected_thread()
        global thread_map
        tid = vireo_current()
        if tid in thread_map:
            thread_map[tid].set_exited()
            del thread_map[tid]

VCreate('vireo_create', internal=True)
VExit('vireo_exit', internal=True)

Re: "previous frame inner to this frame" error when unwinding fibers

[Andrey Turkin]

[-- Attachment #1: Type: text/plain, Size: 5490 bytes --]

Hi Tom,

Thanks for the update.

Re stopping the unwind for green threads - callback makes sense I
think; I guess most users would want to chop off some useless tail
there. But also custom unwinders might want to have some clean way to
do the same for whatever reason. I currently just do
create_unwind_info() without filling any registers; that works but
results in an ugly error message.

Re inner-than thing - this is orthogonal to green threads. This is
something that happens because of unwinder stitching together
different stacks; it doesn't have to be due to green threads. In fact,
with the green threads support it might not be necessary to do the
stiching; we might get away with doing several bts for the threads we
want shown together, or something like a custom command to switch
between callee/caller contexts etc. But anyway, if this is to be
implemented, it seems to me this belongs to the unwinders domain.

Re PoC implementation on GH - I tried it out. There was an obvious bug
(see attached patch), after dealing with that I was able to create a
green thread for my own test example (boost coroutines, using core
file) as well as Vireo examples (live process) with your python file.
Thread exists, it is shown in info threads with the name I gave it in
python, I can switch to it etc, but the registers don't get fetched
and bt, info frame etc shows information of a native thread with id 1
(even if I'm switching from the different thread, and creating the
green thread while switched to another thread, and tid passed to
create_green_thread being some random number which is definitely not
1). py_green_thread::fetch_registers doesn't get called for some
reason, so the Python counterpart doesn't as well.

пт, 22 дек. 2023 г. в 22:19, Tom Tromey <tom@tromey.com>:
>
> Tom> Yeah, the API on the branch is very simple.
>
> I made a new branch and reworked it a bit, dropping a few stale parts.
> It's in my github as "green-threads-v2"; you can try it if you want.
>
> I've appended a sample .py that works with the "vireo" user-space
> threads package so you can see how it works.
>
> I still haven't implemented a way for green threads to know when to stop
> unwinding.  And, I haven't added a way to indicate that the "inner than"
> requirement should be lifted.
>
> If you have ideas for how those ought to work, that would be good to
> hear.  We talked about doing this in an unwinder but it seems kind of
> heavy to require an unwinder to supplement green threads; though maybe
> in your case that's ok.
>
> One thought I had was to have a callback on the green thread object that
> could be passed a frame to see if it is the outermost frame.  This way
> green threads, at least, could stop unwinding at their scheduler; and
> users wanting to debug a scheduler could switch to a "real" thread to
> see that.
>
> Maybe inner-than could just be an optional flag passed to
> create_green_thread.
>
> Tom
>
>
> import gdb
>
> thread_map = {}
>
> main_thread = None
>
> # From glibc/sysdeps/unix/sysv/linux/x86/sys/ucontext.h
> x8664_regs = [ 'r8', 'r9', 'r10', 'r11', 'r12', 'r13', 'r14',
>                'r15', 'rdi', 'rsi', 'rbp', 'rbx', 'rdx', 'rax',
>                'rcx', 'rsp', 'rip', 'efl', 'csgsfs', 'err',
>                'trapno', 'oldmask', 'cr2' ]
>
> def vireo_current():
>     return int(gdb.parse_and_eval('curenv')) + 1
>
> class VireoGreenThread:
>     def __init__(self, tid):
>         self.tid = tid
>
>     def _get_state(self):
>         return gdb.parse_and_eval('envs')[self.tid]['state']
>
>     def fetch(self, reg):
>         """Fetch REG from memory."""
>         global x8664_regs
>         global thread_map
>         thread = thread[self.tid]
>         state = self._get_state()
>         gregs = state['uc_mcontext']['gregs']
>         for i in range(0, len(x8664_regs)):
>             if reg is None or reg == x8664_regs[i]:
>                 thread.write_register(x8664_regs[i], gregs[i])
>
>     def store(self, reg):
>         global x8664_regs
>         global thread_map
>         thread = thread[self.tid]
>         state = self._get_state()
>         gregs = state['uc_mcontext']['gregs']
>         for i in range(0, len(x8664_regs)):
>             if reg is None or reg == x8664_regs[i]:
>                 gregs[i] = thread.read_register(x8664_regs[i])
>
>     def name(self):
>         return "Vireo Thread " + str(self.tid)
>
>     def underlying_thread(self):
>         if vireo_current() == self.tid:
>             global main_thread
>             return main_thread
>         return None
>
> class VFinish(gdb.FinishBreakpoint):
>     def stop(self):
>         tid = int(self.return_value) + 1
>         global thread_map
>         thread_map[tid] = gdb.create_green_thread(tid, VireoGreenThread(tid))
>         return False
>
> class VCreate(gdb.Breakpoint):
>     def stop(self):
>         VFinish(gdb.newest_frame(), True)
>         return False
>
> class VExit(gdb.Breakpoint):
>     def stop(self):
>         global main_thread
>         if main_thread is None:
>             main_thread = gdb.selected_thread()
>         global thread_map
>         tid = vireo_current()
>         if tid in thread_map:
>             thread_map[tid].set_exited()
>             del thread_map[tid]
>
> VCreate('vireo_create', internal=True)
> VExit('vireo_exit', internal=True)

[-- Attachment #2: patch.py --]
[-- Type: application/octet-stream, Size: 468 bytes --]

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6a4867feebc..34cbc7a1b25 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -2754,7 +2754,7 @@ Return the current print options." },
 Output async record to MI channels if any." },
 
   { "create_green_thread", (PyCFunction) gdbpy_create_green_thread,
-    METH_VARARGS,
+    METH_VARARGS | METH_KEYWORDS,
     "create_green_thread (CALLBACK) -> GreenThread.\n\
 Create a new green thread." },
 

Re: "previous frame inner to this frame" error when unwinding fibers

[Tom Tromey]

>>>>> "Andrey" == Andrey Turkin <andrey.turkin@gmail.com> writes:

Andrey> Re stopping the unwind for green threads - callback makes sense I
Andrey> think; I guess most users would want to chop off some useless tail
Andrey> there. But also custom unwinders might want to have some clean way to
Andrey> do the same for whatever reason. I currently just do
Andrey> create_unwind_info() without filling any registers; that works but
Andrey> results in an ugly error message.

For unwinders I think we could add a more principled way to signal the
end of the stack.

Andrey> Re inner-than thing - this is orthogonal to green threads. This is
Andrey> something that happens because of unwinder stitching together
Andrey> different stacks; it doesn't have to be due to green threads. In fact,
Andrey> with the green threads support it might not be necessary to do the
Andrey> stiching; we might get away with doing several bts for the threads we
Andrey> want shown together, or something like a custom command to switch
Andrey> between callee/caller contexts etc. But anyway, if this is to be
Andrey> implemented, it seems to me this belongs to the unwinders domain.

I see what you mean, though stitching together stacks from different
threads is maybe a questionable procedure in gdb.

One issue is that the frame cache is global -- not per-thread.  So,
switching threads will flush it and cause it to be repopulated.  As long
as the inferior doesn't run, this will be "invisible" to the user,
though it may show up as a performance problem.

It'd be nice to fix this but there's some tricky business involving
vfork IIRC.  I think there's a bug open about it.

Also when stitching together stacks, I wonder about the semantics of
things like "finish".  But... maybe as long as selecting a frame also
selects its underlying thread, it could kinda work.

Andrey> Re PoC implementation on GH - I tried it out.

Thank you.

Andrey> There was an obvious bug (see attached patch),

I applied this to my branch.

Andrey> I can switch to it etc, but the registers don't get fetched
Andrey> and bt, info frame etc shows information of a native thread with id 1

I am not sure but I suspect this was this problem in a few methods in
green_thread_target -- they checked the green thread's ptid, but they
should have checked if the green thread has an underlying thread.

The idea here is that when a green thread is currently running on a
native thread, registers should simply come from that; but if the green
thread is inactive, then the Python code should be used.

I made this change on my branch & pushed it again, in case you want to
give it another try.

Tom