From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by sourceware.org (Postfix) with ESMTPS id 2E6743857C52 for ; Fri, 14 Aug 2020 15:27:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2E6743857C52 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=scylladb.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=bdenes@scylladb.com Received: by mail-ed1-x542.google.com with SMTP id v22so7128945edy.0 for ; Fri, 14 Aug 2020 08:27:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=scylladb-com.20150623.gappssmtp.com; s=20150623; h=message-id:subject:from:to:cc:date:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=+Cd1jizMs7KXypwVwIHL+RBbAvOU1p26bJZOgpwKq9I=; b=bn0EdNvmHP7tYrVyNJo+iu4hNdk0xLPQb0Y7C/cHHq8G8fEkvKP//ryko534Uah+7t 5Fhwn2vWJ0D5dM+8Yr+h/+tUp31zEDjuBGoEZrg+h7OSmF9mQXd7WSeY3CHVd3rqi3fY DsyPJiTAsfxUWaffaz4gC6P1NVpYNYriPbc8PznQvZ2geyAj4wJGJ7XsG0XIbsansDwn MW2ug4DxCQ+Lu7gJGOcDTM6/Bhb/68Lrvnuj9wkUGIJA7QQuoGCNGbMn9XZ+TyIQDKwy TOWfb7AAiDCtFjM8BlN1wz+iBsHQsJb0xCkUTFI8jy6ZFIL8peht4zqcN5Iv3FRjhNoQ /ZUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=+Cd1jizMs7KXypwVwIHL+RBbAvOU1p26bJZOgpwKq9I=; b=WeKg6S9Npx06NfyC1uUZDETpawaN01K8amPV2RUt3tgP4wuRUrHiyzPeUjJkxIv/PT nWetQ8PlK9w/0B3gIzQffMIHnoJlj0UtyyFDwIYnPs8rzMYjwtnHsK2aAfTnHyERtEHa iqUJr5PfJsnRxA9MbLusq6zF6x7d5pH4xkgs7cIQfHL1wjQEWCi2HwE82fZaIMZJc1im 6jJqVDfv0AhhpmRSW09FEThnAAeLFiQBU6RtigFlO9qNetuOKfW1GMKMyX22RCYxLBt0 O5VSl94I5d9dPTdz4h6w56X1+RlRSLynGQiKXAhDDZGf2dkg5v34AUroJtR12P1EW9NG GefQ== X-Gm-Message-State: AOAM5318njTYvzJEI4DyDrLYRZ3EhlwG6/5JlarDEhsCvaeFm5wJStHq HZ5r44EiJNXsMepQ6x7hMMrJCuHkMHU99w== X-Google-Smtp-Source: ABdhPJxJlVxUMx1FugYWrlu1OYXrG06FJtz6JgZwu24aCOAYeWm6CEXqnx5S9/32pMrIvWpfO+buPA== X-Received: by 2002:a05:6402:e:: with SMTP id d14mr2819552edu.262.1597418836828; Fri, 14 Aug 2020 08:27:16 -0700 (PDT) Received: from localhost.localdomain ([2a02:2f01:8205:e100:aa9e:e920:cb1e:35d9]) by smtp.gmail.com with ESMTPSA id du2sm7129307ejc.2.2020.08.14.08.27.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Aug 2020 08:27:16 -0700 (PDT) Message-ID: <921f774bd8b173d71033c6d1757a51a75342e204.camel@scylladb.com> Subject: Re: [RFC PATCH v1] Support inspecting green threads From: Botond =?ISO-8859-1?Q?D=E9nes?= To: gdb-patches@sourceware.org Date: Fri, 14 Aug 2020 18:27:15 +0300 In-Reply-To: <20200814152557.2128464-1-bdenes@scylladb.com> References: <20200814152557.2128464-1-bdenes@scylladb.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.4 (3.36.4-1.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Aug 2020 15:27:20 -0000 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 > --- > 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 > > 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> 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 > > /* 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 raw_val(sizeof(val), 0); > + memcpy(raw_val.data(), reinterpret_cast(&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