Re: [PATCH] arm reversible : <phase_2_complete>

Petr Hluzín
2011-12-03T20:30:57Z

On 3 December 2011 20:01, oza Pawandeep <oza.pawandeep@gmail.com> wrote:
> Hi Tom and Petr,
>
> This patch includes both of your comments; I have worked both on
> formatting and comments, and try to make the patch look ok.
> the patch is derived from gdb-7.3.50.20111203 current snapshot.
>

In function decode_insn:
> +
> +  struct
> +    {
> +      gdb_byte buf[insn_size];
> +    } u_buf;
> +
> +  uint32_t ret=0, insn_id = 0;
> +
> +  memset (&u_buf, 0, sizeof(u_buf));
> +  if (target_read_memory (arm_record->this_addr, &u_buf.buf[0], insn_size))

I wonder why is there a `struct u_buf'. Having local variable
`buf[insn_size];' would be sufficient and obvious. I am sorry to not
discover that earlier. The same thing applies to arm_process_record().

In arm_process_record ()
> +
> +  struct
> +      {
> +        gdb_byte buf[2];
> +      } u_buf;
> +
> +  ...
> +
> +  arm_record.arm_insn = (uint32_t) extract_unsigned_integer (&u_buf.buf[0],
> +                                   THUMB_INSN_SIZE_BYTES ,
> +                                   gdbarch_byte_order (arm_record.gdbarch));

Well, when I said that you probably forgot to copy
extract_unsigned_integer() I should have also said that you should
have also copied the line
target_read_memory (arm_record->this_addr, &u_buf.buf[0], insn_size)
Right now extract_unsigned_integer() reads an uninitialized buffer. :-T

--
Petr Hluzin