Add support for recording xsave x86 instruction

  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
  

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
  

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,
  

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
  

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,