This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] x86-64: fix displaced stepping for VEX, XOP, and EVEX encoded insns
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Beulich <JBeulich at suse dot com>, GDB <gdb-patches at sourceware dot org>
- Date: Thu, 7 Feb 2019 16:09:07 +0000
- Subject: Re: [PATCH] x86-64: fix displaced stepping for VEX, XOP, and EVEX encoded insns
- References: <5C5C3C360200007800214AC3@prv1-mh.provo.novell.com>
On 02/07/2019 02:09 PM, Jan Beulich wrote:
> Improper decoding has lead to wrong use of onebyte_has_modrm[] in all
> those cases, leading to misbehavior in the debuggee (wrong data accessed
> or faults raised) when RIP-relative operands were used. Fix VEX handling
> and add XOP and EVEX recognition.
>
Thanks!
> I apologize for this not being accompanied by a testsuite extension, but
> it was hard enough already to find a couple of hours to investigate and
> fix the actual issue here.
I'd be really good to add a testcase though.
<https://sourceware.org/ml/gdb-patches/2017-11/msg00748.html>
added gdb.arch/amd64-disp-step-avx.S. Can we add something there?
Can you provide examples of instructions that we were getting wrong?
Note that re. EVEX, back in that referenced patch I said:
~~~
I think we may need a similar treatment for EVEX-encoded instructions,
but I didn't address that simply because I couldn't find any
EVEX-encoded RIP-relative instruction in the gas testsuite.
~~~
since you seem to know about some EVEX-encoded instruction that is
mishandled today, it'd be really good to add a test for it too.
Thanks,
Pedro Alves
>
> (For XOP there may be another problem with the immediates two of the
> forms use. I didn't look into that aspect yet.)
>
> gdb/
> 2019-02-07 Jan Beulich <jbeulich@suse.com>
>
> * amd64-tdep.c (xop_prefix_p, evex_prefix_p): New.
> (fixup_riprel): Use them.
> (amd64_get_insn_details): Likewise. Handle VEX/XOP/EVEX cases
> independent of legacy encoding ones.
>
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1147,6 +1147,22 @@ vex3_prefix_p (gdb_byte pfx)
> return pfx == 0xc4;
> }
>
> +/* True if PFX is the start of an XOP prefix. */
> +
> +static bool
> +xop_prefix_p (const gdb_byte *pfx)
> +{
> + return pfx[0] == 0x8f && (pfx[1] & 0x38);
> +}
> +
> +/* True if PFX is the start of an EVEX prefix. */
> +
> +static bool
> +evex_prefix_p (const gdb_byte *pfx)
> +{
> + return pfx[0] == 0x62 && !(pfx[1] & 0x0c) && (pfx[2] & 4);
> +}
> +
> /* Skip the legacy instruction prefixes in INSN.
> We assume INSN is properly sentineled so we don't have to worry
> about falling off the end of the buffer. */
> @@ -1260,11 +1276,11 @@ static void
> amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
> {
> gdb_byte *start = insn;
> - int need_modrm;
> + int need_modrm = 0;
>
> details->raw_insn = insn;
>
> - details->opcode_len = -1;
> + details->opcode_len = 1;
> details->enc_prefix_offset = -1;
> details->opcode_offset = -1;
> details->modrm_offset = -1;
> @@ -1283,16 +1299,35 @@ amd64_get_insn_details (gdb_byte *insn,
> /* Don't record the offset in this case because this prefix has
> no REX.B equivalent. */
> insn += 2;
> + need_modrm = -1;
> }
> else if (vex3_prefix_p (*insn))
> {
> details->enc_prefix_offset = insn - start;
> + need_modrm = (insn[1] & 0x1f) == 1 ? -1 : 1;
> + insn += 3;
> + }
> + else if (xop_prefix_p (insn))
> + {
> + details->enc_prefix_offset = insn - start;
> insn += 3;
> + need_modrm = 1;
> + }
> + else if (evex_prefix_p (insn))
> + {
> + details->enc_prefix_offset = insn - start;
> + need_modrm = (insn[1] & 3) == 1 ? -1 : 1;
> + insn += 4;
> }
>
> details->opcode_offset = insn - start;
>
> - if (*insn == TWO_BYTE_OPCODE_ESCAPE)
> + if (need_modrm < 0)
> + {
> + /* VEX/EVEX-encoded would-be-two-byte opcode. */
> + need_modrm = twobyte_has_modrm[*insn];
> + }
> + else if (!need_modrm && *insn == TWO_BYTE_OPCODE_ESCAPE)
> {
> /* Two or three-byte opcode. */
> ++insn;
> @@ -1315,11 +1350,10 @@ amd64_get_insn_details (gdb_byte *insn,
> break;
> }
> }
> - else
> + else if (!need_modrm)
> {
> /* One-byte opcode. */
> need_modrm = onebyte_has_modrm[*insn];
> - details->opcode_len = 1;
> }
>
> if (need_modrm)
> @@ -1363,7 +1397,8 @@ fixup_riprel (struct gdbarch *gdbarch, a
> arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details);
> tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno);
>
> - /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */
> + /* Position of the not-B bit in the 3-byte VEX prefix, the EVEX prefix,
> + and the XOP prefix (always in byte 1). */
> static constexpr gdb_byte VEX3_NOT_B = 0x20;
>
> /* REX.B should be unset (VEX.!B set) as we were using rip-relative
> @@ -1374,7 +1409,8 @@ fixup_riprel (struct gdbarch *gdbarch, a
> gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset];
> if (rex_prefix_p (pfx[0]))
> pfx[0] &= ~REX_B;
> - else if (vex3_prefix_p (pfx[0]))
> + else if (vex3_prefix_p (pfx[0]) || xop_prefix_p (pfx)
> + || evex_prefix_p (pfx))
> pfx[1] |= VEX3_NOT_B;
> else
> gdb_assert_not_reached ("unhandled prefix");
>
>
>