* [binutils-gdb] x86: properly respect rex/{rex}
@ 2023-12-22 8:37 Jan Beulich
0 siblings, 0 replies; only message in thread
From: Jan Beulich @ 2023-12-22 8:37 UTC (permalink / raw)
To: bfd-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ce7056886a6e52a4fc91b5e3b28da2c0735d0b19
commit ce7056886a6e52a4fc91b5e3b28da2c0735d0b19
Author: Jan Beulich <jbeulich@suse.com>
Date: Fri Dec 22 09:33:12 2023 +0100
x86: properly respect rex/{rex}
This addresses two issues: For one, a user specified "rex" did not cause
the diagnostic to trigger when there was no other need for a REX prefix;
instead, another than the specified insn+operands was encoded. And then
(which is what this started from) .insn didn't respect {rex} (and was
otherwise similarly flawed as ordinary insns).
The latter requires splitting out code from md_assemble(), for it to
become re-usable. Besides the addition to address the first issue, that
code then also needs generalizing to account for immediate operands, as
with .insn we can't make assumptions anymore on all respective templates
having at most two operands (we still can build upon there being at most
two non-immediate operands, though). While moving the code also simplify
the first if(), by folding redundant checks.
In the new testcase also test a few more things which afaics weren't
tested till now.
Diff:
---
gas/config/tc-i386.c | 133 +++++++++++++++++---------------
gas/testsuite/gas/i386/rex-bad.l | 10 +++
gas/testsuite/gas/i386/rex-bad.s | 13 ++++
gas/testsuite/gas/i386/x86-64-pseudos.d | 1 +
gas/testsuite/gas/i386/x86-64-pseudos.s | 2 +
gas/testsuite/gas/i386/x86-64.exp | 1 +
6 files changed, 98 insertions(+), 62 deletions(-)
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index c84374a08d1..6e2f6e51457 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -4126,6 +4126,74 @@ build_evex_prefix (void)
i.vex.bytes[3] |= i.mask.reg->reg_num;
}
+static void establish_rex (void)
+{
+ /* Note that legacy encodings have at most 2 non-immediate operands. */
+ unsigned int first = i.imm_operands;
+ unsigned int last = i.operands > first ? i.operands - first - 1 : first;
+
+ /* Respect a user-specified REX prefix. */
+ i.rex |= i.prefix[REX_PREFIX] & REX_OPCODE;
+
+ /* For 8 bit registers we need an empty rex prefix. Also if the
+ instruction already has a prefix, we need to convert old
+ registers to new ones. */
+
+ if ((i.types[first].bitfield.class == Reg && i.types[first].bitfield.byte
+ && ((i.op[first].regs->reg_flags & RegRex64) != 0 || i.rex != 0))
+ || (i.types[last].bitfield.class == Reg && i.types[last].bitfield.byte
+ && ((i.op[last].regs->reg_flags & RegRex64) != 0 || i.rex != 0)))
+ {
+ unsigned int x;
+
+ i.rex |= REX_OPCODE;
+ for (x = first; x <= last; x++)
+ {
+ /* Look for 8 bit operand that uses old registers. */
+ if (i.types[x].bitfield.class == Reg && i.types[x].bitfield.byte
+ && (i.op[x].regs->reg_flags & RegRex64) == 0)
+ {
+ gas_assert (!(i.op[x].regs->reg_flags & RegRex));
+ /* In case it is "hi" register, give up. */
+ if (i.op[x].regs->reg_num > 3)
+ as_bad (_("can't encode register '%s%s' in an "
+ "instruction requiring REX prefix"),
+ register_prefix, i.op[x].regs->reg_name);
+
+ /* Otherwise it is equivalent to the extended register.
+ Since the encoding doesn't change this is merely
+ cosmetic cleanup for debug output. */
+ i.op[x].regs += 8;
+ }
+ }
+ }
+
+ if (i.rex == 0 && i.rex_encoding)
+ {
+ /* Check if we can add a REX_OPCODE byte. Look for 8 bit operand
+ that uses legacy register. If it is "hi" register, don't add
+ the REX_OPCODE byte. */
+ unsigned int x;
+
+ for (x = first; x <= last; x++)
+ if (i.types[x].bitfield.class == Reg
+ && i.types[x].bitfield.byte
+ && (i.op[x].regs->reg_flags & RegRex64) == 0
+ && i.op[x].regs->reg_num > 3)
+ {
+ gas_assert (!(i.op[x].regs->reg_flags & RegRex));
+ i.rex_encoding = false;
+ break;
+ }
+
+ if (i.rex_encoding)
+ i.rex = REX_OPCODE;
+ }
+
+ if (i.rex != 0)
+ add_prefix (REX_OPCODE | i.rex);
+}
+
static void
process_immext (void)
{
@@ -5623,66 +5691,7 @@ md_assemble (char *line)
i.op[0].disps->X_op = O_symbol;
}
- /* For 8 bit registers we need an empty rex prefix. Also if the
- instruction already has a prefix, we need to convert old
- registers to new ones. */
-
- if ((i.types[0].bitfield.class == Reg && i.types[0].bitfield.byte
- && (i.op[0].regs->reg_flags & RegRex64) != 0)
- || (i.types[1].bitfield.class == Reg && i.types[1].bitfield.byte
- && (i.op[1].regs->reg_flags & RegRex64) != 0)
- || (((i.types[0].bitfield.class == Reg && i.types[0].bitfield.byte)
- || (i.types[1].bitfield.class == Reg && i.types[1].bitfield.byte))
- && i.rex != 0))
- {
- int x;
-
- i.rex |= REX_OPCODE;
- for (x = 0; x < 2; x++)
- {
- /* Look for 8 bit operand that uses old registers. */
- if (i.types[x].bitfield.class == Reg && i.types[x].bitfield.byte
- && (i.op[x].regs->reg_flags & RegRex64) == 0)
- {
- gas_assert (!(i.op[x].regs->reg_flags & RegRex));
- /* In case it is "hi" register, give up. */
- if (i.op[x].regs->reg_num > 3)
- as_bad (_("can't encode register '%s%s' in an "
- "instruction requiring REX prefix."),
- register_prefix, i.op[x].regs->reg_name);
-
- /* Otherwise it is equivalent to the extended register.
- Since the encoding doesn't change this is merely
- cosmetic cleanup for debug output. */
-
- i.op[x].regs = i.op[x].regs + 8;
- }
- }
- }
-
- if (i.rex == 0 && i.rex_encoding)
- {
- /* Check if we can add a REX_OPCODE byte. Look for 8 bit operand
- that uses legacy register. If it is "hi" register, don't add
- the REX_OPCODE byte. */
- int x;
- for (x = 0; x < 2; x++)
- if (i.types[x].bitfield.class == Reg
- && i.types[x].bitfield.byte
- && (i.op[x].regs->reg_flags & RegRex64) == 0
- && i.op[x].regs->reg_num > 3)
- {
- gas_assert (!(i.op[x].regs->reg_flags & RegRex));
- i.rex_encoding = false;
- break;
- }
-
- if (i.rex_encoding)
- i.rex = REX_OPCODE;
- }
-
- if (i.rex != 0)
- add_prefix (REX_OPCODE | i.rex);
+ establish_rex ();
insert_lfence_before (last_insn);
@@ -11698,8 +11707,8 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
build_evex_prefix ();
i.rex &= REX_OPCODE;
}
- else if (i.rex != 0)
- add_prefix (REX_OPCODE | i.rex);
+ else
+ establish_rex ();
last_insn = &seg_info(now_seg)->tc_segment_info_data.last_insn;
output_insn (last_insn);
diff --git a/gas/testsuite/gas/i386/rex-bad.l b/gas/testsuite/gas/i386/rex-bad.l
new file mode 100644
index 00000000000..407558ec541
--- /dev/null
+++ b/gas/testsuite/gas/i386/rex-bad.l
@@ -0,0 +1,10 @@
+.*: Assembler messages:
+.*:4: Error: same .*
+.*:5: Error: same .*
+.*:6: Error: same .*
+.*:7: Error: same .*
+.*:9: Error: .* REX .*
+.*:10: Error: .* REX .*
+.*:12: Error: .* REX .*
+.*:13: Error: .* REX .*
+#pass
diff --git a/gas/testsuite/gas/i386/rex-bad.s b/gas/testsuite/gas/i386/rex-bad.s
new file mode 100644
index 00000000000..f6ba6b48b2c
--- /dev/null
+++ b/gas/testsuite/gas/i386/rex-bad.s
@@ -0,0 +1,13 @@
+ .text
+
+_start:
+ rex.w add (%rax,%rax), %rax
+ rex.r add (%rax,%rax), %r8
+ rex.b add (%r8,%rax), %eax
+ rex.x add (%rax,%r8), %eax
+
+ rex mov %al, %ch
+ rex mov %ah, %cl
+
+ .insn rex 0x88, %al, %ch
+ .insn rex 0x88, %ah, %cl
diff --git a/gas/testsuite/gas/i386/x86-64-pseudos.d b/gas/testsuite/gas/i386/x86-64-pseudos.d
index 0cc75ef2457..866a804ab92 100644
--- a/gas/testsuite/gas/i386/x86-64-pseudos.d
+++ b/gas/testsuite/gas/i386/x86-64-pseudos.d
@@ -470,4 +470,5 @@ Disassembly of section .text:
+[a-f0-9]+: 41 8a 45 00 mov 0x0\(%r13\),%al
+[a-f0-9]+: 67 41 8a 45 00 mov 0x0\(%r13d\),%al
+[a-f0-9]+: 67 41 8a 85 00 00 00 00 mov 0x0\(%r13d\),%al
+ +[a-f0-9]+: 40 8a c1 rex mov %cl,%al
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-pseudos.s b/gas/testsuite/gas/i386/x86-64-pseudos.s
index 08fac8381c6..06f0b62d049 100644
--- a/gas/testsuite/gas/i386/x86-64-pseudos.s
+++ b/gas/testsuite/gas/i386/x86-64-pseudos.s
@@ -438,3 +438,5 @@ _start:
mov al, BYTE PTR [r13]
{disp8} mov al, BYTE PTR [r13d]
{disp32} mov al, BYTE PTR [r13d]
+
+ .insn {rex} 0x8a, al, cl
diff --git a/gas/testsuite/gas/i386/x86-64.exp b/gas/testsuite/gas/i386/x86-64.exp
index a7f5547017f..e4b0cc8b85b 100644
--- a/gas/testsuite/gas/i386/x86-64.exp
+++ b/gas/testsuite/gas/i386/x86-64.exp
@@ -596,6 +596,7 @@ if { ![istarget "*-*-aix*"]
&& ![istarget "*-*-solaris*"]
&& ![istarget "*-*-sysv*"] } then {
run_dump_test "rex"
+ run_list_test "rex-bad"
}
# ELF specific tests
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-12-22 8:37 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-22 8:37 [binutils-gdb] x86: properly respect rex/{rex} 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).