[PATCH] gdb/record: Support for rdtscp in i386_process_record.

[PATCH] gdb/record: Support for rdtscp in i386_process_record.

[Cupertino Miranda]

Hi everyone,

Looking forward to your review.
This was detected internally presumably when debugging a system compiled
with intel compiler.

I changed one of the tests to replicate the issue.
Please advise if it is not the proper location to verify this.

Best regards,
Cupertino


This patch adds support for process recording of the instruction rdtscp in
x86 architecture.
Debugging applications with "record full" fail to record with the error
message "Process record does not support instruction 0xf01f9".
---
 gdb/i386-tdep.c                              |  8 ++++++++
 gdb/testsuite/gdb.reverse/insn-reverse-x86.c | 13 ++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index e00c3bd9d56..e379c179885 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7008,6 +7008,9 @@ Do you want to stop the program?"),
       goto no_support;
       break;
 
+    case 0x0f01f9:  /* rdtscp */
+      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
+      [[fallthrough]];
     case 0x0f31:    /* rdtsc */
       I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
       I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
@@ -7117,6 +7120,11 @@ Do you want to stop the program?"),
     case 0x0f01:
       if (i386_record_modrm (&ir))
 	return -1;
+      if (ir.modrm == 0xf9)
+	{
+	  opcode = (opcode << 8) | 0xf9;
+	  goto reswitch;
+	}
       switch (ir.reg)
 	{
 	case 0:  /* sgdt */
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
index 2b4fb4c10e0..23888ba7350 100644
--- a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
+++ b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
@@ -270,6 +270,16 @@ rdseed (void)
 #endif
 }
 
+/* Test rdtscp support.  */
+
+void
+rdtscp (void)
+{
+#ifdef __x86_64__
+  __asm__ volatile ("rdtscp");
+#endif
+}
+
 /* Initialize arch-specific bits.  */
 
 static void
@@ -283,5 +293,6 @@ initialize (void)
 static testcase_ftype testcases[] =
 {
   rdrand,
-  rdseed
+  rdseed,
+  rdtscp
 };
-- 
2.30.2

Re: [PATCH] gdb/record: Support for rdtscp in i386_process_record.

[Cupertino Miranda]

Hi Guinevere,

Sorry, I did not add you in CC in the first place.
Looking forward to your review.

Thanks,
Cupertino

Cupertino Miranda writes:

> Hi everyone,
>
> Looking forward to your review.
> This was detected internally presumably when debugging a system compiled
> with intel compiler.
>
> I changed one of the tests to replicate the issue.
> Please advise if it is not the proper location to verify this.
>
> Best regards,
> Cupertino
>
>
> This patch adds support for process recording of the instruction rdtscp in
> x86 architecture.
> Debugging applications with "record full" fail to record with the error
> message "Process record does not support instruction 0xf01f9".
> ---
>  gdb/i386-tdep.c                              |  8 ++++++++
>  gdb/testsuite/gdb.reverse/insn-reverse-x86.c | 13 ++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index e00c3bd9d56..e379c179885 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -7008,6 +7008,9 @@ Do you want to stop the program?"),
>        goto no_support;
>        break;
>
> +    case 0x0f01f9:  /* rdtscp */
> +      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
> +      [[fallthrough]];
>      case 0x0f31:    /* rdtsc */
>        I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
>        I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
> @@ -7117,6 +7120,11 @@ Do you want to stop the program?"),
>      case 0x0f01:
>        if (i386_record_modrm (&ir))
>  	return -1;
> +      if (ir.modrm == 0xf9)
> +	{
> +	  opcode = (opcode << 8) | 0xf9;
> +	  goto reswitch;
> +	}
>        switch (ir.reg)
>  	{
>  	case 0:  /* sgdt */
> diff --git a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
> index 2b4fb4c10e0..23888ba7350 100644
> --- a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
> +++ b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
> @@ -270,6 +270,16 @@ rdseed (void)
>  #endif
>  }
>
> +/* Test rdtscp support.  */
> +
> +void
> +rdtscp (void)
> +{
> +#ifdef __x86_64__
> +  __asm__ volatile ("rdtscp");
> +#endif
> +}
> +
>  /* Initialize arch-specific bits.  */
>
>  static void
> @@ -283,5 +293,6 @@ initialize (void)
>  static testcase_ftype testcases[] =
>  {
>    rdrand,
> -  rdseed
> +  rdseed,
> +  rdtscp
>  };

Re: [PATCH] gdb/record: Support for rdtscp in i386_process_record.

[Guinevere Larsen]

On 07/12/2023 09:57, Cupertino Miranda wrote:
> Hi Guinevere,
>
> Sorry, I did not add you in CC in the first place.
> Looking forward to your review.

Hi!

Don't worry, I don't think we usually add responsible maintainers to CC 
lists anyway, I had my eye on this patch already :)

>
> Thanks,
> Cupertino
>
> Cupertino Miranda writes:
>
>> Hi everyone,
>>
>> Looking forward to your review.
>> This was detected internally presumably when debugging a system compiled
>> with intel compiler.
>>
>> I changed one of the tests to replicate the issue.
>> Please advise if it is not the proper location to verify this.
>>
>> Best regards,
>> Cupertino
>>
>>
>> This patch adds support for process recording of the instruction rdtscp in
>> x86 architecture.
>> Debugging applications with "record full" fail to record with the error
>> message "Process record does not support instruction 0xf01f9".

Looks good to me. Since this only touches code relating recording,

Approved-by: Guinevere Larsen <blarsen@redhat.com>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>> ---
>>   gdb/i386-tdep.c                              |  8 ++++++++
>>   gdb/testsuite/gdb.reverse/insn-reverse-x86.c | 13 ++++++++++++-
>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index e00c3bd9d56..e379c179885 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -7008,6 +7008,9 @@ Do you want to stop the program?"),
>>         goto no_support;
>>         break;
>>
>> +    case 0x0f01f9:  /* rdtscp */
>> +      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
>> +      [[fallthrough]];
>>       case 0x0f31:    /* rdtsc */
>>         I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
>>         I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
>> @@ -7117,6 +7120,11 @@ Do you want to stop the program?"),
>>       case 0x0f01:
>>         if (i386_record_modrm (&ir))
>>   	return -1;
>> +      if (ir.modrm == 0xf9)
>> +	{
>> +	  opcode = (opcode << 8) | 0xf9;
>> +	  goto reswitch;
>> +	}
>>         switch (ir.reg)
>>   	{
>>   	case 0:  /* sgdt */
>> diff --git a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
>> index 2b4fb4c10e0..23888ba7350 100644
>> --- a/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
>> +++ b/gdb/testsuite/gdb.reverse/insn-reverse-x86.c
>> @@ -270,6 +270,16 @@ rdseed (void)
>>   #endif
>>   }
>>
>> +/* Test rdtscp support.  */
>> +
>> +void
>> +rdtscp (void)
>> +{
>> +#ifdef __x86_64__
>> +  __asm__ volatile ("rdtscp");
>> +#endif
>> +}
>> +
>>   /* Initialize arch-specific bits.  */
>>
>>   static void
>> @@ -283,5 +293,6 @@ initialize (void)
>>   static testcase_ftype testcases[] =
>>   {
>>     rdrand,
>> -  rdseed
>> +  rdseed,
>> +  rdtscp
>>   };

Re: [PATCH] gdb/record: Support for rdtscp in i386_process_record.

[Cupertino Miranda]

Thanks for your attention and fast reviewing.

Committed!

Cupertino

Guinevere Larsen writes:

> On 07/12/2023 09:57, Cupertino Miranda wrote:
>> Hi Guinevere,
>>
>> Sorry, I did not add you in CC in the first place.
>> Looking forward to your review.
>
> Hi!
>
> Don't worry, I don't think we usually add responsible maintainers to CC lists
> anyway, I had my eye on this patch already :)
>
>>
>> Thanks,
>> Cupertino
>>
>> Cupertino Miranda writes:
>>
>>> Hi everyone,
>>>
>>> Looking forward to your review.
>>> This was detected internally presumably when debugging a system compiled
>>> with intel compiler.
>>>
>>> I changed one of the tests to replicate the issue.
>>> Please advise if it is not the proper location to verify this.
>>>
>>> Best regards,
>>> Cupertino
>>>
>>>
>>> This patch adds support for process recording of the instruction rdtscp in
>>> x86 architecture.
>>> Debugging applications with "record full" fail to record with the error
>>> message "Process record does not support instruction 0xf01f9".
>
> Looks good to me. Since this only touches code relating recording,
>
> Approved-by: Guinevere Larsen <blarsen@redhat.com>