This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch v4 11/24] record-btrace: supply register target methods
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Markus Metzger <markus dot t dot metzger at intel dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sun, 18 Aug 2013 21:07:27 +0200
- Subject: Re: [patch v4 11/24] record-btrace: supply register target methods
- References: <1372842874-28951-1-git-send-email-markus dot t dot metzger at intel dot com> <1372842874-28951-12-git-send-email-markus dot t dot metzger at intel dot com>
On Wed, 03 Jul 2013 11:14:21 +0200, Markus Metzger wrote:
> Supply target methods to allow reading the PC. Forbid anything else.
>
> 2013-07-03 Markus Metzger <markus.t.metzger@intel.com>
>
> * record-btrace.c (record_btrace_fetch_registers,
> record_btrace_store_registers,
> record_btrace_to_prepare_to_store): New.
> (init_record_btrace_ops): Add the above.
>
>
> ---
> gdb/record-btrace.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 95 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index 5e41b20..e9c0801 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -32,6 +32,7 @@
> #include "ui-out.h"
> #include "symtab.h"
> #include "filenames.h"
> +#include "regcache.h"
>
> /* The target_ops of record-btrace. */
> static struct target_ops record_btrace_ops;
> @@ -752,6 +753,97 @@ record_btrace_is_replaying (void)
> return 0;
> }
>
> +/* The to_fetch_registers method of target record-btrace. */
> +
> +static void
> +record_btrace_fetch_registers (struct target_ops *ops,
> + struct regcache *regcache, int regno)
> +{
> + struct btrace_insn_iterator *replay;
> + struct thread_info *tp;
> +
> + tp = find_thread_ptid (inferior_ptid);
> + if (tp == NULL)
> + return;
Are you aware when it can happen? If not then:
gdb_assert (tp != NULL);
> +
> + replay = tp->btrace.replay;
> + if (replay != NULL)
> + {
> + const struct btrace_insn *insn;
> + struct gdbarch *gdbarch;
> + int pcreg;
> +
> + gdbarch = get_regcache_arch (regcache);
> + pcreg = gdbarch_pc_regnum (gdbarch);
> + if (pcreg < 0)
> + return;
> +
> + /* We can only provide the PC register. */
> + if (regno >= 0 && regno != pcreg)
> + return;
> +
> + insn = btrace_insn_get (replay);
> + if (insn == NULL)
> + return;
Shouldn't here be rather an error?
> +
> + regcache_raw_supply (regcache, regno, &insn->pc);
> + }
> + else
> + {
> + struct target_ops *t;
> +
> + for (t = ops->beneath; t != NULL; t = t->beneath)
> + if (t->to_fetch_registers != NULL)
> + {
> + t->to_fetch_registers (t, regcache, regno);
> + break;
> + }
> + }
> +}
> +
> +/* The to_store_registers method of target record-btrace. */
> +
> +static void
> +record_btrace_store_registers (struct target_ops *ops,
> + struct regcache *regcache, int regno)
> +{
> + struct target_ops *t;
> +
> + if (record_btrace_is_replaying ())
> + return;
Currently I get:
(gdb) p $rax
$1 = <unavailable>
(gdb) p $rax=1
$2 = <unavailable>
I would find more appropriate an error() here so that we get:
(gdb) p $rax
$1 = <unavailable>
(gdb) p $rax=1
Some error message.
With gdbserver trace one gets:
(gdb) print globalc
$1 = <unavailable>
(gdb) print globalc=1
Cannot access memory at address 0x602120
which is not so convenient as it comes from gdbserver E01 response:
gdb_write_memory -> if (current_traceframe >= 0) return EIO;
as I checked.
> +
> + if (may_write_registers == 0)
> + error (_("Writing to registers is not allowed (regno %d)"), regno);
Here should be rather:
gdb_assert (may_write_registers == 0);
as target_store_registers() would not pass the call here otherwise.
> +
> + for (t = ops->beneath; t != NULL; t = t->beneath)
> + if (t->to_store_registers != NULL)
> + {
> + t->to_store_registers (t, regcache, regno);
> + return;
> + }
> +
> + noprocess ();
> +}
> +
> +/* The to_prepare_to_store method of target record-btrace. */
> +
> +static void
> +record_btrace_prepare_to_store (struct target_ops *ops,
> + struct regcache *regcache)
> +{
> + struct target_ops *t;
> +
> + if (record_btrace_is_replaying ())
> + return;
> +
> + for (t = ops->beneath; t != NULL; t = t->beneath)
> + if (t->to_prepare_to_store != NULL)
> + {
> + t->to_prepare_to_store (t, regcache);
> + return;
> + }
> +}
> +
> /* Initialize the record-btrace target ops. */
>
> static void
> @@ -779,6 +871,9 @@ init_record_btrace_ops (void)
> ops->to_call_history_from = record_btrace_call_history_from;
> ops->to_call_history_range = record_btrace_call_history_range;
> ops->to_record_is_replaying = record_btrace_is_replaying;
> + ops->to_fetch_registers = record_btrace_fetch_registers;
> + ops->to_store_registers = record_btrace_store_registers;
> + ops->to_prepare_to_store = record_btrace_prepare_to_store;
> ops->to_stratum = record_stratum;
> ops->to_magic = OPS_MAGIC;
> }
> --
> 1.7.1