This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/6] disasm: add struct disas_insn to describe to-be-disassembled instruction
- From: Pedro Alves <palves at redhat dot com>
- To: Markus Metzger <markus dot t dot metzger at intel dot com>, dje at google dot com
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 09 Oct 2015 13:51:12 +0100
- Subject: Re: [PATCH 2/6] disasm: add struct disas_insn to describe to-be-disassembled instruction
- Authentication-results: sourceware.org; auth=none
- References: <1442847283-10200-1-git-send-email-markus dot t dot metzger at intel dot com> <1442847283-10200-3-git-send-email-markus dot t dot metzger at intel dot com>
On 09/21/2015 03:54 PM, Markus Metzger wrote:
> Add a new struct disas_insn to add additional fields describing the
> to-be-disassembled instruction. The additional fields are:
>
> number an optional instruction number, zero if omitted.
> is_speculative a predicate saying whether the instruction was
> executed speculatively.
>
> Replace the PC parameter of dump_insn with a pointer to the above struct.
>
> If non-zero, the instruction number is printed first. It will also appear
> as a new optional field "insn-number" in MI. The field will be present if
> insn_num is non-zero.
>
> If is_speculative is set, speculative execution will be indicated by a "?"
> following the new instruction number field. Unless the PC is omitted, it
> will overwrite the first byte of the PC prefix. It will appear as a new
> optional field "is-speculative" in MI. The field will contain "?" and will
> be present if is_speculative is set.
>
I think the log would be much clearer if the rationale was specified in
terms of why this is necessary, and if we saw a before/after example.
Also, being a user/frontend visible change, shouldn't these new
fields be documented and mentioned in NEWS?
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -27,11 +27,25 @@
> #define DISASSEMBLY_FILENAME (0x1 << 3)
> #define DISASSEMBLY_OMIT_PC (0x1 << 4)
> #define DISASSEMBLY_SOURCE (0x1 << 5)
> +#define DISASSEMBLY_SPECULATIVE (0x1 << 6)
>
> struct gdbarch;
> struct ui_out;
> struct ui_file;
>
> +/* An instruction to be disassembled. */
> +
> +struct disas_insn {
{ goes on the next line.
Thanks,
Pedro Alves