RE: [PATCH v2] Fix alignment of disassemble /r

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: Leonardo Boquillon
> [mailto:leonardo.boquillon@tallertechnologies.com]
> Sent: Wednesday, April 6, 2016 7:47 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2] Fix alignment of disassemble /r
> 
> Hi Markus,
> I sketched a function to compute the opcode length on record-btrace [1] but
> I'm not sure how to test it. Could you please give me a hint about how to?
> Can be it tested over x86?

Hi Leonardo,

Thanks for adding this.

For testing, you'd need a 4th Generation Core (Haswell) or later or any Atom
processor.  The code is used by the "record instruction-history" command
which requires the "btrace" recording method; use "record btrace" to start
recording.  There are tests in gdb/testsuite/gdb.btrace that might serve as
template.

If you don't have the necessary hardware for testing I can do this for you.

> static size_t
> get_insn_set_longest_opcode(struct gdbarch *gdbarch, struct
> disassemble_info* di,
>                             const struct btrace_insn_iterator *begin,
>                             const struct btrace_insn_iterator *end)

Indentation is off.

> {
>   size_t longest_opcode = 0;

How about int instead of size_t to avoid conversions below?

>   struct btrace_insn_iterator it;
> 
>   for (it = *begin; btrace_insn_cmp (&it, end) != 0; btrace_insn_next (&it, 1))
>     {
>       const struct btrace_insn *insn = btrace_insn_get (&it);
> 
>       if (insn != NULL)
> {
>  const int current_length = gdbarch_print_insn(gdbarch, insn->pc, di);

I'd use gdb_insn_length here.

>  longest_opcode =
>    (current_length > longest_opcode) ? current_length : longest_opcode;
> }
>     }
> 
>   return longest_opcode;
> }

Looks good.  This should also simplify your patch as we won't need two
versions of gdb_pretty_print_insn anymore.

Looking forward to the next version of your patch.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928