Re: [RFC PATCH v1] Support inspecting green threads

["Botond Dénes" <bdenes@scylladb.com>]

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