diff mbox series

gdb/record: Support for rdtscp in i386_process_record.

Message ID 20231206152301.214669-1-cupertino.miranda@oracle.com
State New
Headers
Series gdb/record: Support for rdtscp in i386_process_record. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Cupertino Miranda Dec. 6, 2023, 3:23 p.m. UTC 
  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(-)

Comments

Cupertino Miranda Dec. 7, 2023, 8:57 a.m. UTC | #1 
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
>  };
Guinevere Larsen Dec. 7, 2023, 9:45 a.m. UTC | #2 
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>
Cupertino Miranda Dec. 7, 2023, 10:58 a.m. UTC | #3 
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>
diff mbox series

Patch

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
 };