[RFC PATCH v1] Support inspecting green threads

[RFC PATCH v1] Support inspecting green threads

[Botond Dénes]

Some applications use green threads that have their own stacks. One such
example is Scylla [1] which is using the seastar framework [2], which
provides green threads in the form of seastar::thread [3]. These threads
are created with `setcontext()` and later we switch in/out using
`setjmp()`/`longjmp()`

When debugging Scylla (or any other seastar application) it is essential
to be able to inspect the stacks of these green threads. For this we
used a python extension, which essentially emulates a `longjmp()` call,
restoring the registers from a `jmpbuf`. This approach worked fine for
some time, however we started looking for another way to do this
several reasons:
* Writing to registers is dangerous, any bug in the python script or GDB
  can result in crashing the debugged live application. Very undesirable
  when having to debug on a production server.
* Doesn't work in coredumps, where registers are not writable.
* Stopped working for some time, resulting in a crash of GDB.

This patch aims to solve this problem by providing a family of commands
which allow inspecting green threads.
* fiber view - allows inspecting a green thread. This command takes a
  list of registers as its arguments. More on these later.
* fiber reset - reset the viewed fiber to the stack proper.

The implementation turned out to be quite simple. The command just
propagates the registers to override to the sentinel frame, which will
return the overridden values for registers of interest, instead of
consulting the current register cache, thus simulating a register
restore. This has two important advantages: it works in coredumps and it
is save. If the user passes wrong register values, it might crash gdb,
but as the application state is not mangled with, it should survive
unscathed.

TODO:
* Better interface for the command: instead of a fixed set of registers
  (those that happen to be in glibc's jmpbuf on amd64), allow the user
  to pass any amount of named registers to override.
* More documentation.
* More testing.
* Code style.

I apologize for the mixed code style, but I wanted to probe my approach
first, before spending more time on polishing.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
---
 gdb/frame.c          | 16 ++++++++++++-
 gdb/frame.h          |  7 ++++++
 gdb/sentinel-frame.c | 30 ++++++++++++++++++++++---
 gdb/sentinel-frame.h |  2 +-
 gdb/stack.c          | 53 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index f65d43f9fc..17c139cfe9 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -53,6 +53,8 @@
 
 static struct frame_info *sentinel_frame;
 
+static struct saved_registers fiber_regs_override;
+
 /* Number of calls to reinit_frame_cache.  */
 static unsigned int frame_cache_generation = 0;
 
@@ -1570,7 +1572,7 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
   /* Explicitly initialize the sentinel frame's cache.  Provide it
      with the underlying regcache.  In the future additional
      information, such as the frame's thread will be added.  */
-  frame->prologue_cache = sentinel_frame_cache (regcache);
+  frame->prologue_cache = sentinel_frame_cache (regcache, &fiber_regs_override);
   /* For the moment there is only one sentinel frame implementation.  */
   frame->unwind = &sentinel_frame_unwind;
   /* Link this frame back to itself.  The frame is self referential
@@ -2934,6 +2936,18 @@ frame_prepare_for_sniffer (struct frame_info *frame,
   frame->unwind = unwind;
 }
 
+void set_fiber_register_override(saved_registers regs_override)
+{
+  fiber_regs_override = std::move(regs_override);
+  reinit_frame_cache();
+}
+
+void reset_fiber_register_override()
+{
+  fiber_regs_override.reg_map.clear();
+  reinit_frame_cache();
+}
+
 static struct cmd_list_element *set_backtrace_cmdlist;
 static struct cmd_list_element *show_backtrace_cmdlist;
 
diff --git a/gdb/frame.h b/gdb/frame.h
index 1c6afad1ae..a8ddd8af07 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -71,6 +71,7 @@
 
 #include "language.h"
 #include "cli/cli-option.h"
+#include <map>
 
 struct symtab_and_line;
 struct frame_unwind;
@@ -955,5 +956,11 @@ extern void set_frame_previous_pc_masked (struct frame_info *frame);
 
 extern bool get_frame_pc_masked (const struct frame_info *frame);
 
+struct saved_registers {
+    std::map<int, const std::vector<gdb_byte>> reg_map;
+};
+
+extern void set_fiber_register_override(saved_registers regs_override);
+extern void reset_fiber_register_override();
 
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/sentinel-frame.c b/gdb/sentinel-frame.c
index 1d601ca27b..b669a36656 100644
--- a/gdb/sentinel-frame.c
+++ b/gdb/sentinel-frame.c
@@ -23,19 +23,23 @@
 #include "sentinel-frame.h"
 #include "inferior.h"
 #include "frame-unwind.h"
+#include "frame.h"
 
 struct frame_unwind_cache
 {
   struct regcache *regcache;
+  struct saved_registers *saved_regs;
 };
 
 void *
-sentinel_frame_cache (struct regcache *regcache)
+sentinel_frame_cache (struct regcache *regcache, struct saved_registers *saved_regs)
 {
   struct frame_unwind_cache *cache = 
     FRAME_OBSTACK_ZALLOC (struct frame_unwind_cache);
 
   cache->regcache = regcache;
+  cache->saved_regs = saved_regs;
+
   return cache;
 }
 
@@ -50,8 +54,28 @@ sentinel_frame_prev_register (struct frame_info *this_frame,
     = (struct frame_unwind_cache *) *this_prologue_cache;
   struct value *value;
 
-  value = cache->regcache->cooked_read_value (regnum);
-  VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
+  if (!cache->saved_regs) {
+      value = cache->regcache->cooked_read_value (regnum);
+      VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
+      return value;
+  }
+
+  auto it = cache->saved_regs->reg_map.find(regnum);
+
+  if (it == cache->saved_regs->reg_map.end())
+    {
+      value = cache->regcache->cooked_read_value (regnum);
+      VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
+      return value;
+    }
+
+  auto* arch = cache->regcache->arch ();
+
+  value = allocate_value (register_type (arch, regnum));
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+
+  memcpy(value_contents_raw (value), it->second.data(), it->second.size());
 
   return value;
 }
diff --git a/gdb/sentinel-frame.h b/gdb/sentinel-frame.h
index cef5fd21bd..20fd8d8d81 100644
--- a/gdb/sentinel-frame.h
+++ b/gdb/sentinel-frame.h
@@ -30,7 +30,7 @@ struct regcache;
 /* Pump prime the sentinel frame's cache.  Since this needs the
    REGCACHE provide that here.  */
 
-extern void *sentinel_frame_cache (struct regcache *regcache);
+extern void *sentinel_frame_cache (struct regcache *regcache, struct saved_registers *saved_regs = nullptr);
 
 /* At present there is only one type of sentinel frame.  */
 
diff --git a/gdb/stack.c b/gdb/stack.c
index 265e764dc2..bb22d01813 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -55,6 +55,9 @@
 #include "gdbsupport/def-vector.h"
 #include "cli/cli-option.h"
 #include "cli/cli-style.h"
+#include "user-regs.h"
+
+#include <sstream>
 
 /* The possible choices of "set print frame-arguments", and the value
    of this setting.  */
@@ -3291,6 +3294,41 @@ find_frame_for_address (CORE_ADDR address)
   return NULL;
 }
 
+static void
+fiber_base_command (const char *arg, int from_tty)
+{
+}
+
+static void
+fiber_view_command (const char *arg, int from_tty)
+{
+  saved_registers regs_override;
+
+  std::stringstream ss(arg, std::ios_base::in);
+
+  struct gdbarch *gdbarch = get_frame_arch (get_current_frame());
+
+  const static char* jmpbuf_regs[] = {"rbx", "rbp", "r12", "r13", "r14", "r15", "rsp", "rip"};
+
+  for (auto reg : jmpbuf_regs) {
+      uint64_t val;
+      ss >> std::hex >> val;
+
+      const auto regnum = user_reg_map_name_to_regnum (gdbarch, reg, strlen(reg));
+
+      std::vector<gdb_byte> raw_val(sizeof(val), 0);
+      memcpy(raw_val.data(), reinterpret_cast<gdb_byte*>(&val), sizeof(val));
+      regs_override.reg_map.emplace(regnum, std::move(raw_val));
+  }
+
+  set_fiber_register_override(regs_override);
+}
+
+static void
+fiber_reset_command (const char *arg, int from_tty)
+{
+    reset_fiber_register_override();
+}
 \f
 
 /* Commands with a prefix of `frame apply'.  */
@@ -3305,6 +3343,8 @@ static struct cmd_list_element *select_frame_cmd_list = NULL;
 /* Commands with a prefix of `info frame'.  */
 static struct cmd_list_element *info_frame_cmd_list = NULL;
 
+static struct cmd_list_element *fiber_cmd_list = NULL;
+
 void _initialize_stack ();
 void
 _initialize_stack ()
@@ -3597,6 +3637,19 @@ source line."),
 			        NULL,
 			        show_disassemble_next_line,
 			        &setlist, &showlist);
+
+  add_prefix_cmd ("fiber", class_stack, &fiber_base_command,
+		  _("Inspect or change the currently viewed fiber.\n"),
+		  &fiber_cmd_list, "fiber", 1, &cmdlist);
+
+  add_cmd ("view", class_stack, &fiber_view_command,
+	   _("Select the fiber to view.\n"),
+	   &fiber_cmd_list);
+
+  add_cmd ("reset", class_stack, &fiber_reset_command,
+	   _("Reset to the currently viewed fiber.\n"),
+	   &fiber_cmd_list);
+
   disassemble_next_line = AUTO_BOOLEAN_FALSE;
 
   gdb::option::add_setshow_cmds_for_options
-- 
2.26.2

Re: [RFC PATCH v1] Support inspecting green threads

[Botond Dénes]

On Fri, 2020-08-14 at 18:25 +0300, Botond Dénes wrote:
> Some applications use green threads that have their own stacks. One
> such
> example is Scylla [1] which is using the seastar framework [2], which
> provides green threads in the form of seastar::thread [3]. These
> threads
> are created with `setcontext()` and later we switch in/out using
> `setjmp()`/`longjmp()`
> 
> When debugging Scylla (or any other seastar application) it is
> essential
> to be able to inspect the stacks of these green threads. For this we
> used a python extension, which essentially emulates a `longjmp()`
> call,
> restoring the registers from a `jmpbuf`. This approach worked fine
> for
> some time, however we started looking for another way to do this
> several reasons:
> * Writing to registers is dangerous, any bug in the python script or
> GDB
>   can result in crashing the debugged live application. Very
> undesirable
>   when having to debug on a production server.
> * Doesn't work in coredumps, where registers are not writable.
> * Stopped working for some time, resulting in a crash of GDB.
> 
> This patch aims to solve this problem by providing a family of
> commands
> which allow inspecting green threads.
> * fiber view - allows inspecting a green thread. This command takes a
>   list of registers as its arguments. More on these later.
> * fiber reset - reset the viewed fiber to the stack proper.
> 
> The implementation turned out to be quite simple. The command just
> propagates the registers to override to the sentinel frame, which
> will
> return the overridden values for registers of interest, instead of
> consulting the current register cache, thus simulating a register
> restore. This has two important advantages: it works in coredumps and
> it
> is save. If the user passes wrong register values, it might crash
> gdb,
> but as the application state is not mangled with, it should survive
> unscathed.
> 
> TODO:
> * Better interface for the command: instead of a fixed set of
> registers
>   (those that happen to be in glibc's jmpbuf on amd64), allow the
> user
>   to pass any amount of named registers to override.
> * More documentation.
> * More testing.
> * Code style.
> 
> I apologize for the mixed code style, but I wanted to probe my
> approach
> first, before spending more time on polishing.


Forgot to add links:
[1] https://github.com/scylladb/scylla
[2] https://github.com/scylladb/seastar
[3] http://docs.seastar.io/master/group__thread-module.html

> 
> Signed-off-by: Botond Dénes <bdenes@scylladb.com>
> ---
>  gdb/frame.c          | 16 ++++++++++++-
>  gdb/frame.h          |  7 ++++++
>  gdb/sentinel-frame.c | 30 ++++++++++++++++++++++---
>  gdb/sentinel-frame.h |  2 +-
>  gdb/stack.c          | 53
> ++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 103 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index f65d43f9fc..17c139cfe9 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -53,6 +53,8 @@
>  
>  static struct frame_info *sentinel_frame;
>  
> +static struct saved_registers fiber_regs_override;
> +
>  /* Number of calls to reinit_frame_cache.  */
>  static unsigned int frame_cache_generation = 0;
>  
> @@ -1570,7 +1572,7 @@ create_sentinel_frame (struct program_space
> *pspace, struct regcache *regcache)
>    /* Explicitly initialize the sentinel frame's cache.  Provide it
>       with the underlying regcache.  In the future additional
>       information, such as the frame's thread will be added.  */
> -  frame->prologue_cache = sentinel_frame_cache (regcache);
> +  frame->prologue_cache = sentinel_frame_cache (regcache,
> &fiber_regs_override);
>    /* For the moment there is only one sentinel frame
> implementation.  */
>    frame->unwind = &sentinel_frame_unwind;
>    /* Link this frame back to itself.  The frame is self referential
> @@ -2934,6 +2936,18 @@ frame_prepare_for_sniffer (struct frame_info
> *frame,
>    frame->unwind = unwind;
>  }
>  
> +void set_fiber_register_override(saved_registers regs_override)
> +{
> +  fiber_regs_override = std::move(regs_override);
> +  reinit_frame_cache();
> +}
> +
> +void reset_fiber_register_override()
> +{
> +  fiber_regs_override.reg_map.clear();
> +  reinit_frame_cache();
> +}
> +
>  static struct cmd_list_element *set_backtrace_cmdlist;
>  static struct cmd_list_element *show_backtrace_cmdlist;
>  
> diff --git a/gdb/frame.h b/gdb/frame.h
> index 1c6afad1ae..a8ddd8af07 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -71,6 +71,7 @@
>  
>  #include "language.h"
>  #include "cli/cli-option.h"
> +#include <map>
>  
>  struct symtab_and_line;
>  struct frame_unwind;
> @@ -955,5 +956,11 @@ extern void set_frame_previous_pc_masked (struct
> frame_info *frame);
>  
>  extern bool get_frame_pc_masked (const struct frame_info *frame);
>  
> +struct saved_registers {
> +    std::map<int, const std::vector<gdb_byte>> reg_map;
> +};
> +
> +extern void set_fiber_register_override(saved_registers
> regs_override);
> +extern void reset_fiber_register_override();
>  
>  #endif /* !defined (FRAME_H)  */
> diff --git a/gdb/sentinel-frame.c b/gdb/sentinel-frame.c
> index 1d601ca27b..b669a36656 100644
> --- a/gdb/sentinel-frame.c
> +++ b/gdb/sentinel-frame.c
> @@ -23,19 +23,23 @@
>  #include "sentinel-frame.h"
>  #include "inferior.h"
>  #include "frame-unwind.h"
> +#include "frame.h"
>  
>  struct frame_unwind_cache
>  {
>    struct regcache *regcache;
> +  struct saved_registers *saved_regs;
>  };
>  
>  void *
> -sentinel_frame_cache (struct regcache *regcache)
> +sentinel_frame_cache (struct regcache *regcache, struct
> saved_registers *saved_regs)
>  {
>    struct frame_unwind_cache *cache = 
>      FRAME_OBSTACK_ZALLOC (struct frame_unwind_cache);
>  
>    cache->regcache = regcache;
> +  cache->saved_regs = saved_regs;
> +
>    return cache;
>  }
>  
> @@ -50,8 +54,28 @@ sentinel_frame_prev_register (struct frame_info
> *this_frame,
>      = (struct frame_unwind_cache *) *this_prologue_cache;
>    struct value *value;
>  
> -  value = cache->regcache->cooked_read_value (regnum);
> -  VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
> +  if (!cache->saved_regs) {
> +      value = cache->regcache->cooked_read_value (regnum);
> +      VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
> +      return value;
> +  }
> +
> +  auto it = cache->saved_regs->reg_map.find(regnum);
> +
> +  if (it == cache->saved_regs->reg_map.end())
> +    {
> +      value = cache->regcache->cooked_read_value (regnum);
> +      VALUE_NEXT_FRAME_ID (value) = sentinel_frame_id;
> +      return value;
> +    }
> +
> +  auto* arch = cache->regcache->arch ();
> +
> +  value = allocate_value (register_type (arch, regnum));
> +  VALUE_LVAL (value) = lval_register;
> +  VALUE_REGNUM (value) = regnum;
> +
> +  memcpy(value_contents_raw (value), it->second.data(), it-
> >second.size());
>  
>    return value;
>  }
> diff --git a/gdb/sentinel-frame.h b/gdb/sentinel-frame.h
> index cef5fd21bd..20fd8d8d81 100644
> --- a/gdb/sentinel-frame.h
> +++ b/gdb/sentinel-frame.h
> @@ -30,7 +30,7 @@ struct regcache;
>  /* Pump prime the sentinel frame's cache.  Since this needs the
>     REGCACHE provide that here.  */
>  
> -extern void *sentinel_frame_cache (struct regcache *regcache);
> +extern void *sentinel_frame_cache (struct regcache *regcache, struct
> saved_registers *saved_regs = nullptr);
>  
>  /* At present there is only one type of sentinel frame.  */
>  
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 265e764dc2..bb22d01813 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -55,6 +55,9 @@
>  #include "gdbsupport/def-vector.h"
>  #include "cli/cli-option.h"
>  #include "cli/cli-style.h"
> +#include "user-regs.h"
> +
> +#include <sstream>
>  
>  /* The possible choices of "set print frame-arguments", and the
> value
>     of this setting.  */
> @@ -3291,6 +3294,41 @@ find_frame_for_address (CORE_ADDR address)
>    return NULL;
>  }
>  
> +static void
> +fiber_base_command (const char *arg, int from_tty)
> +{
> +}
> +
> +static void
> +fiber_view_command (const char *arg, int from_tty)
> +{
> +  saved_registers regs_override;
> +
> +  std::stringstream ss(arg, std::ios_base::in);
> +
> +  struct gdbarch *gdbarch = get_frame_arch (get_current_frame());
> +
> +  const static char* jmpbuf_regs[] = {"rbx", "rbp", "r12", "r13",
> "r14", "r15", "rsp", "rip"};
> +
> +  for (auto reg : jmpbuf_regs) {
> +      uint64_t val;
> +      ss >> std::hex >> val;
> +
> +      const auto regnum = user_reg_map_name_to_regnum (gdbarch, reg,
> strlen(reg));
> +
> +      std::vector<gdb_byte> raw_val(sizeof(val), 0);
> +      memcpy(raw_val.data(), reinterpret_cast<gdb_byte*>(&val),
> sizeof(val));
> +      regs_override.reg_map.emplace(regnum, std::move(raw_val));
> +  }
> +
> +  set_fiber_register_override(regs_override);
> +}
> +
> +static void
> +fiber_reset_command (const char *arg, int from_tty)
> +{
> +    reset_fiber_register_override();
> +}
>  
>  
>  /* Commands with a prefix of `frame apply'.  */
> @@ -3305,6 +3343,8 @@ static struct cmd_list_element
> *select_frame_cmd_list = NULL;
>  /* Commands with a prefix of `info frame'.  */
>  static struct cmd_list_element *info_frame_cmd_list = NULL;
>  
> +static struct cmd_list_element *fiber_cmd_list = NULL;
> +
>  void _initialize_stack ();
>  void
>  _initialize_stack ()
> @@ -3597,6 +3637,19 @@ source line."),
>  			        NULL,
>  			        show_disassemble_next_line,
>  			        &setlist, &showlist);
> +
> +  add_prefix_cmd ("fiber", class_stack, &fiber_base_command,
> +		  _("Inspect or change the currently viewed fiber.\n"),
> +		  &fiber_cmd_list, "fiber", 1, &cmdlist);
> +
> +  add_cmd ("view", class_stack, &fiber_view_command,
> +	   _("Select the fiber to view.\n"),
> +	   &fiber_cmd_list);
> +
> +  add_cmd ("reset", class_stack, &fiber_reset_command,
> +	   _("Reset to the currently viewed fiber.\n"),
> +	   &fiber_cmd_list);
> +
>    disassemble_next_line = AUTO_BOOLEAN_FALSE;
>  
>    gdb::option::add_setshow_cmds_for_options

Re: [RFC PATCH v1] Support inspecting green threads

[H.J. Lu]

On Fri, Aug 14, 2020 at 8:29 AM Botond Dénes <bdenes@scylladb.com> wrote:
>
> Some applications use green threads that have their own stacks. One such
> example is Scylla [1] which is using the seastar framework [2], which
> provides green threads in the form of seastar::thread [3]. These threads
> are created with `setcontext()` and later we switch in/out using
> `setjmp()`/`longjmp()`
>

FYI, Intel CET doesn't allow switching ucontext created by setcontext
with setjmp/longjmp.  setcontext or swapcontext must be used.

-- 
H.J.

Re: [RFC PATCH v1] Support inspecting green threads

[Tom Tromey]

>>>>> "Botond" == Botond Dénes <bdenes@scylladb.com> writes:

Botond> Some applications use green threads that have their own stacks. One such
Botond> example is Scylla [1] which is using the seastar framework [2], which
Botond> provides green threads in the form of seastar::thread [3]. These threads
Botond> are created with `setcontext()` and later we switch in/out using
Botond> `setjmp()`/`longjmp()`

Thanks for bringing this up.  We've talked about adding a facility like
this a couple of times -- maybe it's finally time.

Botond> This patch aims to solve this problem by providing a family of commands
Botond> which allow inspecting green threads.
Botond> * fiber view - allows inspecting a green thread. This command takes a
Botond>   list of registers as its arguments. More on these later.
Botond> * fiber reset - reset the viewed fiber to the stack proper.

I like the name.

Botond> TODO:
Botond> * Better interface for the command: instead of a fixed set of registers
Botond>   (those that happen to be in glibc's jmpbuf on amd64), allow the user
Botond>   to pass any amount of named registers to override.
Botond> * More documentation.
Botond> * More testing.
Botond> * Code style.

Botond> I apologize for the mixed code style, but I wanted to probe my approach
Botond> first, before spending more time on polishing.

Yeah, don't worry about that yet.

It may interest you to know that there's already an implementation of
something like this in gdb -- the ravenscar thread code.  Ravenscar is
an Ada runtime that implements green threads.  (It is a bit weird though
in that these threads never exit.)

gdb handles this by making "fake" threads corresponding to each green
thread.  If you "next" or the like, it uses the underlying real thread
when communicating with the remote.

One thought I had was that maybe we could generalize this code.  In
conjunction with a Python API, we could make it so that 3rd parties (the
green thread library) could provide the knowledge needed to hook gdb to
the threads.

A consequence of taking this approach would be that fibers could be
integrated with threads, with the upside being that fibers would
automatically start working in gdb GUIs.  (But maybe there are downsides
as well -- for instance if a program uses setcontext in a
non-thread-like way.  This is worth discussing.)

I think we'd have to add some kind of "fiber" flag to threads and then
have a check on operations like "step" to make sure that an attempt to
step a paused thread is rejected.  I don't know for sure but maybe we'd
need some kind of tweak to ptid as well.  Finally, we'd need something
like the "random thread switch" patch that I've been wanting to get in
for Ravenscar :-)

A typical way for this to work would be to have some Python code that
sets hidden breakpoints in the thread-start and thread-exit code in a
green thread library.  Then, when a thread is started, register it with
gdb core as one of these "fiber" threads.  The Python code would also be
responsible for supplying registers on request.

One other thing I'm not sure of in this approach is how non-stop would
work.  I suppose the Python code would need to help with this as well;
but that seems odd because it implies maybe adding otherwise undesirable
facilities to green threads libraries.  So, perhaps we'd need to change
how this works in gdb instead.  (I think this isn't an issue for
Ravenscar at present because, IIUC, Ravenscar statically assigns threads
to CPUs; but also because presumably nobody has ever tried the
combination.)

Tom

Re: [RFC PATCH v1] Support inspecting green threads

[Botond Dénes]

On Mon, 2020-08-17 at 15:34 -0600, Tom Tromey wrote:
> > > > > > "Botond" == Botond Dénes <bdenes@scylladb.com> writes:
> 
> Botond> Some applications use green threads that have their own
> stacks. One such
> Botond> example is Scylla [1] which is using the seastar framework
> [2], which
> Botond> provides green threads in the form of seastar::thread [3].
> These threads
> Botond> are created with `setcontext()` and later we switch in/out
> using
> Botond> `setjmp()`/`longjmp()`
> 
> Thanks for bringing this up.  We've talked about adding a facility
> like
> this a couple of times -- maybe it's finally time.
> 
> Botond> This patch aims to solve this problem by providing a family
> of commands
> Botond> which allow inspecting green threads.
> Botond> * fiber view - allows inspecting a green thread. This command
> takes a
> Botond>   list of registers as its arguments. More on these later.
> Botond> * fiber reset - reset the viewed fiber to the stack proper.
> 
> I like the name.


Glad to hear it. :)

> 
> Botond> TODO:
> Botond> * Better interface for the command: instead of a fixed set of
> registers
> Botond>   (those that happen to be in glibc's jmpbuf on amd64), allow
> the user
> Botond>   to pass any amount of named registers to override.
> Botond> * More documentation.
> Botond> * More testing.
> Botond> * Code style.
> 
> Botond> I apologize for the mixed code style, but I wanted to probe
> my approach
> Botond> first, before spending more time on polishing.
> 
> Yeah, don't worry about that yet.
> 
> It may interest you to know that there's already an implementation of
> something like this in gdb -- the ravenscar thread code.  Ravenscar
> is
> an Ada runtime that implements green threads.  (It is a bit weird
> though
> in that these threads never exit.)
> 
> gdb handles this by making "fake" threads corresponding to each green
> thread.  If you "next" or the like, it uses the underlying real
> thread
> when communicating with the remote.
> 
> One thought I had was that maybe we could generalize this code.  In
> conjunction with a Python API, we could make it so that 3rd parties
> (the
> green thread library) could provide the knowledge needed to hook gdb
> to
> the threads.
> 
> A consequence of taking this approach would be that fibers could be
> integrated with threads, with the upside being that fibers would
> automatically start working in gdb GUIs.  (But maybe there are
> downsides
> as well -- for instance if a program uses setcontext in a
> non-thread-like way.  This is worth discussing.)
> 
> I think we'd have to add some kind of "fiber" flag to threads and
> then
> have a check on operations like "step" to make sure that an attempt
> to
> step a paused thread is rejected.  I don't know for sure but maybe
> we'd
> need some kind of tweak to ptid as well.


This would indeed make these green threads first class citizens in gdb.
Now I started thinking whether we could extend this support to
stackless fibers. In seastar there are two kinds of fibers: stackful
(we call these `seastar::thread` -- these are the green threads) and
stackless. The latter is future-promise based and we have experimental
glue code to enable C++20 coroutines based on this.

These stackless fibers are made up of continuation objects, which form
an intrusive forward linked-list. Each continuation object is a
callable and a pointer to the next continuation that should be executed
next, with the value computed by this continuation.

These fibers are an absolute pain to debug in gdb. Backtracing of
course just takes you back to the event loop. As the continuations are
classes templated on lambdas, their type name (even if known based on
the vptr of the continuation object) is unreadable.

I'm wondering if we could piece together these continuations into a
pseudo backtrace, making at least lambda captures (those the optimizer
forgot to destroy) inspectable. AFAIK gdb already has a notion of
pseudo frames that it uses to render inline frames into backtraces.
We should of course allow this mode to be disabled because sometimes
you do want to look at the event loop frames.

>   Finally, we'd need something
> like the "random thread switch" patch that I've been wanting to get
> in
> for Ravenscar :-)
> 
> A typical way for this to work would be to have some Python code that
> sets hidden breakpoints in the thread-start and thread-exit code in a
> green thread library.  Then, when a thread is started, register it
> with
> gdb core as one of these "fiber" threads.  The Python code would also
> be
> responsible for supplying registers on request.
> 
> One other thing I'm not sure of in this approach is how non-stop
> would
> work.  I suppose the Python code would need to help with this as
> well;
> but that seems odd because it implies maybe adding otherwise
> undesirable
> facilities to green threads libraries.  So, perhaps we'd need to
> change
> how this works in gdb instead.  (I think this isn't an issue for
> Ravenscar at present because, IIUC, Ravenscar statically assigns
> threads
> to CPUs; but also because presumably nobody has ever tried the
> combination.)
> 
> Tom

Re: [RFC PATCH v1] Support inspecting green threads

[Tom Tromey]

Botond> Now I started thinking whether we could extend this support to
Botond> stackless fibers.
...
Botond> These stackless fibers are made up of continuation objects, which form
Botond> an intrusive forward linked-list. Each continuation object is a
Botond> callable and a pointer to the next continuation that should be executed
Botond> next, with the value computed by this continuation.

Botond> These fibers are an absolute pain to debug in gdb. Backtracing of
Botond> course just takes you back to the event loop. As the continuations are
Botond> classes templated on lambdas, their type name (even if known based on
Botond> the vptr of the continuation object) is unreadable.

Botond> I'm wondering if we could piece together these continuations into a
Botond> pseudo backtrace, making at least lambda captures (those the optimizer
Botond> forgot to destroy) inspectable. AFAIK gdb already has a notion of
Botond> pseudo frames that it uses to render inline frames into backtraces.
Botond> We should of course allow this mode to be disabled because sometimes
Botond> you do want to look at the event loop frames.

Yeah, this might be somewhat doable today by writing a customer frame
filter and some helper code.  The idea would be to capture locations as
the chain of continuations progresses, then replay these locations in
the frame filter.

One problem with this sort of approach is that local variables would be
unavailable for these saved frames, but that's just generally an issue
with continuations I suppose.

Tom