[PATCH 1/1] [RFC] gdb: corelow: make possible to modify (set) registers for a corefile

[PATCH 1/1] [RFC] gdb: corelow: make possible to modify (set) registers for a corefile

[Roman Pen]

Despite the fact that this is a hairy hack this change eases debugging
of a jmp_buf (setjmp()) and user contexts (makecontext()), which are
highly used in QEMU project as a part of coroutines.

This change allows setting registers for a corefile, thus QEMU gdb
script (qemu/scripts/qemugdb/coroutine.py) is allowed to investigate
backtrace of a preempted coroutine context.  Previously only debugging
of a live process was allowed.

This patch caches all register on a first attempt to modify register
'(gdb) set $REG = ADDR' and then cached copy is always returned from
get_core_registers().

This change should not break previous behaviour if nobody sets any
register, i.e. on each get_core_registers() call registers from a
corefile will be reread.

Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Pedro Alves <palves@redhat.com>
Cc: Daniel Jacobowitz <dan@codesourcery.com>
Cc: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org

QEMU guys who can be interested in this new gdb behaviour:

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 gdb/corelow.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index c46af0a8a59d..4a4b20ed57a0 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -74,6 +74,15 @@ static struct gdbarch *core_gdbarch = NULL;
    unix child targets.  */
 static struct target_section_table *core_data;
 
+/* Cached registers. Once registers are modified (set) for a corefile,
+   they are cached and then are always fetched from get_core_registers().
+   This hairy hack is used only for one purpose: give a possibility to
+   debug jmp_buf (setjmp()) and user contexts (makecontext()). */
+
+static struct cached_reg {
+	char regp[MAX_REGISTER_SIZE];
+} *core_cachedregs;
+
 static void core_files_info (struct target_ops *);
 
 static struct core_fns *sniff_core_bfd (bfd *);
@@ -213,6 +222,11 @@ core_close (struct target_ops *self)
     }
   core_vec = NULL;
   core_gdbarch = NULL;
+  if (core_cachedregs)
+    {
+      xfree(core_cachedregs);
+      core_cachedregs = NULL;
+    }
 }
 
 static void
@@ -620,6 +634,18 @@ get_core_registers (struct target_ops *ops,
     }
 
   gdbarch = get_regcache_arch (regcache);
+
+  if (core_cachedregs)
+    {
+      /* If registers were once modified (set) for a corefile,
+         follow this path and always return cached registers */
+
+      for (i = 0; i < gdbarch_num_regs(gdbarch); i++)
+        regcache_raw_supply(regcache, i, &core_cachedregs[i]);
+
+      return;
+    }
+
   if (gdbarch_iterate_over_regset_sections_p (gdbarch))
     gdbarch_iterate_over_regset_sections (gdbarch,
 					  get_core_registers_cb,
@@ -639,6 +665,29 @@ get_core_registers (struct target_ops *ops,
 }
 
 static void
+set_core_registers (struct target_ops *self, struct regcache *regcache,
+                    int regnum)
+{
+  struct gdbarch *gdbarch;
+  int i;
+
+  gdbarch = get_regcache_arch(regcache);
+
+  if (core_cachedregs == NULL)
+    core_cachedregs = (struct cached_reg*)xcalloc(gdbarch_num_regs(gdbarch),
+                                                  sizeof(*core_cachedregs));
+
+  for (i = 0; i < gdbarch_num_regs(gdbarch); i++)
+    regcache_raw_collect(regcache, i, &core_cachedregs[i]);
+}
+
+static void
+prepare_core_registers (struct target_ops *self, struct regcache *arg1)
+{
+    /* nothing here */
+}
+
+static void
 core_files_info (struct target_ops *t)
 {
   print_section_info (core_data, core_bfd);
@@ -1050,6 +1099,8 @@ init_core_ops (void)
   core_ops.to_close = core_close;
   core_ops.to_detach = core_detach;
   core_ops.to_fetch_registers = get_core_registers;
+  core_ops.to_store_registers = set_core_registers;
+  core_ops.to_prepare_to_store = prepare_core_registers;
   core_ops.to_xfer_partial = core_xfer_partial;
   core_ops.to_files_info = core_files_info;
   core_ops.to_insert_breakpoint = ignore;
-- 
2.11.0

Re: [PATCH 1/1] [RFC] gdb: corelow: make possible to modify (set) registers for a corefile

[Ulrich Weigand]

Roman Pen wrote:

> Despite the fact that this is a hairy hack this change eases debugging
> of a jmp_buf (setjmp()) and user contexts (makecontext()), which are
> highly used in QEMU project as a part of coroutines.
> 
> This change allows setting registers for a corefile, thus QEMU gdb
> script (qemu/scripts/qemugdb/coroutine.py) is allowed to investigate
> backtrace of a preempted coroutine context.  Previously only debugging
> of a live process was allowed.
> 
> This patch caches all register on a first attempt to modify register
> '(gdb) set $REG = ADDR' and then cached copy is always returned from
> get_core_registers().
> 
> This change should not break previous behaviour if nobody sets any
> register, i.e. on each get_core_registers() call registers from a
> corefile will be reread.

I'm wondering why you need that extra copy of the registers;
there already should be a regcache that would be able to hold
any modified values.

It is not currently possible to actually change those values
in the regcache because there is no to_store_registers routine.
But simply adding such a routine that does nothing (just like
to_prepare_to_store in your patch) should hopefully be enough ...

In any case, it would be good to add or extend a test case to
verify that this feature is working as intended.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/1] [RFC] gdb: corelow: make possible to modify (set) registers for a corefile

[Roman Penyaev]

On Tue, Mar 14, 2017 at 2:14 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Roman Pen wrote:
>
>> Despite the fact that this is a hairy hack this change eases debugging
>> of a jmp_buf (setjmp()) and user contexts (makecontext()), which are
>> highly used in QEMU project as a part of coroutines.
>>
>> This change allows setting registers for a corefile, thus QEMU gdb
>> script (qemu/scripts/qemugdb/coroutine.py) is allowed to investigate
>> backtrace of a preempted coroutine context.  Previously only debugging
>> of a live process was allowed.
>>
>> This patch caches all register on a first attempt to modify register
>> '(gdb) set $REG = ADDR' and then cached copy is always returned from
>> get_core_registers().
>>
>> This change should not break previous behaviour if nobody sets any
>> register, i.e. on each get_core_registers() call registers from a
>> corefile will be reread.
>
> I'm wondering why you need that extra copy of the registers;
> there already should be a regcache that would be able to hold
> any modified values.
>
> It is not currently possible to actually change those values
> in the regcache because there is no to_store_registers routine.
> But simply adding such a routine that does nothing (just like
> to_prepare_to_store in your patch) should hopefully be enough ...

Unfortunately it is not.  'regcache' argument, which is passed to
get_core_registers(), has all registers set to zero, i.e. registers
are not preserved between 'to_fetch_registers' and 'to_store_registers'
calls.  Thus on each get_core_registers() call we have to fill in
'regcache'.  By default registers are always fetched from a corefile,
which prevents modifying them, that's why I need some extra registers
array.

>
> In any case, it would be good to add or extend a test case to
> verify that this feature is working as intended.

Good point, I will take a look.

Thanks.

--
Roman