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: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>, "dje at google dot com" <dje at google dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 20 Oct 2015 12:29:15 +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> <5617B840 dot 7040504 at redhat dot com> <A78C989F6D9628469189715575E55B23331E6065 at IRSMSX104 dot ger dot corp dot intel dot com>
(Closing the loop on v1... Sorry for the delay.)
On 10/12/2015 09:44 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Pedro Alves
>> Sent: Friday, October 9, 2015 2:51 PM
>> To: Metzger, Markus T; dje@google.com
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH 2/6] disasm: add struct disas_insn to describe to-be-
>> disassembled instruction
>
>
>> 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.
>
> I added the following to the beginning of the commit message to motivate
> the patch:
>
> The "record instruction-history" command prints for each instruction, in
> addition to the instruction's disassembly:
>
> - the instruction number in the recorded execution trace
> - a '?' before the instruction if it was executed speculatively
>
> To allow the "record instruction-history" command to use GDB's disassembly
> infrastructure, we extend dump_insn to optionally print those additional
> fields.
Ah, that's clears things, thanks.
>
>
>> Also, being a user/frontend visible change, shouldn't these new
>> fields be documented and mentioned in NEWS?
>
> There is no UI change and thus also no before/after example.
>
> The new optional fields are currently not used. They will be used by the
> "record instruction-history" command in the last patch of this series.
> Even then, there is no UI change. Both the "record instruction-history"
> and the "disassemble" command behave as they did before.
>
OK.
> There is a change to the MI output of "record instruction-history".
> As I didn't do any conscious MI support for record btrace, I don't expect
> it to be working. I'm using the ui_out_* functions so there might be some
> form of MI support. AFAIK it is not being used.
I see. Note that when an MI frontend invokes a CLI command (through
-interpreter-exec console ..."), the output it gets is still CLI output.
The fields passed to ui_out_* functions only get converted to MI attributes
if the command entered was a real MI command. IOW, if there's no MI command
equivalent of "record instruction-history", then there's no such thing as
'MI output of "record instruction-history"'.
This sentence:
"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."
was what made me believe there was some MI command that would now output
that field. But it now sounds to me that e.g., -data-disassemble output does
not really change. (If it does change, then we need to extend the manual where
it documents the "Result" of that command (see "GDB/MI Data Manipulation").)
Thanks,
Pedro Alves