| Message ID | C7AECD2C-570A-4934-A153-FFA8B66E8185@arm.com |
|---|---|
| State | New |
| Headers | show |
PING > On 12 Jun 2017, at 14:59, Alan Hayward <Alan.Hayward@arm.com> wrote: > > >> On 12 Jun 2017, at 12:05, Yao Qi <qiyaoltc@gmail.com> wrote: >> >> Alan Hayward <Alan.Hayward@arm.com> writes: >> >>> - regcache_cooked_read (regcache, entry->u.reg.num, reg); >>> - regcache_cooked_write (regcache, entry->u.reg.num, >>> - record_full_get_loc (entry)); >>> - memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); >> >> The original code is about swapping contents of register REG in regcache >> and record_full_get_loc (entry), and the length is known entry->u.reg.len. >> > > Yes. > >>> + value = regcache->cooked_read_value (entry->u.reg.num); >>> + gdb_assert (value != NULL); >>> + regcache->cooked_write (entry->u.reg.num, record_full_get_loc (entry)); >>> + memcpy (record_full_get_loc (entry), value_contents_all (value), >>> + entry->u.reg.len); >>> + release_value (value); >>> + value_free (value); >> >> It is a overkill to use value to swap these two buffers, IMO. How about >> xmalloc "reg" instead? >> > > Given that the code doesn’t use any of the error checking, then agreed. > > > Tested on a --enable-targets=all build with board files unix and > native-gdbserver. > > > Ok to commit? > Alan. > > 2017-06-12 Alan Hayward <alan.hayward@arm.com> > > * gdb/record-full.c (record_full_exec_insn): Allocate buffer. > > > diff --git a/gdb/record-full.c b/gdb/record-full.c > index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 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]; > + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); > > if (record_debug > 1) > fprintf_unfiltered (gdb_stdlog, > @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, > regcache_cooked_write (regcache, entry->u.reg.num, > record_full_get_loc (entry)); > memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); > + xfree (reg); > } > break; > > >
Alan Hayward <Alan.Hayward@arm.com> writes: > 2017-06-12 Alan Hayward <alan.hayward@arm.com> > > * gdb/record-full.c (record_full_exec_insn): Allocate buffer. Patch is good to me.
On 06/12/2017 02:59 PM, Alan Hayward wrote: > diff --git a/gdb/record-full.c b/gdb/record-full.c > index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 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]; > + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); > > if (record_debug > 1) > fprintf_unfiltered (gdb_stdlog, > @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, > regcache_cooked_write (regcache, entry->u.reg.num, > record_full_get_loc (entry)); > memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); > + xfree (reg); You can use gdb::byte_vector reg (entry->u.reg.len); to avoid the explicit xfree here. (and a potential leak if regcache_* throws). Thanks, Pedro Alves
diff --git a/gdb/record-full.c b/gdb/record-full.c index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 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]; + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); if (record_debug > 1) fprintf_unfiltered (gdb_stdlog, @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, regcache_cooked_write (regcache, entry->u.reg.num, record_full_get_loc (entry)); memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); + xfree (reg); } break;