[PATCH 1/3] x86-64: fix Intel64 handling of branch with data16 prefix

Jan Beulich jbeulich@suse.com
Fri Dec 20 10:38:00 GMT 2019


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 too: 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.

Once in this area also
- suppress Disp<N> conversion based on address size prefix when this
  prefix doesn't control the width of the displacement,
- suppress meaningless setting of Disp32 in logic dealing with just
  64-bit code,
- add the missing Disp32S to XBEGIN.

gas/
2019-12-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386-intel.c (match_template): Don't transform
	displacement width flags for non-indirect branches.
	(flip_code16): New.
	(output_branch, output_jump): Use it.
	(i386_displacement): Also check BaseIndex when deciding
	whether an operand belongs to a direct branch. 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. Don't set Disp32 in 64-bit mode.
	* testsuite/gas/i386/i386.exp: Convert x86-64-branch-3 from list
	to dump test.
	* 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.

opcodes/
2019-12-XX  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.
	* i386-opc.tbl (xbegin): Add Disp32S.
	* i386-tbl.h: Re-generate.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5885,7 +5885,9 @@ match_template (char mnem_suffix)
 
       /* Address size prefix will turn Disp64/Disp32/Disp16 operand
 	 into Disp32/Disp16/Disp32 operand.  */
-      if (i.prefix[ADDR_PREFIX] != 0)
+      if (i.prefix[ADDR_PREFIX]
+	  && (!t->opcode_modifier.jump
+	      || t->opcode_modifier.jump == JUMP_ABSOLUTE))
 	  {
 	    /* There should be only one Disp operand.  */
 	    switch (flag_code)
@@ -7861,6 +7863,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 +7894,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 +8036,7 @@ output_jump (void)
 	{
 	  FRAG_APPEND_1_CHAR (DATA_PREFIX_OPCODE);
 	  i.prefixes -= 1;
-	  code16 ^= CODE16;
+	  code16 ^= flip_code16(code16);
 	}
 
       size = 4;
@@ -9937,6 +9951,7 @@ i386_displacement (char *disp_start, cha
 
   operand_type_set (&bigdisp, 0);
   if (i.jumpabsolute
+      || i.types[this_operand].bitfield.baseindex
       || (current_templates->start->opcode_modifier.jump != JUMP
 	  && current_templates->start->opcode_modifier.jump != JUMP_DWORD))
     {
@@ -9958,18 +9973,37 @@ i386_displacement (char *disp_start, cha
     }
   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.disp32 = 1;
-	      bigdisp.bitfield.disp32s = 1;
-	    }
+	    bigdisp.bitfield.disp32s = 1;
 	}
       else
 	{
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1124,7 +1124,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
 
 	run_dump_test "x86-64-jump"
 	run_dump_test "x86-64-branch-2"
-	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"
--- /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
--- 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 
--- 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
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -297,6 +297,7 @@ fetch_data (struct disassemble_info *inf
 #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 }
@@ -560,7 +561,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,
@@ -10971,7 +10973,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 */
@@ -14830,10 +14832,12 @@ OP_J (int bytemode, int sizeflag)
       break;
     case v_mode:
       if (isa64 != intel64)
+    case dqw_mode:
 	USED_REX (REX_W);
       if ((sizeflag & DFLAG)
 	  || (address_mode == mode_64bit
-	      && (isa64 == intel64 || (rex & REX_W))))
+	      && ((isa64 == intel64 && bytemode != dqw_mode)
+		  || (rex & REX_W))))
 	disp = get32s ();
       else
 	{
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -2552,7 +2552,7 @@ xrelease, 0, 0xf3, None, 1, CpuHLE, No_b
 
 // RTM instructions
 xabort, 1, 0xc6f8, None, 2, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8 }
-xbegin, 1, 0xc7f8, None, 2, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp16|Disp32 }
+xbegin, 1, 0xc7f8, None, 2, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp16|Disp32|Disp32S }
 xend, 0, 0xf01d5, None, 3, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 0 }
 xtest, 0, 0xf01d6, None, 3, CpuHLE|CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 0 }
 



More information about the Binutils mailing list