public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
From: gdb-buildbot@sergiodj.net
To: gdb-testers@sourceware.org
Subject: [binutils-gdb] x86-64: fix Intel64 handling of branch with data16 prefix
Date: Fri, 27 Dec 2019 10:16:00 -0000	[thread overview]
Message-ID: <376cd056100dff2d6fc842aa013d0bbffdef363d@gdb-build> (raw)

*** TEST RESULTS FOR COMMIT 376cd056100dff2d6fc842aa013d0bbffdef363d ***

commit 376cd056100dff2d6fc842aa013d0bbffdef363d
Author:     Jan Beulich <jbeulich@suse.com>
AuthorDate: Fri Dec 27 09:38:34 2019 +0100
Commit:     Jan Beulich <jbeulich@suse.com>
CommitDate: Fri Dec 27 09:38:34 2019 +0100

    x86-64: fix Intel64 handling of branch with data16 prefix
    
    The expectation of x86-64-branch-3 for "call" / "jmp" with an obvious
    direct destination to translate to an indirect _far_ branch is plain
    wrong. The operand size prefix should have no effect at all on the
    interpretation of the operand. The main underlying issue here is that
    the Intel64 templates of the direct branches don't include Disp16, yet
    various assumptions exist that it would always be there when there's
    also Disp32/Disp32S, toggled by the operand size prefix (which is
    being ignored by direct branches in Intel64 mode).
    
    Along these lines it was also wrong to base the displacement width
    decision solely on the operand size prefix: REX.W cancels this effect
    and hence needs taking into consideration, too.
    
    A disassembler change is needed here as well: XBEGIN was wrongly treated
    the same as direct CALL/JMP, which isn't the case - the operand size
    prefix does affect displacement size there, it's merely ignored when it
    comes to updating [ER]IP.

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 9a51605129..578d76fe1a 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,16 @@
+2019-12-27  Jan Beulich  <jbeulich@suse.com>
+
+	* config/tc-i386.c (flip_code16): New.
+	(output_branch, output_jump): Use it.
+	(i386_displacement): Restrict template set to just direct
+	branches when handling a respective operand. Don't set Disp16
+	when in Intel64 mode and there's a respective template.
+	* testsuite/gas/i386/i386.exp: Convert x86-64-branch-3 from list
+	to dump test. Drop its XFail again.
+	* testsuite/gas/i386/x86-64-branch-3.d: New.
+	* testsuite/gas/i386/x86-64-branch-3.l: Delete.
+	* testsuite/gas/i386/x86-64-branch-3.s: Add XBEGIN case.
+
 2019-12-27  Jan Beulich  <jbeulich@suse.com>
 
 	* config/tc-i386.c (i386_addressing_mode): Declare.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 155e636d2b..770fa527a0 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7861,6 +7861,18 @@ build_modrm_byte (void)
   return default_seg;
 }
 
+static unsigned int
+flip_code16 (unsigned int code16)
+{
+  gas_assert (i.tm.operands == 1);
+
+  return !(i.prefix[REX_PREFIX] & REX_W)
+	 && (code16 ? i.tm.operand_types[0].bitfield.disp32
+		      || i.tm.operand_types[0].bitfield.disp32s
+		    : i.tm.operand_types[0].bitfield.disp16)
+	 ? CODE16 : 0;
+}
+
 static void
 output_branch (void)
 {
@@ -7880,7 +7892,7 @@ output_branch (void)
     {
       prefix = 1;
       i.prefixes -= 1;
-      code16 ^= CODE16;
+      code16 ^= flip_code16(code16);
     }
   /* Pentium4 branch hints.  */
   if (i.prefix[SEG_PREFIX] == CS_PREFIX_OPCODE /* not taken */
@@ -8022,7 +8034,7 @@ output_jump (void)
 	{
 	  FRAG_APPEND_1_CHAR (DATA_PREFIX_OPCODE);
 	  i.prefixes -= 1;
-	  code16 ^= CODE16;
+	  code16 ^= flip_code16(code16);
 	}
 
       size = 4;
@@ -9960,12 +9972,34 @@ i386_displacement (char *disp_start, char *disp_end)
     }
   else
     {
-      /* For PC-relative branches, the width of the displacement
-	 is dependent upon data size, not address size.  */
+      /* For PC-relative branches, the width of the displacement may be
+	 dependent upon data size, but is never dependent upon address size.
+	 Also make sure to not unintentionally match against a non-PC-relative
+	 branch template.  */
+      static templates aux_templates;
+      const insn_template *t = current_templates->start;
+      bfd_boolean has_intel64 = FALSE;
+
+      aux_templates.start = t;
+      while (++t < current_templates->end)
+	{
+	  if (t->opcode_modifier.jump
+	      != current_templates->start->opcode_modifier.jump)
+	    break;
+	  if (t->opcode_modifier.intel64)
+	    has_intel64 = TRUE;
+	}
+      if (t < current_templates->end)
+	{
+	  aux_templates.end = t;
+	  current_templates = &aux_templates;
+	}
+
       override = (i.prefix[DATA_PREFIX] != 0);
       if (flag_code == CODE_64BIT)
 	{
-	  if (override || i.suffix == WORD_MNEM_SUFFIX)
+	  if ((override || i.suffix == WORD_MNEM_SUFFIX)
+	      && (!intel64 || !has_intel64))
 	    bigdisp.bitfield.disp16 = 1;
 	  else
 	    bigdisp.bitfield.disp32s = 1;
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index 8caf8ff7e4..59a2723319 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1120,8 +1120,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 
 	run_dump_test "x86-64-jump"
 	run_dump_test "x86-64-branch-2"
-	setup_xfail "*-*-*"
-	run_list_test "x86-64-branch-3" "-al -mintel64"
+	run_dump_test "x86-64-branch-3"
 	run_list_test "x86-64-branch-4" "-al -mintel64"
 
 	run_dump_test "x86-64-gotpcrel"
diff --git a/gas/testsuite/gas/i386/x86-64-branch-3.d b/gas/testsuite/gas/i386/x86-64-branch-3.d
new file mode 100644
index 0000000000..c66647cd80
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-branch-3.d
@@ -0,0 +1,16 @@
+#as: -J -mintel64
+#objdump: -dwr -Mintel64
+#name: x86-64 branch 3
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <bar-0x6>:
+[ 	]*[a-f0-9]+:	66 e9 00 00 00 00    	data16 jmpq 6 <bar>	2: R_X86_64_PLT32	foo-0x4
+
+0+6 <bar>:
+[ 	]*[a-f0-9]+:	89 c3                	mov    %eax,%ebx
+[ 	]*[a-f0-9]+:	66 e8 00 00 00 00    	data16 callq e <bar\+0x8>	a: R_X86_64_PLT32	foo-0x4
+[ 	]*[a-f0-9]+:	66 c7 f8 00 00       	xbeginw 13 <bar\+0xd>	11: R_X86_64_PC16	foo-0x2
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-branch-3.l b/gas/testsuite/gas/i386/x86-64-branch-3.l
deleted file mode 100644
index de3c2dd49b..0000000000
--- a/gas/testsuite/gas/i386/x86-64-branch-3.l
+++ /dev/null
@@ -1,17 +0,0 @@
-.*: Assembler messages:
-.*:2: Warning: indirect jmp without `\*'
-.*:7: Warning: indirect call without `\*'
-GAS LISTING .*
-
-
-[ 	]*1[ 	]+\.text
-[ 	]*2[ 	]+0000 66FF2C25 		data16 jmp foo
-\*\*\*\*  Warning: indirect jmp without `\*'
-[ 	]*2[ 	]+00000000 
-[ 	]*3[ 	]+
-[ 	]*4[ 	]+bar:
-[ 	]*5[ 	]+0008 89C3     		mov %eax, %ebx
-[ 	]*6[ 	]+
-[ 	]*7[ 	]+000a 66FF1C25 		data16 call foo
-\*\*\*\*  Warning: indirect call without `\*'
-[ 	]*7[ 	]+00000000 
diff --git a/gas/testsuite/gas/i386/x86-64-branch-3.s b/gas/testsuite/gas/i386/x86-64-branch-3.s
index 16c85a3f7b..42bdac001f 100644
--- a/gas/testsuite/gas/i386/x86-64-branch-3.s
+++ b/gas/testsuite/gas/i386/x86-64-branch-3.s
@@ -5,3 +5,5 @@ bar:
 	mov %eax, %ebx
 
 	data16 call foo
+
+	data16 xbegin foo
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index d38abe2e44..9a15123042 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,10 @@
+2019-12-27  Jan Beulich  <jbeulich@suse.com>
+
+	* i386-dis.c (Jdqw): Define.
+	(dqw_mode): Adjust associated comment.
+	(rm_table): Use Jdqw for XBEGIN.
+	(OP_J): Handle dqw_mode.
+
 2019-12-27  Jan Beulich  <jbeulich@suse.com>
 
 	* i386-gen.c (process_i386_operand_type): Don't set Disp32 for
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 2a0e765c55..6693b70629 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -296,6 +296,7 @@ fetch_data (struct disassemble_info *info, bfd_byte *addr)
 #define I1 { OP_I, const_1_mode }
 #define Jb { OP_J, b_mode }
 #define Jv { OP_J, v_mode }
+#define Jdqw { OP_J, dqw_mode }
 #define Cm { OP_C, m_mode }
 #define Dm { OP_D, m_mode }
 #define Td { OP_T, d_mode }
@@ -558,7 +559,8 @@ enum
   v_bndmk_mode,
   /* operand size depends on REX prefixes.  */
   dq_mode,
-  /* registers like dq_mode, memory like w_mode.  */
+  /* registers like dq_mode, memory like w_mode, displacements like
+     v_mode without considering Intel64 ISA.  */
   dqw_mode,
   /* bounds operand */
   bnd_mode,
@@ -10969,7 +10971,7 @@ static const struct dis386 rm_table[][8] = {
   },
   {
     /* RM_C7_REG_7 */
-    { "xbeginT",	{ Skip_MODRM, Jv }, 0 },
+    { "xbeginT",	{ Skip_MODRM, Jdqw }, 0 },
   },
   {
     /* RM_0F01_REG_0 */
@@ -14828,10 +14830,12 @@ OP_J (int bytemode, int sizeflag)
       break;
     case v_mode:
       if (isa64 == amd64)
+    case dqw_mode:
 	USED_REX (REX_W);
       if ((sizeflag & DFLAG)
 	  || (address_mode == mode_64bit
-	      && (isa64 != amd64 || (rex & REX_W))))
+	      && ((isa64 != amd64 && bytemode != dqw_mode)
+		  || (rex & REX_W))))
 	disp = get32s ();
       else
 	{


             reply	other threads:[~2019-12-27 10:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-27 10:16 gdb-buildbot [this message]
2019-12-27 10:27 ` Failures on Ubuntu-Aarch64-native-extended-gdbserver-m64, branch master gdb-buildbot
2019-12-27 10:28 ` Failures on Fedora-i686, " gdb-buildbot
2019-12-27 10:29 ` Failures on Fedora-x86_64-m32, " gdb-buildbot
2019-12-27 10:37 ` Failures on Fedora-x86_64-cc-with-index, " gdb-buildbot
2019-12-27 11:02 ` Failures on Fedora-x86_64-m64, " gdb-buildbot
2019-12-27 11:11 ` Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot
2019-12-27 11:12 ` Failures on Fedora-x86_64-native-extended-gdbserver-m64, " gdb-buildbot
2019-12-27 17:01 ` Failures on Fedora-x86_64-native-gdbserver-m64, " gdb-buildbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=376cd056100dff2d6fc842aa013d0bbffdef363d@gdb-build \
    --to=gdb-buildbot@sergiodj.net \
    --cc=gdb-testers@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).