Re: [PATCH] gdb: add 'maintenance print record-instruction' command

[Bruno Larsen <blarsen@redhat.com>]

On 07/12/2022 15:37, Lancelot SIX wrote:
> Hi,
>
> I do not not know this part well (to say the least), so my comments will
> only focus on style
>
> On Wed, Dec 07, 2022 at 02:50:00PM +0100, Bruno Larsen via Gdb-patches wrote:
>> While chasing some reverse debugging bugs, I found myself wondering what
>> was recorded by GDB to undo and redo a certain instruction. This commit
>> implements a simple way of printing that information.
>> ---
>>   gdb/NEWS            |  6 ++++
>>   gdb/doc/gdb.texinfo |  8 +++++
>>   gdb/record-full.c   | 81 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 95 insertions(+)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index c4ccfcc9e32..d6ce6bf86a0 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -103,6 +103,12 @@
>>   
>>   * New commands
>>   
>> +maintenance print record-instruction [ N ]
>> +  Print the recorded information for a given instruction. If N is not given
>> +  prints how GDB would undo the last instruction executed. If N is negative,
>> +  prints how GDB would undo the N-th previous instruction, and if N is
>> +  positive, it prints how GDB will redo the N-th following instruction.
>> +
>>   maintenance set ignore-prologue-end-flag on|off
>>   maintenance show ignore-prologue-end-flag
>>     This setting, which is off by default, controls whether GDB ignores the
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 5b566669975..807af351e79 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -40531,6 +40531,14 @@ that symbol is described.  The type chain produced by this command is
>>   a recursive definition of the data type as stored in @value{GDBN}'s
>>   data structures, including its flags and contained types.
>>   
>> +@kindex maint print record-instruction
>> +@item maint print record-instruction
>> +@itemx maint print record-instruction @var{N}
>> +@cindex print how GDB recorded a given instruction.  If N is not positive
>> +number, it prints the values stored by the inferior before the N-th previous
>> +instruction was exectued.  If N is positive, print the values after the N-th
>> +following instruction is executed.  If N is not given, 0 is assumed.
>> +
>>   @kindex maint selftest
>>   @cindex self tests
>>   @item maint selftest @r{[}-verbose@r{]} @r{[}@var{filter}@r{]}
>> diff --git a/gdb/record-full.c b/gdb/record-full.c
>> index 48b92281fe6..47cdf75eea4 100644
>> --- a/gdb/record-full.c
>> +++ b/gdb/record-full.c
>> @@ -2764,6 +2764,79 @@ set_record_full_insn_max_num (const char *args, int from_tty,
>>       }
>>   }
>>   
>> +/* Implement the 'maintenance print record-instruction' command.  */
>> +
>> +static void
>> +maintenance_print_record_instruction (const char *args, int from_tty)
> I think, usually the command implementation have a _command suffix in
> the name.  I do not know if this is a hard rule.
None maintenance_print functions end with _command, so I don't think it 
is a hard rule, but if you think I should add it, I can do it.

I agree with everything else you said, it'll all be fixed in v2.

-- 
Cheers,
Bruno