* [PATCH] x86-64: fix unused register determination for displaced stepping
@ 2019-02-08 15:21 Jan Beulich
2019-02-26 12:26 ` Ping: " Jan Beulich
2019-04-05 8:30 ` Jan Beulich
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2019-02-08 15:21 UTC (permalink / raw)
To: GDB
For one, just like %rdx is an implied source operand for DIV/IDIV, %rcx
is one for shifts and rotates. Hence the register is better excluded
altogether as well. And then VEX- and XOP-encoded GPR insns often have a
3rd operand (encoded in the VEX/XOP prefix) which needs to be accounted
for as well.
Then again %rbp was mistakenly recorded as used in the specific case of
%rip-relative addressing we care about here. I'd like to note though
that there's a certain risk associated with using %rbp as replacement
base address register: Possible addressing related faults would then no
longer surface as #GP(0), but #SS(0). But it doesn't look to have been
the intention to avoid use of %rbp here.
As a side note, amd64_get_unused_input_int_reg() does too much for the
limited purpose it's getting used for anyway: It'll get called with
%rip-relative memory operands only, so
- there's always a ModR/M byte,
- ModR/M.mod is always going to be 0,
- ModR/M.rm is always going to be 5.
It might be worthwhile to remove the dead code (perhaps replaced by
assertions), at which point the comment getting changed here could be
adjusted in a different way, and it would become recognizable again that
we indeed can't run out of available (unused) registers.
---
This takes "x86-64: fix displaced stepping for VEX, XOP, and EVEX
encoded insns" as a prerequisite.
As with the earlier fix, time constraints are the reason for this not
being accompanied by a testsuite extension.
gdb/
2019-02-08 Jan Beulich <jbeulich@suse.com>
* amd64-tdep.c (): .
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1050,6 +1050,8 @@ struct amd64_insn
int opcode_offset;
/* The offset to the modrm byte or -1 if not present. */
int modrm_offset;
+ /* The VEX.vvvv encoded GPR or -1 if not present. */
+ int vex_gpr;
/* The raw instruction. */
gdb_byte *raw_insn;
@@ -1209,7 +1211,7 @@ amd64_get_unused_input_int_reg (const st
/* 1 bit for each reg */
int used_regs_mask = 0;
- /* There can be at most 3 int regs used as inputs in an insn, and we have
+ /* There can be at most 4 int regs used as inputs in an insn, and we have
7 to choose from (RAX ... RDI, sans RSP).
This allows us to take a conservative approach and keep things simple.
E.g. By avoiding RAX, we don't have to specifically watch for opcodes
@@ -1217,6 +1219,8 @@ amd64_get_unused_input_int_reg (const st
/* Avoid RAX. */
used_regs_mask |= 1 << EAX_REG_NUM;
+ /* Similarily avoid RCX, implicit operand in shifts/rotates. */
+ used_regs_mask |= 1 << ECX_REG_NUM;
/* Similarily avoid RDX, implicit operand in divides. */
used_regs_mask |= 1 << EDX_REG_NUM;
/* Avoid RSP. */
@@ -1246,12 +1250,15 @@ amd64_get_unused_input_int_reg (const st
used_regs_mask |= 1 << base;
used_regs_mask |= 1 << idx;
}
- else
+ else if (mod != 0 || rm != 5)
{
used_regs_mask |= 1 << rm;
}
}
+ if (details->vex_gpr >= 0 && details->vex_gpr < 8)
+ used_regs_mask |= 1 << details->vex_gpr;
+
gdb_assert (used_regs_mask < 256);
gdb_assert (used_regs_mask != 255);
@@ -1284,6 +1291,7 @@ amd64_get_insn_details (gdb_byte *insn,
details->enc_prefix_offset = -1;
details->opcode_offset = -1;
details->modrm_offset = -1;
+ details->vex_gpr = -1;
/* Skip legacy instruction prefixes. */
insn = amd64_skip_prefixes (insn);
@@ -1305,11 +1313,17 @@ amd64_get_insn_details (gdb_byte *insn,
{
details->enc_prefix_offset = insn - start;
need_modrm = (insn[1] & 0x1f) == 1 ? -1 : 1;
+ /* Record the 3rd (register) operand for VEX-encoded GPR insns. */
+ if ( (insn[1] & 0x1f) > 1 && insn[3] >= 0xf0 )
+ details->vex_gpr = (~insn[2] >> 3) & 0xf;
insn += 3;
}
else if (xop_prefix_p (insn))
{
details->enc_prefix_offset = insn - start;
+ /* Record the 3rd (register) operand for XOP-encoded GPR insns. */
+ if ( (insn[1] & 0x1f) > 8 && insn[3] < 0x20 )
+ details->vex_gpr = (~insn[2] >> 3) & 0xf;
insn += 3;
need_modrm = 1;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Ping: [PATCH] x86-64: fix unused register determination for displaced stepping
2019-02-08 15:21 [PATCH] x86-64: fix unused register determination for displaced stepping Jan Beulich
@ 2019-02-26 12:26 ` Jan Beulich
2019-04-05 8:30 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2019-02-26 12:26 UTC (permalink / raw)
To: GDB
>>> On 08.02.19 at 16:21, wrote:
> For one, just like %rdx is an implied source operand for DIV/IDIV, %rcx
> is one for shifts and rotates. Hence the register is better excluded
> altogether as well. And then VEX- and XOP-encoded GPR insns often have a
> 3rd operand (encoded in the VEX/XOP prefix) which needs to be accounted
> for as well.
>
> Then again %rbp was mistakenly recorded as used in the specific case of
> %rip-relative addressing we care about here. I'd like to note though
> that there's a certain risk associated with using %rbp as replacement
> base address register: Possible addressing related faults would then no
> longer surface as #GP(0), but #SS(0). But it doesn't look to have been
> the intention to avoid use of %rbp here.
>
> As a side note, amd64_get_unused_input_int_reg() does too much for the
> limited purpose it's getting used for anyway: It'll get called with
> %rip-relative memory operands only, so
> - there's always a ModR/M byte,
> - ModR/M.mod is always going to be 0,
> - ModR/M.rm is always going to be 5.
> It might be worthwhile to remove the dead code (perhaps replaced by
> assertions), at which point the comment getting changed here could be
> adjusted in a different way, and it would become recognizable again that
> we indeed can't run out of available (unused) registers.
>
> ---
> This takes "x86-64: fix displaced stepping for VEX, XOP, and EVEX
> encoded insns" as a prerequisite.
>
> As with the earlier fix, time constraints are the reason for this not
> being accompanied by a testsuite extension.
>
> gdb/
> 2019-02-08 Jan Beulich <jbeulich@suse.com>
>
> * amd64-tdep.c (): .
>
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1050,6 +1050,8 @@ struct amd64_insn
> int opcode_offset;
> /* The offset to the modrm byte or -1 if not present. */
> int modrm_offset;
> + /* The VEX.vvvv encoded GPR or -1 if not present. */
> + int vex_gpr;
>
> /* The raw instruction. */
> gdb_byte *raw_insn;
> @@ -1209,7 +1211,7 @@ amd64_get_unused_input_int_reg (const st
> /* 1 bit for each reg */
> int used_regs_mask = 0;
>
> - /* There can be at most 3 int regs used as inputs in an insn, and we have
> + /* There can be at most 4 int regs used as inputs in an insn, and we have
> 7 to choose from (RAX ... RDI, sans RSP).
> This allows us to take a conservative approach and keep things simple.
> E.g. By avoiding RAX, we don't have to specifically watch for opcodes
> @@ -1217,6 +1219,8 @@ amd64_get_unused_input_int_reg (const st
>
> /* Avoid RAX. */
> used_regs_mask |= 1 << EAX_REG_NUM;
> + /* Similarily avoid RCX, implicit operand in shifts/rotates. */
> + used_regs_mask |= 1 << ECX_REG_NUM;
> /* Similarily avoid RDX, implicit operand in divides. */
> used_regs_mask |= 1 << EDX_REG_NUM;
> /* Avoid RSP. */
> @@ -1246,12 +1250,15 @@ amd64_get_unused_input_int_reg (const st
> used_regs_mask |= 1 << base;
> used_regs_mask |= 1 << idx;
> }
> - else
> + else if (mod != 0 || rm != 5)
> {
> used_regs_mask |= 1 << rm;
> }
> }
>
> + if (details->vex_gpr >= 0 && details->vex_gpr < 8)
> + used_regs_mask |= 1 << details->vex_gpr;
> +
> gdb_assert (used_regs_mask < 256);
> gdb_assert (used_regs_mask != 255);
>
> @@ -1284,6 +1291,7 @@ amd64_get_insn_details (gdb_byte *insn,
> details->enc_prefix_offset = -1;
> details->opcode_offset = -1;
> details->modrm_offset = -1;
> + details->vex_gpr = -1;
>
> /* Skip legacy instruction prefixes. */
> insn = amd64_skip_prefixes (insn);
> @@ -1305,11 +1313,17 @@ amd64_get_insn_details (gdb_byte *insn,
> {
> details->enc_prefix_offset = insn - start;
> need_modrm = (insn[1] & 0x1f) == 1 ? -1 : 1;
> + /* Record the 3rd (register) operand for VEX-encoded GPR insns. */
> + if ( (insn[1] & 0x1f) > 1 && insn[3] >= 0xf0 )
> + details->vex_gpr = (~insn[2] >> 3) & 0xf;
> insn += 3;
> }
> else if (xop_prefix_p (insn))
> {
> details->enc_prefix_offset = insn - start;
> + /* Record the 3rd (register) operand for XOP-encoded GPR insns. */
> + if ( (insn[1] & 0x1f) > 8 && insn[3] < 0x20 )
> + details->vex_gpr = (~insn[2] >> 3) & 0xf;
> insn += 3;
> need_modrm = 1;
> }
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86-64: fix unused register determination for displaced stepping
2019-02-08 15:21 [PATCH] x86-64: fix unused register determination for displaced stepping Jan Beulich
2019-02-26 12:26 ` Ping: " Jan Beulich
@ 2019-04-05 8:30 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2019-04-05 8:30 UTC (permalink / raw)
To: GDB
>>> On 08.02.19 at 16:21, wrote:
> For one, just like %rdx is an implied source operand for DIV/IDIV, %rcx
> is one for shifts and rotates. Hence the register is better excluded
> altogether as well. And then VEX- and XOP-encoded GPR insns often have a
> 3rd operand (encoded in the VEX/XOP prefix) which needs to be accounted
> for as well.
>
> Then again %rbp was mistakenly recorded as used in the specific case of
> %rip-relative addressing we care about here. I'd like to note though
> that there's a certain risk associated with using %rbp as replacement
> base address register: Possible addressing related faults would then no
> longer surface as #GP(0), but #SS(0). But it doesn't look to have been
> the intention to avoid use of %rbp here.
>
> As a side note, amd64_get_unused_input_int_reg() does too much for the
> limited purpose it's getting used for anyway: It'll get called with
> %rip-relative memory operands only, so
> - there's always a ModR/M byte,
> - ModR/M.mod is always going to be 0,
> - ModR/M.rm is always going to be 5.
> It might be worthwhile to remove the dead code (perhaps replaced by
> assertions), at which point the comment getting changed here could be
> adjusted in a different way, and it would become recognizable again that
> we indeed can't run out of available (unused) registers.
>
> ---
> This takes "x86-64: fix displaced stepping for VEX, XOP, and EVEX
> encoded insns" as a prerequisite.
>
> As with the earlier fix, time constraints are the reason for this not
> being accompanied by a testsuite extension.
>
> gdb/
> 2019-02-08 Jan Beulich <jbeulich@suse.com>
>
> * amd64-tdep.c (): .
Btw, I've only now noticed this omission of mine. Here is the
missing chunk:
* amd64-tdep.c (struct amd64_insn): New field vex_gpr.
(amd64_get_unused_input_int_reg): Correct comment. Also avoid
ECX. Don't record EBP as used when it's not. Use vex_gpr.
(amd64_get_insn_details): Set vex_gpr.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-05 8:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 15:21 [PATCH] x86-64: fix unused register determination for displaced stepping Jan Beulich
2019-02-26 12:26 ` Ping: " Jan Beulich
2019-04-05 8:30 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).