diff mbox

Remove MAX_REGISTER_SIZE from record-full.c

Message ID 92701E3D-1AB0-44F6-B3C8-6D2A8E489985@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Feb. 24, 2017, 10:21 a.m. UTC 
  Cannot avoid using buffer in record_full_exec_insn because it is doing an
in place swap.

Tested using make check with board files unix and native-gdbserver.

Ok to commit?

Alan.


2017-02-24  Alan Hayward  <alan.hayward@arm.com>

	* record-full.c (record_full_exec_insn): Use vec instead of array.
	(record_full_core_open_1): Call max_register_size.
	(record_full_core_fetch_registers): Likewise.
	(record_full_core_store_registers): Likewise.

Comments

Yao Qi March 1, 2017, 1:06 p.m. UTC | #1 
Alan Hayward <Alan.Hayward@arm.com> writes:

> Cannot avoid using buffer in record_full_exec_insn because it is doing an
> in place swap.

Your patch does change something other than record_full_exec_insn.  Need
more descriptions about them.

> @@ -792,15 +792,17 @@ static void
>  record_full_core_open_1 (const char *name, int from_tty)
>  {
>    struct regcache *regcache = get_current_regcache ();
> -  int regnum = gdbarch_num_regs (get_regcache_arch (regcache));
> +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +  int regnum = gdbarch_num_regs (gdbarch);
> +  int regmaxsize = max_register_size (gdbarch);
>    int i;
>
>    /* Get record_full_core_regbuf.  */
>    target_fetch_registers (regcache, -1);
> -  record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum);

We can see record_full_core_regbuf is allocated in target open, and
released on target close, used in fetch registers and store registers.
Can we replace it with a regcache?  In target open, copy
get_current_regcache () to it, and release it on target close.  In
target fetch/store registers, use it as well.

> @@ -2263,25 +2271,6 @@ init_record_full_core_ops (void)
>  }
>
>  /* Record log save-file format
> -   Version 1 (never released)
> -
> -   Header:
> -     4 bytes: magic number htonl(0x20090829).
> -       NOTE: be sure to change whenever this file format changes!
> -
> -   Records:
> -     record_full_end:
> -       1 byte:  record type (record_full_end, see enum record_full_type).
> -     record_full_reg:
> -       1 byte:  record type (record_full_reg, see enum record_full_type).
> -       8 bytes: register id (network byte order).
> -       MAX_REGISTER_SIZE bytes: register value.
> -     record_full_mem:
> -       1 byte:  record type (record_full_mem, see enum record_full_type).
> -       8 bytes: memory length (network byte order).
> -       8 bytes: memory address (network byte order).
> -       n bytes: memory value (n == memory length).
> -

Although format V1 is never released, and is not being used, we can't
simply remove it without any justifications.  They were added in
https://sourceware.org/ml/gdb-patches/2009-10/msg00380.html but nobody
raised the question why do we need this v1 format.

Hui, do you recall any reason that we keep v1 format in comment?  If
not, do you think we can remove these comments?  The comments go into
Alan's radar because MAX_REGISTER_SIZE is used in the comments and he
wants to remove MAX_REGISTER_SIZE.
diff mbox

Patch

diff --git a/gdb/record-full.c b/gdb/record-full.c
index bd95acc6b1854753eb5da541cd7934eb3b8c9113..290d833c36e453111d240a412feb21c93c7af617 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -698,7 +698,7 @@  record_full_exec_insn (struct regcache *regcache,
     {
     case record_full_reg: /* reg */
       {
-        gdb_byte reg[MAX_REGISTER_SIZE];
+	std::vector<gdb_byte> reg (register_size (gdbarch, entry->u.reg.num));

         if (record_debug > 1)
           fprintf_unfiltered (gdb_stdlog,
@@ -707,10 +707,10 @@  record_full_exec_insn (struct regcache *regcache,
                               host_address_to_string (entry),
                               entry->u.reg.num);

-        regcache_cooked_read (regcache, entry->u.reg.num, reg);
-        regcache_cooked_write (regcache, entry->u.reg.num,
+	regcache_cooked_read (regcache, entry->u.reg.num, reg.data ());
+	regcache_cooked_write (regcache, entry->u.reg.num,
 			       record_full_get_loc (entry));
-        memcpy (record_full_get_loc (entry), reg, entry->u.reg.len);
+	memcpy (record_full_get_loc (entry), reg.data (), entry->u.reg.len);
       }
       break;

@@ -792,15 +792,17 @@  static void
 record_full_core_open_1 (const char *name, int from_tty)
 {
   struct regcache *regcache = get_current_regcache ();
-  int regnum = gdbarch_num_regs (get_regcache_arch (regcache));
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  int regnum = gdbarch_num_regs (gdbarch);
+  int regmaxsize = max_register_size (gdbarch);
   int i;

   /* Get record_full_core_regbuf.  */
   target_fetch_registers (regcache, -1);
-  record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum);
+  record_full_core_regbuf = (gdb_byte *) xmalloc (regmaxsize * regnum);
   for (i = 0; i < regnum; i ++)
     regcache_raw_collect (regcache, i,
-			  record_full_core_regbuf + MAX_REGISTER_SIZE * i);
+			  record_full_core_regbuf + regmaxsize * i);

   /* Get record_full_core_start and record_full_core_end.  */
   if (build_section_table (core_bfd, &record_full_core_start,
@@ -2047,6 +2049,9 @@  record_full_core_fetch_registers (struct target_ops *ops,
 				  struct regcache *regcache,
 				  int regno)
 {
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  int regmaxsize = max_register_size (gdbarch);
+
   if (regno < 0)
     {
       int num = gdbarch_num_regs (get_regcache_arch (regcache));
@@ -2054,11 +2059,11 @@  record_full_core_fetch_registers (struct target_ops *ops,

       for (i = 0; i < num; i ++)
         regcache_raw_supply (regcache, i,
-                             record_full_core_regbuf + MAX_REGISTER_SIZE * i);
+			     record_full_core_regbuf + regmaxsize * i);
     }
   else
     regcache_raw_supply (regcache, regno,
-                         record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
+			 record_full_core_regbuf + regmaxsize * regno);
 }

 /* "to_prepare_to_store" method for prec over corefile.  */
@@ -2076,9 +2081,12 @@  record_full_core_store_registers (struct target_ops *ops,
                              struct regcache *regcache,
                              int regno)
 {
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  int regmaxsize = max_register_size (gdbarch);
+
   if (record_full_gdb_operation_disable)
     regcache_raw_collect (regcache, regno,
-                          record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
+			  record_full_core_regbuf + regmaxsize * regno);
   else
     error (_("You can't do that without a process to debug."));
 }
@@ -2263,25 +2271,6 @@  init_record_full_core_ops (void)
 }

 /* Record log save-file format
-   Version 1 (never released)
-
-   Header:
-     4 bytes: magic number htonl(0x20090829).
-       NOTE: be sure to change whenever this file format changes!
-
-   Records:
-     record_full_end:
-       1 byte:  record type (record_full_end, see enum record_full_type).
-     record_full_reg:
-       1 byte:  record type (record_full_reg, see enum record_full_type).
-       8 bytes: register id (network byte order).
-       MAX_REGISTER_SIZE bytes: register value.
-     record_full_mem:
-       1 byte:  record type (record_full_mem, see enum record_full_type).
-       8 bytes: memory length (network byte order).
-       8 bytes: memory address (network byte order).
-       n bytes: memory value (n == memory length).
-
    Version 2
      4 bytes: magic number netorder32(0x20091016).
        NOTE: be sure to change whenever this file format changes!