[PATCH] Add support for recording xsave x86 instruction

[PATCH] Add support for recording xsave x86 instruction

[Pierre Marsais]

Latest version of glibc's ld.so use the xsave instruction in the
resolver. This breaks gdb record when calling shared libraries:

```
$ gcc -o fail -ggdb -x c - <<EOF
#include <stdlib.h>

int main() {
        exit(0);
}
EOF
$ gdb ./fail
Reading symbols from ./fail...done.
(gdb) b main
Breakpoint 1 at 0x113d: file <stdin>, line 4.
(gdb) r
Starting program: /tmp/fail

Breakpoint 1, main () at <stdin>:4
4       <stdin>: No such file or directory.
(gdb) record
(gdb) c
Continuing.
Process record does not support instruction 0xfae64 at address
0x7ffff7fe96dc.
```

In order to record xsave instructions, we record the first 512 bytes of
legacy XSAVE Area and the following 64 bytes of XSAVE Header, and for
each the feature of bit set of xcr0. At the moment we don't check if
the user requested to save less fields, we record all the supported
fields.

gdb/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* i386-tdep.c: Include "nat/x86-cpuid.h".
	(i386_process_record): Handle xsave instruction.

gdb/testsuite/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* gdb.reverse/i386-xsave-reverse.c: New file.
	* gdb.reverse/i386-xsave-reverse.exp: New file.
---
 gdb/i386-tdep.c                               | 23 ++++++
 .../gdb.reverse/i386-xsave-reverse.c          | 34 +++++++++
 .../gdb.reverse/i386-xsave-reverse.exp        | 75 +++++++++++++++++++
 3 files changed, 132 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..78dbbfe5f0 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -31,6 +31,7 @@
 #include "gdbcmd.h"
 #include "gdbcore.h"
 #include "gdbtypes.h"
+#include "nat/x86-cpuid.h"
 #include "objfiles.h"
 #include "osabi.h"
 #include "regcache.h"
@@ -7385,6 +7386,28 @@ no_support_3dnow_data:
             return -1;
           break;
 
+        case 4: /* xsave */
+          uint64_t tmpu64;
+          if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+            return -1;
+          if (record_full_arch_list_add_mem (tmpu64, 512 + 64))
+            return -1;
+
+          for (int i = 2; i < 64; i++) {
+            if (!((1 << i) & tdep->xcr0))
+              continue;
+
+            unsigned int size, offset, tmp1, tmp2;
+
+            if (!__get_cpuid_count(0xd, i, &size, &offset, &tmp1, &tmp2))
+              return -1;
+
+            if (record_full_arch_list_add_mem (tmpu64 + offset, size))
+              return -1;
+          }
+
+          break;
+
         case 5:    /* lfence */
         case 6:    /* mfence */
         case 7:    /* sfence clflush */
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
new file mode 100644
index 0000000000..d0e87158a2
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
@@ -0,0 +1,34 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+void xsave_test(void) {
+	char buf[4096] __attribute__ ((aligned (64))) = { 0 };
+
+	asm ("xor %%eax, %%eax\n\t"
+	     "not %%eax\n\t"
+	     "mov %%eax, %%edx\n\t"
+	     "xsave %0":"=m"(buf) ::"eax", "edx");
+} /* end xsave_test */
+
+int
+main ()
+{
+  xsave_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
new file mode 100644
index 0000000000..3ea8935c0e
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
@@ -0,0 +1,75 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+if ![supports_reverse] {
+    return
+}
+
+
+if ![istarget "*86*-*linux*"] then {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 [list debug $additional_flags]]} {
+    return -1
+}
+
+set end_of_main          [gdb_get_line_number " end of main "]
+set end_xsave_test         [gdb_get_line_number " end xsave_test "]
+
+runto main
+
+if [supports_process_record] {
+    # Activate process record/replay
+    gdb_test_no_output "record" "turn on process record"
+}
+
+global hex
+global decimal
+
+#xsave_test
+
+gdb_test "break $end_xsave_test" \
+    "Breakpoint $decimal at .* line $end_xsave_test\." \
+    "set breakpoint at end of xsave_test"
+
+set test "continue to end of xsave_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_test .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+	untested i386-xsave-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xor.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats 4095 times>" \
+    "verify xsave buffer after reverse xsave"
-- 
2.19.0

RE: [PATCH] Add support for recording xsave x86 instruction

[Metzger, Markus T]

Hello Pierre,

> +        case 4: /* xsave */

We should also check MOD != 3.


> +          uint64_t tmpu64;
> +          if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> +            return -1;
> +          if (record_full_arch_list_add_mem (tmpu64, 512 + 64))
> +            return -1;
> +
> +          for (int i = 2; i < 64; i++) {
> +            if (!((1 << i) & tdep->xcr0))
> +              continue;
> +
> +            unsigned int size, offset, tmp1, tmp2;
> +
> +            if (!__get_cpuid_count(0xd, i, &size, &offset, &tmp1, &tmp2))
> +              return -1;

This would check the native configuration, correct?  What if we recorded
remotely on a different x86 box?

Also I think that we would need to check the inferior architecture to handle
32-bit compatibility mode.

SIZE may be zero.  We should probably check that and continue.  I'm not sure
whether we can actually run into this case but it doesn't hurt to check.

Nit: there's a space before (.


> +
> +            if (record_full_arch_list_add_mem (tmpu64 + offset, size))
> +              return -1;

Looks like this assumes the standard (non-compacted) XSAVE format.

For the compacted format, the offset must be computed by accumulating
the sizes of preceding components.


> diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
> b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c

> +void xsave_test(void) {

Nit: space before (.


> +	char buf[4096] __attribute__ ((aligned (64))) = { 0 };

The test could query the XSAVE buffer size.


> +	asm ("xor %%eax, %%eax\n\t"
> +	     "not %%eax\n\t"
> +	     "mov %%eax, %%edx\n\t"
> +	     "xsave %0":"=m"(buf) ::"eax", "edx"); } /* end xsave_test */

The } should probably go onto the next line.


> +if ![istarget "*86*-*linux*"] then {
> +    verbose "Skipping i386 reverse tests."
> +    return
> +}

Why exclude 64-bit?


> +runto main

You'd typically check whether that succeeds and abort the test if it doesn't.


> +if [supports_process_record] {
> +    # Activate process record/replay
> +    gdb_test_no_output "record" "turn on process record"
> +}

Shouldn't we abort the test if record is not supported?


> +gdb_test "reverse-step" "xor.*" "reverse-step to xsave"
> +
> +gdb_test "print buf" ".* = '\\\\000' <repeats 4095 times>" \
> +    "verify xsave buffer after reverse xsave"

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add support for recording xsave x86 instruction

[Pierre Marsais]

Hi,

Thanks for the review.

On Thu, Sep 27, 2018 at 08:44:44AM +0000, Metzger, Markus T wrote:
>> +            if (!__get_cpuid_count(0xd, i, &size, &offset, &tmp1, &tmp2))
>> +              return -1;
>
> This would check the native configuration, correct?  What if we recorded
> remotely on a different x86 box?

Oops, yes. I don't find how to query the offsets/sizes remotely, however
there is XSTATE areas sizes in gdb/common/x86-xstate.h. I think we can
assume that those values are correct.

> Also I think that we would need to check the inferior architecture to handle
> 32-bit compatibility mode.

I'm not sure to follow you. In which cases 32-bit behaves differently
than 64-bit ?

>> +
>> +            if (record_full_arch_list_add_mem (tmpu64 + offset, size))
>> +              return -1;
>
> Looks like this assumes the standard (non-compacted) XSAVE format.
>
> For the compacted format, the offset must be computed by accumulating
> the sizes of preceding components.

If I'm not mistaken, the compact format is only used by XSAVEC
instruction, which doesn't have the same opcode. The XSAVE instruction
seems unrelated to this format.

>> +if ![istarget "*86*-*linux*"] then {
>> +    verbose "Skipping i386 reverse tests."
>> +    return
>> +}
>
> Why exclude 64-bit?

Isn't this roughly the same as:
[ istarget "x86_64-*linux*" ] && [ istarget "i?86-*linux*" ]
thus excluding all arch but 32 and 64 bit x86 ?

Anyways it seems to be working on my x86_64 linux with
$ make check TESTS=gdb.reverse/i386-xsave-reverse.exp

Regards,

-- 
Pierre "Pimzero" MARSAIS,
EPITA 2018; GISTRE | ACU | LSE

[PATCH v2] Add support for recording xsave x86 instruction

[Pierre Marsais]

Latest version of glibc's ld.so use the xsave instruction in the
resolver. This breaks gdb record when calling shared libraries:

```
$ gcc -o fail -ggdb -x c - <<EOF
#include <stdlib.h>

int main() {
        exit(0);
}
EOF
$ gdb ./fail
Reading symbols from ./fail...done.
(gdb) b main
Breakpoint 1 at 0x113d: file <stdin>, line 4.
(gdb) r
Starting program: /tmp/fail

Breakpoint 1, main () at <stdin>:4
4       <stdin>: No such file or directory.
(gdb) record
(gdb) c
Continuing.
Process record does not support instruction 0xfae64 at address
0x7ffff7fe96dc.
```

In order to record xsave instructions, we record the from the beginning
of the XSAVE area to the end of the last state component in the Request
Feature Bitmap (whose value is XCR0 & EDX:EAX). At the moment we don't
account for non requested state component within the XSAVE area.

gdb/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* i386-tdep.c: (i386_process_record): Handle xsave instruction.

gdb/testsuite/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* gdb.reverse/i386-xsave-reverse.c: New file.
	* gdb.reverse/i386-xsave-reverse.exp: New file.
---
Changes in v2:
* Address Markus review
* Do not support instruction when MOD == 3
* Query XSAVE area buffer size with cpuid in tests
* Use X86_XSTATE_SIZE macro to get XSAVE area size instead of cpuid

 gdb/i386-tdep.c                               | 35 +++++++++
 .../gdb.reverse/i386-xsave-reverse.c          | 47 ++++++++++++
 .../gdb.reverse/i386-xsave-reverse.exp        | 75 +++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..9c52adbd1b 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7385,6 +7385,41 @@ no_support_3dnow_data:
             return -1;
           break;
 
+        case 4: /* xsave */
+          {
+            if (ir.mod == 3) {
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            ULONGEST rfbm_eax, rfbm_edx;
+
+            regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &rfbm_eax);
+            rfbm_eax &= (1ULL << 32) - 1;
+
+            regcache_raw_read_unsigned (regcache, I386_EDX_REGNUM, &rfbm_edx);
+            rfbm_edx &= (1ULL << 32) - 1;
+
+            uint64_t rfbm = tdep->xcr0 & (rfbm_eax | (rfbm_edx << 32));
+
+            if (rfbm & ~X86_XSTATE_ALL_MASK) {
+              printf_unfiltered (_("Process record does not support XSAVE "
+                                   "instruction with RFBM=%" PRIx64 ".\n"),
+                                   rfbm);
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            uint64_t tmpu64;
+            if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+              return -1;
+
+            if (record_full_arch_list_add_mem (tmpu64,
+                                               X86_XSTATE_SIZE (rfbm)))
+              return -1;
+          }
+          break;
+
         case 5:    /* lfence */
         case 6:    /* mfence */
         case 7:    /* sfence clflush */
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
new file mode 100644
index 0000000000..5e90d1ad9c
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+#include <cpuid.h>
+#include <stdint.h>
+#include <string.h>
+
+uint32_t get_xsave_buffer_size(void) {
+  uint32_t size, eax, ecx, edx;
+  __get_cpuid_count(0xd, 0, &eax, &size, &ecx, &edx);
+
+  return size;
+}
+
+void xsave_test(void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset(buf, 0, xsave_buf_sze);
+
+  asm ("xor %%eax, %%eax\n\t"
+       "not %%eax\n\t"
+       "mov %%eax, %%edx\n\t"
+       "xsave %0":"=m"(buf) ::"eax", "edx");
+} /* end xsave_test */
+
+int
+main ()
+{
+  xsave_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
new file mode 100644
index 0000000000..f47f507101
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
@@ -0,0 +1,75 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+if {![supports_reverse] || ![supports_process_record]} {
+    return
+}
+
+if ![istarget "*86*-*linux*"] then {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+         [list debug $additional_flags]]} {
+    return -1
+}
+
+set end_of_main          [gdb_get_line_number " end of main "]
+set end_xsave_test         [gdb_get_line_number " end xsave_test "]
+
+if ![runto main] then {
+    fail "run to main"
+    return
+}
+
+# Activate process record/replay
+gdb_test_no_output "record" "turn on process record"
+
+global hex
+global decimal
+
+#xsave_test
+
+gdb_test "break $end_xsave_test" \
+    "Breakpoint $decimal at .* line $end_xsave_test\." \
+    "set breakpoint at end of xsave_test"
+
+set test "continue to end of xsave_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsave-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xor.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsave"
-- 
2.19.0

RE: [PATCH] Add support for recording xsave x86 instruction

[Metzger, Markus T]

Hello Pierre,

> On Thu, Sep 27, 2018 at 08:44:44AM +0000, Metzger, Markus T wrote:
> >> +            if (!__get_cpuid_count(0xd, i, &size, &offset, &tmp1, &tmp2))
> >> +              return -1;
> >
> > This would check the native configuration, correct?  What if we
> > recorded remotely on a different x86 box?
> 
> Oops, yes. I don't find how to query the offsets/sizes remotely, however there is
> XSTATE areas sizes in gdb/common/x86-xstate.h. I think we can assume that
> those values are correct.

OK.  Other parts of GDB are using those, as well, rather than querying cpuid.


> > Also I think that we would need to check the inferior architecture to
> > handle 32-bit compatibility mode.
> 
> I'm not sure to follow you. In which cases 32-bit behaves differently than 64-bit ?

Fewer registers.  XSAVE is not writing the upper registers area.


> >> +            if (record_full_arch_list_add_mem (tmpu64 + offset, size))
> >> +              return -1;
> >
> > Looks like this assumes the standard (non-compacted) XSAVE format.
> >
> > For the compacted format, the offset must be computed by accumulating
> > the sizes of preceding components.
> 
> If I'm not mistaken, the compact format is only used by XSAVEC instruction, which
> doesn't have the same opcode. The XSAVE instruction seems unrelated to this
> format.

You're right.  It doesn't write the full header ,though.  And there's a special case
with XCR0[1].


> >> +if ![istarget "*86*-*linux*"] then {
> >> +    verbose "Skipping i386 reverse tests."
> >> +    return
> >> +}
> >
> > Why exclude 64-bit?
> 
> Isn't this roughly the same as:
> [ istarget "x86_64-*linux*" ] && [ istarget "i?86-*linux*" ] thus excluding all arch
> but 32 and 64 bit x86 ?

I mistook it for i?86.

Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH v3] Add support for recording xsave x86 instruction

[Pierre Marsais]

Latest version of glibc's ld.so use the xsave instruction in the
resolver. This breaks gdb record when calling shared libraries:

```
$ gcc -o fail -ggdb -x c - <<EOF
#include <stdlib.h>

int main() {
        exit(0);
}
EOF
$ gdb ./fail
Reading symbols from ./fail...done.
(gdb) b main
Breakpoint 1 at 0x113d: file <stdin>, line 4.
(gdb) r
Starting program: /tmp/fail

Breakpoint 1, main () at <stdin>:4
4       <stdin>: No such file or directory.
(gdb) record
(gdb) c
Continuing.
Process record does not support instruction 0xfae64 at address
0x7ffff7fe96dc.
```

In order to record xsave instructions, we record the from the beginning
of the XSAVE area to the end of the last state component in the Request
Feature Bitmap (whose value is XCR0 & EDX:EAX). At the moment we don't
account for non requested state component within the XSAVE area.

gdb/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* i386-tdep.c: (i386_process_record): Handle xsave instruction.

gdb/testsuite/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* gdb.reverse/i386-xsave-reverse.c: New file.
	* gdb.reverse/i386-xsave-reverse.exp: New file.
---
Changes in v3:
* Only save XSAVE SSE area when requested
* Do not save XMM8-XMM15 registers in XSAVE area for 32-bit mode
* Account for the hole between the legacy region and the XSAVE header
* Add a test without SSE in the Request Feature Bitmap
* Improve style of tests

Changes in v2:
* Address Markus review
* Do not support instruction when MOD == 3
* Query XSAVE area buffer size with cpuid in tests
* Use X86_XSTATE_SIZE macro to get XSAVE area size instead of cpuid

 gdb/i386-tdep.c                               |  45 ++++++++
 .../gdb.reverse/i386-xsave-reverse.c          |  53 +++++++++
 .../gdb.reverse/i386-xsave-reverse.exp        | 102 ++++++++++++++++++
 3 files changed, 200 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..a9fe290307 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7385,6 +7385,51 @@ no_support_3dnow_data:
             return -1;
           break;
 
+        case 4: /* xsave */
+          {
+            if (ir.mod == 3) {
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            ULONGEST rfbm_eax, rfbm_edx;
+
+            regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &rfbm_eax);
+            rfbm_eax &= (1ULL << 32) - 1;
+
+            regcache_raw_read_unsigned (regcache, I386_EDX_REGNUM, &rfbm_edx);
+            rfbm_edx &= (1ULL << 32) - 1;
+
+            uint64_t rfbm = tdep->xcr0 & (rfbm_eax | (rfbm_edx << 32));
+
+            if (rfbm & ~X86_XSTATE_ALL_MASK) {
+              printf_unfiltered (_("Process record does not support XSAVE "
+                                   "instruction with RFBM=%" PRIx64 ".\n"),
+                                   rfbm);
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            uint64_t tmpu64;
+            if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+              return -1;
+
+            uint64_t legacy_size = 160; /* x87 xsave size */
+            if (rfbm & X86_XSTATE_SSE) {
+              if (ir.regmap[X86_RECORD_R8_REGNUM])
+                legacy_size = 416; /* 64-bit sse xsave size */
+              else
+                legacy_size = 288; /* 32-bit sse xsave size */
+            }
+            if (record_full_arch_list_add_mem (tmpu64, legacy_size))
+              return -1;
+
+            uint64_t size = X86_XSTATE_SIZE (rfbm) - 512;
+            if (record_full_arch_list_add_mem (tmpu64 + 512, size))
+              return -1;
+          }
+          break;
+
         case 5:    /* lfence */
         case 6:    /* mfence */
         case 7:    /* sfence clflush */
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
new file mode 100644
index 0000000000..18453405a6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+#include <cpuid.h>
+#include <stdint.h>
+#include <string.h>
+
+uint32_t get_xsave_buffer_size (void) {
+  uint32_t size, eax, ecx, edx;
+  __get_cpuid_count (0xd, 0, &eax, &size, &ecx, &edx);
+
+  return size;
+}
+
+void xsave_test (void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size ();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset (buf, 0, xsave_buf_sze);
+
+  asm ("xsave %0":"=m"(buf) : "a"(~0L), "d"(~0L));
+} /* end xsave_test */
+
+void xsave_no_sse_test (void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size ();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset (buf, 0, xsave_buf_sze);
+
+  asm ("xsave %0":"=m"(buf): "a"((1 << 1) ^ ~0L), "d"(~0L));
+} /* end xsave_no_sse_test */
+
+int
+main ()
+{
+  xsave_test ();
+  xsave_no_sse_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
new file mode 100644
index 0000000000..f3c0f93bfc
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
@@ -0,0 +1,102 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+if {![supports_reverse] || ![supports_process_record]} {
+    return
+}
+
+if ![istarget "*86*-*linux*"] then {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+         [list debug $additional_flags]]} {
+    return -1
+}
+
+set end_of_main          [gdb_get_line_number " end of main "]
+set end_xsave_test         [gdb_get_line_number " end xsave_test "]
+set end_xsave_no_sse_test         [gdb_get_line_number " end xsave_no_sse_test "]
+
+if ![runto main] then {
+    fail "run to main"
+    return
+}
+
+# Activate process record/replay
+gdb_test_no_output "record" "turn on process record"
+
+global hex
+global decimal
+
+#xsave_test
+
+gdb_test "break $end_xsave_test" \
+    "Breakpoint $decimal at .* line $end_xsave_test\." \
+    "set breakpoint at end of xsave_test"
+
+set test "continue to end of xsave_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsave-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xsave.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsave"
+
+#xsave_no_sse_test
+
+gdb_test "continue" \
+    " end xsave_test .*" \
+    "continue to end of xsave_test"
+
+gdb_test "break $end_xsave_no_sse_test" \
+    "Breakpoint $decimal at .* line $end_xsave_no_sse_test\." \
+    "set breakpoint at end of xsave_no_sse_test"
+
+set test "continue to end of xsave_no_sse_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_no_sse_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsave-no-sse-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xsave.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsave (no sse)"
-- 
2.19.0

Re: [PATCH] Add support for recording xsave x86 instruction

[Pierre Marsais]

Hi,

Thanks for the quick reply.

On Mon, Oct 01, 2018 at 06:58:32AM +0000, Metzger, Markus T wrote:
> > > Also I think that we would need to check the inferior architecture to
> > > handle 32-bit compatibility mode.
> > 
> > I'm not sure to follow you. In which cases 32-bit behaves differently than 64-bit ?
> 
> Fewer registers.  XSAVE is not writing the upper registers area.

> > >> +            if (record_full_arch_list_add_mem (tmpu64 + offset, size))
> > >> +              return -1;
> > >
> > > Looks like this assumes the standard (non-compacted) XSAVE format.
> > >
> > > For the compacted format, the offset must be computed by accumulating
> > > the sizes of preceding components.
> > 
> > If I'm not mistaken, the compact format is only used by XSAVEC instruction, which
> > doesn't have the same opcode. The XSAVE instruction seems unrelated to this
> > format.
> 
> You're right.  It doesn't write the full header ,though.  And there's a special case
> with XCR0[1].

Once again, thank you for finding this. I think I've addressed all your
concerns in the v3 of the patch.

Regards,

-- 
Pierre "Pimzero" MARSAIS,
EPITA 2018; GISTRE | ACU | LSE

Re: [PATCH v3] Add support for recording xsave x86 instruction

[H.J. Lu]

On Tue, Oct 2, 2018 at 4:56 PM Pierre Marsais
<pierre.marsais@lse.epita.fr> wrote:
>
> Latest version of glibc's ld.so use the xsave instruction in the
> resolver. This breaks gdb record when calling shared libraries:
>
> ```
> $ gcc -o fail -ggdb -x c - <<EOF
> #include <stdlib.h>
>
> int main() {
>         exit(0);
> }
> EOF
> $ gdb ./fail
> Reading symbols from ./fail...done.
> (gdb) b main
> Breakpoint 1 at 0x113d: file <stdin>, line 4.
> (gdb) r
> Starting program: /tmp/fail
>
> Breakpoint 1, main () at <stdin>:4
> 4       <stdin>: No such file or directory.
> (gdb) record
> (gdb) c
> Continuing.
> Process record does not support instruction 0xfae64 at address
> 0x7ffff7fe96dc.
> ```
>
> In order to record xsave instructions, we record the from the beginning
> of the XSAVE area to the end of the last state component in the Request
> Feature Bitmap (whose value is XCR0 & EDX:EAX). At the moment we don't
> account for non requested state component within the XSAVE area.
>
> gdb/ChangeLog:
> 2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>
>
>         * i386-tdep.c: (i386_process_record): Handle xsave instruction.
>

Don't you also need to handle xsavec since  ld.so uses if it is usable?

H.J.

[PATCH v4 2/3] Do not mistreat instructions as cmpxchg8b

[Pierre Marsais]

All x86 instructions starting with opcode 0x0f7c where considered as
cmpxchg8b if ir.mod == 3, regardless of ir.reg. However, there are some
instructions (such as xsavec) sharing the same opcode, but with
different ir.reg.

This change throws an error when recording on unsupported instructions
instead of considering them as cmpxchg8b.

gdb/ChangeLog:

2018-10-05  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* i386-tdep.c: (i386_process_record): Improve decoding of
	instructions starting with 0x0f7c.
---
 gdb/i386-tdep.c | 76 ++++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a9fe290307..90c78e0bbc 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5477,39 +5477,49 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
     case 0x0fc7:    /* cmpxchg8b / rdrand / rdseed */
       if (i386_record_modrm (&ir))
 	return -1;
-      if (ir.mod == 3)
-	{
-	  /* rdrand and rdseed use the 3 bits of the REG field of ModR/M as
-	     an extended opcode.  rdrand has bits 110 (/6) and rdseed
-	     has bits 111 (/7).  */
-	  if (ir.reg == 6 || ir.reg == 7)
-	    {
-	      /* The storage register is described by the 3 R/M bits, but the
-		 REX.B prefix may be used to give access to registers
-		 R8~R15.  In this case ir.rex_b + R/M will give us the register
-		 in the range R8~R15.
-
-		 REX.W may also be used to access 64-bit registers, but we
-		 already record entire registers and not just partial bits
-		 of them.  */
-	      I386_RECORD_FULL_ARCH_LIST_ADD_REG (ir.rex_b + ir.rm);
-	      /* These instructions also set conditional bits.  */
-	      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
-	      break;
-	    }
-	  else
-	    {
-	      /* We don't handle this particular instruction yet.  */
-	      ir.addr -= 2;
-	      opcode = opcode << 8 | ir.modrm;
-	      goto no_support;
-	    }
-	}
-      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
-      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
-      if (i386_record_lea_modrm (&ir))
-	return -1;
-      I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+      switch (ir.reg) {
+        case 1: /* cmpxchg8b */
+          if (ir.mod == 3)
+            {
+              ir.addr -= 2;
+              opcode = opcode << 8 | ir.modrm;
+              goto no_support;
+            }
+          I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+          I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+          if (i386_record_lea_modrm (&ir))
+            return -1;
+          I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+          break;
+        case 6: /* rdrand */
+        case 7: /* rdseed */
+          if (ir.mod != 3)
+            {
+              /* We don't handle this particular instruction yet.  */
+              ir.addr -= 2;
+              opcode = opcode << 8 | ir.modrm;
+              goto no_support;
+            }
+          /* rdrand and rdseed use the 3 bits of the REG field of ModR/M as
+             an extended opcode.  rdrand has bits 110 (/6) and rdseed
+             has bits 111 (/7).  */
+          /* The storage register is described by the 3 R/M bits, but the
+             REX.B prefix may be used to give access to registers
+             R8~R15.  In this case ir.rex_b + R/M will give us the register
+             in the range R8~R15.
+
+             REX.W may also be used to access 64-bit registers, but we
+             already record entire registers and not just partial bits
+             of them.  */
+          I386_RECORD_FULL_ARCH_LIST_ADD_REG (ir.rex_b + ir.rm);
+          /* These instructions also set conditional bits.  */
+          I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+          break;
+        default:
+          ir.addr -= 2;
+          opcode = opcode << 8 | ir.modrm;
+          goto no_support;
+      }
       break;
 
     case 0x50:    /* push */
-- 
2.19.0

[PATCH v4 3/3] Add support for recording xsavec x86 instruction

[Pierre Marsais]

This patch add support of the xsavec instruction. The logic is highly
similar to the logic of xsave. The "init optimization" is not handled in
this patch.

The values of X86_XSAVEC_* were obtained by testing for AVX and MPX, by
reading Intel x86 manuals, and looking at
arch/x86/include/asm/fpu/types.h in Linux source.

gdb/ChangeLog:
2018-10-05  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* common/x86-xstate.h (X86_XSAVEC_AVX_SIZE,
	X86_XSAVEC_BNDREGS_SIZE, X86_XSAVEC_BNDCFG_SIZE,
	X86_XSAVEC_K_SIZE, X86_XSAVEC_ZMM_H_SIZE,  X86_XSAVEC_ZMM_SIZE,
	X86_XSAVEC_PKRU_SIZE, X86_XSAVEC_SIZE(XCR0)): New macros.
	* i386-tdep.c: (i386_process_record): Handle xsavec instruction.

gdb/testsuite/ChangeLog:
2018-10-05  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* gdb.reverse/i386-xsave-reverse.c (xsavec_test): New function.
	(main): Use xsavec_test.
	* gdb.reverse/i386-xsave-reverse.exp: Test for xsavec
	instruction.
---
 gdb/common/x86-xstate.h                       | 17 +++++++
 gdb/i386-tdep.c                               | 44 +++++++++++++++++++
 .../gdb.reverse/i386-xsave-reverse.c          |  9 ++++
 .../gdb.reverse/i386-xsave-reverse.exp        | 33 ++++++++++++--
 4 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/gdb/common/x86-xstate.h b/gdb/common/x86-xstate.h
index 51e5c3c785..8a5b167610 100644
--- a/gdb/common/x86-xstate.h
+++ b/gdb/common/x86-xstate.h
@@ -72,6 +72,23 @@
       (HAS_MPX (XCR0) ? X86_XSTATE_BNDCFG_SIZE : \
        (HAS_AVX (XCR0) ? X86_XSTATE_AVX_SIZE : X86_XSTATE_SSE_SIZE))))
 
+#define X86_XSAVEC_AVX_SIZE	256
+#define X86_XSAVEC_BNDREGS_SIZE	64
+#define X86_XSAVEC_BNDCFG_SIZE	64
+#define X86_XSAVEC_K_SIZE	64
+#define X86_XSAVEC_ZMM_H_SIZE	512
+#define X86_XSAVEC_ZMM_SIZE	1024
+#define X86_XSAVEC_PKRU_SIZE	8
+
+#define X86_XSAVEC_SIZE(XCR0) \
+    (((XCR0) & X86_XSTATE_AVX ? X86_XSAVEC_AVX_SIZE : 0 ) + \
+     ((XCR0) & X86_XSTATE_BNDREGS ? X86_XSAVEC_BNDREGS_SIZE : 0) + \
+     ((XCR0) & X86_XSTATE_BNDCFG ? X86_XSAVEC_BNDCFG_SIZE : 0) + \
+     ((XCR0) & X86_XSTATE_K ? X86_XSAVEC_K_SIZE : 0) + \
+     ((XCR0) & X86_XSTATE_ZMM_H ? X86_XSAVEC_ZMM_H_SIZE : 0) + \
+     ((XCR0) & X86_XSTATE_ZMM ? X86_XSAVEC_ZMM_SIZE : 0) + \
+     ((XCR0) & X86_XSTATE_PKRU ? X86_XSAVEC_PKRU_SIZE : 0))
+
 /* Initial value for fctrl register, as defined in the X86 manual, and
    confirmed in the (Linux) kernel source.  When the x87 floating point
    feature is not enabled in an inferior we use this as the value of the
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 90c78e0bbc..c97a4978aa 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5491,6 +5491,50 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
             return -1;
           I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
           break;
+        case 4: /* xsavec */
+          {
+            if (ir.mod == 3) {
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            ULONGEST rfbm_eax, rfbm_edx;
+
+            regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &rfbm_eax);
+            rfbm_eax &= (1ULL << 32) - 1;
+
+            regcache_raw_read_unsigned (regcache, I386_EDX_REGNUM, &rfbm_edx);
+            rfbm_edx &= (1ULL << 32) - 1;
+
+            uint64_t rfbm = tdep->xcr0 & (rfbm_eax | (rfbm_edx << 32));
+
+            if (rfbm & ~X86_XSTATE_ALL_MASK) {
+              printf_unfiltered (_("Process record does not support XSAVEC "
+                                   "instruction with RFBM=%" PRIx64 ".\n"),
+                                   rfbm);
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            uint64_t tmpu64;
+            if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+              return -1;
+
+            uint64_t legacy_size = 160; /* x87 xsave size */
+            if (rfbm & X86_XSTATE_SSE) {
+              if (ir.regmap[X86_RECORD_R8_REGNUM])
+                legacy_size = 416; /* 64-bit sse xsave size */
+              else
+                legacy_size = 288; /* 32-bit sse xsave size */
+            }
+            if (record_full_arch_list_add_mem (tmpu64, legacy_size))
+              return -1;
+
+            uint64_t size = X86_XSAVEC_SIZE (rfbm);
+            if (record_full_arch_list_add_mem (tmpu64 + 512, size))
+              return -1;
+          }
+          break;
         case 6: /* rdrand */
         case 7: /* rdseed */
           if (ir.mod != 3)
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
index 18453405a6..965ca60133 100644
--- a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
@@ -44,10 +44,19 @@ void xsave_no_sse_test (void) {
   asm ("xsave %0":"=m"(buf): "a"((1 << 1) ^ ~0L), "d"(~0L));
 } /* end xsave_no_sse_test */
 
+void xsavec_test (void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size ();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset (buf, 0, xsave_buf_sze);
+
+  asm ("xsavec %0":"=m"(buf) : "a"(~0L), "d"(~0L));
+} /* end xsavec_test */
+
 int
 main ()
 {
   xsave_test ();
   xsave_no_sse_test ();
+  xsavec_test ();
   return 0;	/* end of main */
 }
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
index f3c0f93bfc..d8fa64cdae 100644
--- a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
@@ -38,9 +38,10 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
     return -1
 }
 
-set end_of_main          [gdb_get_line_number " end of main "]
-set end_xsave_test         [gdb_get_line_number " end xsave_test "]
-set end_xsave_no_sse_test         [gdb_get_line_number " end xsave_no_sse_test "]
+set end_of_main             [gdb_get_line_number " end of main "]
+set end_xsave_test          [gdb_get_line_number " end xsave_test "]
+set end_xsave_no_sse_test   [gdb_get_line_number " end xsave_no_sse_test "]
+set end_xsavec_test         [gdb_get_line_number " end xsavec_test "]
 
 if ![runto main] then {
     fail "run to main"
@@ -100,3 +101,29 @@ gdb_test "reverse-step" "xsave.*" "reverse-step to xsave"
 
 gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
     "verify xsave buffer after reverse xsave (no sse)"
+
+#xsavec_test
+
+gdb_test "continue" \
+    " end xsave_no_sse_test .*" \
+    "continue to end of xsave_no_sse_test"
+
+gdb_test "break $end_xsavec_test" \
+    "Breakpoint $decimal at .* line $end_xsavec_test\." \
+    "set breakpoint at end of xsavec_test"
+
+set test "continue to end of xsavec_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsavec_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsavec-no-sse-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xsavec.*" "reverse-step to xsavec"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsavec"
-- 
2.19.0

[PATCH v4 1/3] Add support for recording xsave x86 instruction

[Pierre Marsais]

Latest version of glibc's ld.so use the xsave instruction in the
resolver. This breaks gdb record when calling shared libraries:

```
$ gcc -o fail -ggdb -x c - <<EOF
#include <stdlib.h>

int main() {
        exit(0);
}
EOF
$ gdb ./fail
Reading symbols from ./fail...done.
(gdb) b main
Breakpoint 1 at 0x113d: file <stdin>, line 4.
(gdb) r
Starting program: /tmp/fail

Breakpoint 1, main () at <stdin>:4
4       <stdin>: No such file or directory.
(gdb) record
(gdb) c
Continuing.
Process record does not support instruction 0xfae64 at address
0x7ffff7fe96dc.
```

In order to record xsave instructions, we record the from the beginning
of the XSAVE area to the end of the last state component in the Request
Feature Bitmap (whose value is XCR0 & EDX:EAX). At the moment we don't
account for non requested state component within the XSAVE area.

gdb/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* i386-tdep.c: (i386_process_record): Handle xsave instruction.

gdb/testsuite/ChangeLog:
2018-09-21  Pierre Marsais <pierre.marsais@lse.epita.fr>

	* gdb.reverse/i386-xsave-reverse.c: New file.
	* gdb.reverse/i386-xsave-reverse.exp: New file.
---
Changes in v4:
* Add support of recording xsavec instruction
* Fix decoding of 0x0fc7 instructions/rewrite it as a switch
* Patch 1 is still the same as v3

Changes in v3:
* Only save XSAVE SSE area when requested
* Do not save XMM8-XMM15 registers in XSAVE area for 32-bit mode
* Account for the hole between the legacy region and the XSAVE header
* Add a test without SSE in the Request Feature Bitmap
* Improve style of tests

Changes in v2:
* Address Markus review
* Do not support instruction when MOD == 3
* Query XSAVE area buffer size with cpuid in tests
* Use X86_XSTATE_SIZE macro to get XSAVE area size instead of cpuid

 gdb/i386-tdep.c                               |  45 ++++++++
 .../gdb.reverse/i386-xsave-reverse.c          |  53 +++++++++
 .../gdb.reverse/i386-xsave-reverse.exp        | 102 ++++++++++++++++++
 3 files changed, 200 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..a9fe290307 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7385,6 +7385,51 @@ no_support_3dnow_data:
             return -1;
           break;
 
+        case 4: /* xsave */
+          {
+            if (ir.mod == 3) {
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            ULONGEST rfbm_eax, rfbm_edx;
+
+            regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &rfbm_eax);
+            rfbm_eax &= (1ULL << 32) - 1;
+
+            regcache_raw_read_unsigned (regcache, I386_EDX_REGNUM, &rfbm_edx);
+            rfbm_edx &= (1ULL << 32) - 1;
+
+            uint64_t rfbm = tdep->xcr0 & (rfbm_eax | (rfbm_edx << 32));
+
+            if (rfbm & ~X86_XSTATE_ALL_MASK) {
+              printf_unfiltered (_("Process record does not support XSAVE "
+                                   "instruction with RFBM=%" PRIx64 ".\n"),
+                                   rfbm);
+              opcode = (opcode << 8) | ir.modrm;
+              goto no_support;
+            }
+
+            uint64_t tmpu64;
+            if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+              return -1;
+
+            uint64_t legacy_size = 160; /* x87 xsave size */
+            if (rfbm & X86_XSTATE_SSE) {
+              if (ir.regmap[X86_RECORD_R8_REGNUM])
+                legacy_size = 416; /* 64-bit sse xsave size */
+              else
+                legacy_size = 288; /* 32-bit sse xsave size */
+            }
+            if (record_full_arch_list_add_mem (tmpu64, legacy_size))
+              return -1;
+
+            uint64_t size = X86_XSTATE_SIZE (rfbm) - 512;
+            if (record_full_arch_list_add_mem (tmpu64 + 512, size))
+              return -1;
+          }
+          break;
+
         case 5:    /* lfence */
         case 6:    /* mfence */
         case 7:    /* sfence clflush */
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
new file mode 100644
index 0000000000..18453405a6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.c
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Architecture tests for intel i386 platform.  */
+
+#include <cpuid.h>
+#include <stdint.h>
+#include <string.h>
+
+uint32_t get_xsave_buffer_size (void) {
+  uint32_t size, eax, ecx, edx;
+  __get_cpuid_count (0xd, 0, &eax, &size, &ecx, &edx);
+
+  return size;
+}
+
+void xsave_test (void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size ();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset (buf, 0, xsave_buf_sze);
+
+  asm ("xsave %0":"=m"(buf) : "a"(~0L), "d"(~0L));
+} /* end xsave_test */
+
+void xsave_no_sse_test (void) {
+  uint32_t xsave_buf_sze = get_xsave_buffer_size ();
+  char buf[xsave_buf_sze] __attribute__ ((aligned (64)));
+  memset (buf, 0, xsave_buf_sze);
+
+  asm ("xsave %0":"=m"(buf): "a"((1 << 1) ^ ~0L), "d"(~0L));
+} /* end xsave_no_sse_test */
+
+int
+main ()
+{
+  xsave_test ();
+  xsave_no_sse_test ();
+  return 0;	/* end of main */
+}
diff --git a/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
new file mode 100644
index 0000000000..f3c0f93bfc
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/i386-xsave-reverse.exp
@@ -0,0 +1,102 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests some i386 general instructions for reverse execution.
+#
+
+if {![supports_reverse] || ![supports_process_record]} {
+    return
+}
+
+if ![istarget "*86*-*linux*"] then {
+    verbose "Skipping i386 reverse tests."
+    return
+}
+
+standard_testfile
+
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+         [list debug $additional_flags]]} {
+    return -1
+}
+
+set end_of_main          [gdb_get_line_number " end of main "]
+set end_xsave_test         [gdb_get_line_number " end xsave_test "]
+set end_xsave_no_sse_test         [gdb_get_line_number " end xsave_no_sse_test "]
+
+if ![runto main] then {
+    fail "run to main"
+    return
+}
+
+# Activate process record/replay
+gdb_test_no_output "record" "turn on process record"
+
+global hex
+global decimal
+
+#xsave_test
+
+gdb_test "break $end_xsave_test" \
+    "Breakpoint $decimal at .* line $end_xsave_test\." \
+    "set breakpoint at end of xsave_test"
+
+set test "continue to end of xsave_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsave-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xsave.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsave"
+
+#xsave_no_sse_test
+
+gdb_test "continue" \
+    " end xsave_test .*" \
+    "continue to end of xsave_test"
+
+gdb_test "break $end_xsave_no_sse_test" \
+    "Breakpoint $decimal at .* line $end_xsave_no_sse_test\." \
+    "set breakpoint at end of xsave_no_sse_test"
+
+set test "continue to end of xsave_no_sse_test"
+gdb_test_multiple "continue" $test {
+    -re " end xsave_no_sse_test .*\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re " Illegal instruction.*\r\n$gdb_prompt $" {
+        untested i386-xsave-no-sse-reverse
+        return -1
+    }
+}
+
+gdb_test "reverse-step" "xsave.*" "reverse-step to xsave"
+
+gdb_test "print buf" ".* = '\\\\000' <repeats .* times>" \
+    "verify xsave buffer after reverse xsave (no sse)"
-- 
2.19.0

Re: [PATCH v3] Add support for recording xsave x86 instruction

[Pierre Marsais]

Hi,

On Tue, Oct 02, 2018 at 06:10:54PM -0700, H.J. Lu wrote:
> Don't you also need to handle xsavec since  ld.so uses if it is usable?

Thanks for your comment. I've sent two more patches to support xsavec in
v4.

Best regards,

-- 
Pierre "Pimzero" MARSAIS,
EPITA 2018; GISTRE | ACU | LSE

RE: [PATCH v4 1/3] Add support for recording xsave x86 instruction

[Metzger, Markus T]

Hello Pierre,

> +            if (rfbm & ~X86_XSTATE_ALL_MASK) {
> +              printf_unfiltered (_("Process record does not support XSAVE "
> +                                   "instruction with RFBM=%" PRIx64 ".\n"),
> +                                   rfbm);

The indentation looks a bit off.


> +              opcode = (opcode << 8) | ir.modrm;
> +              goto no_support;
> +            }
> +
> +            uint64_t tmpu64;
> +            if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> +              return -1;
> +
> +            uint64_t legacy_size = 160; /* x87 xsave size */
> +            if (rfbm & X86_XSTATE_SSE) {
> +              if (ir.regmap[X86_RECORD_R8_REGNUM])
> +                legacy_size = 416; /* 64-bit sse xsave size */
> +              else
> +                legacy_size = 288; /* 32-bit sse xsave size */
> +            }
> +            if (record_full_arch_list_add_mem (tmpu64, legacy_size))
> +              return -1;
> +
> +            uint64_t size = X86_XSTATE_SIZE (rfbm) - 512;
> +            if (record_full_arch_list_add_mem (tmpu64 + 512, size))
> +              return -1;

This stores the full xsave area header.  XSAVE only writes the first 8 bytes.

Also it turns out the macros are not correct, either.  Jan Beulich sent a
patch to fix XSAVE handling:
https://sourceware.org/ml/gdb-patches/2018-10/msg00261.html

We'd need to do a similar analysis here.


> +uint32_t get_xsave_buffer_size (void) {

Please use the GNU coding style also in tests.


> +# This test tests some i386 general instructions for reverse execution.
> +#
> +
> +if {![supports_reverse] || ![supports_process_record]} {
> +    return
> +}

You'd typically log them as unsupported.  Not sure if you need to check
supports_reverse, as well.



> +gdb_test "break $end_xsave_test" \
> +    "Breakpoint $decimal at .* line $end_xsave_test\." \
> +    "set breakpoint at end of xsave_test"
> +
> +set test "continue to end of xsave_test"
> +gdb_test_multiple "continue" $test {
> +    -re " end xsave_test .*\r\n$gdb_prompt $" {
> +        pass $test
> +    }
> +    -re " Illegal instruction.*\r\n$gdb_prompt $" {
> +        untested i386-xsave-reverse
> +        return -1
> +    }
> +}

Wouldn't it be easier to check for XSAVE support separately?


The tests don't really cover that record is not saving too much.
I think both tests would pass if we recorded the entire page.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v4 2/3] Do not mistreat instructions as cmpxchg8b

[Metzger, Markus T]

Hello Pierre,

> +      switch (ir.reg) {
> +        case 1: /* cmpxchg8b */
> +          if (ir.mod == 3)
> +            {
> +              ir.addr -= 2;
> +              opcode = opcode << 8 | ir.modrm;
> +              goto no_support;
> +            }

We'd also need to check that neither 66 nor f3 prefixes are used.


> +        case 6: /* rdrand */
> +        case 7: /* rdseed */
> +          if (ir.mod != 3)
> +            {
> +              /* We don't handle this particular instruction yet.  */
> +              ir.addr -= 2;
> +              opcode = opcode << 8 | ir.modrm;
> +              goto no_support;
> +            }

We'd also need to check that f3 prefix is not used.


Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928