public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] gas/x86: displacement handling adjustments
@ 2022-06-30 12:06 Jan Beulich
  2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jan Beulich @ 2022-06-30 12:06 UTC (permalink / raw)
  To: Binutils

The primary goal here is the last patch, increasing the headroom
we have in the operand type structure for future additions, while
at the same time also reducing code size a little. When initially
validating the concept as viable, I stumbled across two issues
which the first two patches deal with. Unfortunately this leads
to a little back and forth in lex_got()'s internal lookup table
(and i386-gen's underlying macros); This would be avoidable only
if folding the last two patches, which I don't think is desirable.

1: x86-64: improve handling of branches to absolute addresses
2: x86: restore masking of displacement kinds
3: x86: fold Disp32S and Disp32

Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] x86-64: improve handling of branches to absolute addresses
  2022-06-30 12:06 [PATCH 0/3] gas/x86: displacement handling adjustments Jan Beulich
@ 2022-06-30 12:08 ` Jan Beulich
  2022-06-30 17:50   ` H.J. Lu
  2022-06-30 12:08 ` [PATCH 2/3] x86: restore masking of displacement kinds Jan Beulich
  2022-06-30 12:10 ` [PATCH 3/3] x86: fold Disp32S and Disp32 Jan Beulich
  2 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2022-06-30 12:08 UTC (permalink / raw)
  To: Binutils

There are two related problems here: The use of "addr32" on a direct
branch would, besides causing a warning, result in operands to be
permitted which mistakenly are refused without "addr32". Plus at some
point not too long ago I'm afraid it may have been me who regressed the
relocation addends emitted for such branches. Correct both problems,
adding a testcase to guard against regressing this again.
---
In principle things like "JECXZ <absolute>" should be permitted as well,
I think. Whether the destination is within reach can only be determined
by the linker. But that likely being a more intrusive change, I guess we
can leave this as is until someone really needs it to work.

If "ELF: emit symbol table when there are relocations" (submitted
earlier) goes in before this change, I'd be inclined to drop the label
again from the new testcase. The original lack of a label there was how
I noticed that other issue, so the testcase here could at once serve to
test that changed behavior as well.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -4975,7 +4975,9 @@ md_assemble (char *line)
   if (i.imm_operands)
     optimize_imm ();
 
-  if (i.disp_operands && !want_disp32 (current_templates->start))
+  if (i.disp_operands && !want_disp32 (current_templates->start)
+      && (!current_templates->start->opcode_modifier.jump
+	  || i.jumpabsolute || i.types[0].bitfield.baseindex))
     {
       for (j = 0; j < i.operands; ++j)
 	{
@@ -5985,7 +5987,9 @@ optimize_disp (void)
 	    /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
 	    if ((i.types[op].bitfield.disp32
 		 || (flag_code == CODE_64BIT
-		     && want_disp32 (current_templates->start)))
+		     && want_disp32 (current_templates->start)
+		     && (!current_templates->start->opcode_modifier.jump
+			 || i.jumpabsolute || i.types[op].bitfield.baseindex)))
 		&& fits_in_unsigned_long (op_disp))
 	      {
 		/* If this operand is at most 32 bits, convert
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1314,6 +1314,7 @@ if [gas_64_check] then {
 	run_dump_test "x86-64-branch-3"
 	run_list_test "x86-64-branch-4" "-al -mintel64"
 	run_list_test "x86-64-branch-5" "-al"
+	run_dump_test "x86-64-branch-6"
 
 	run_dump_test "x86-64-rip-2"
 
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-branch-6.d
@@ -0,0 +1,21 @@
+#objdump: -r
+#name: x86-64 branch 6
+#warning_output: x86-64-branch-6.e
+
+.*: +file format .*
+
+RELOCATION RECORDS FOR \[\.text\]:
+OFFSET +TYPE +VALUE *
+0+01 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+11 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+0+21 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+31 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+0+07 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+0c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+17 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+0+1c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+0+27 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+2c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
+0+37 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+0+3c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-branch-6.s
@@ -0,0 +1,18 @@
+	.text
+
+branch_6:
+	call	0x87654321
+	je	0x87654321
+	jmp	0x87654321
+
+	call	0x876543210
+	je	0x876543210
+	jmp	0x876543210
+
+	addr32 call	0x87654321
+	addr32 je	0x87654321
+	addr32 jmp	0x87654321
+
+	addr32 call	0x876543210
+	addr32 je	0x876543210
+	addr32 jmp	0x876543210
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-branch-6.e
@@ -0,0 +1,7 @@
+.*: Assembler messages:
+.*:12: Warning: skipping prefixes on `call'
+.*:13: Warning: skipping prefixes on `je'
+.*:14: Warning: skipping prefixes on `jmp'
+.*:16: Warning: skipping prefixes on `call'
+.*:17: Warning: skipping prefixes on `je'
+.*:18: Warning: skipping prefixes on `jmp'


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] x86: restore masking of displacement kinds
  2022-06-30 12:06 [PATCH 0/3] gas/x86: displacement handling adjustments Jan Beulich
  2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
@ 2022-06-30 12:08 ` Jan Beulich
  2022-06-30 22:47   ` H.J. Lu
  2022-06-30 12:10 ` [PATCH 3/3] x86: fold Disp32S and Disp32 Jan Beulich
  2 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2022-06-30 12:08 UTC (permalink / raw)
  To: Binutils

Commit 7d5e4556a375 rendered the check near the end of what is now
i386_finalize_displacement() entirely dead for AT&T mode, since for
operands involving a displacement .unspecified will always be set. But
the logic there is bogus anyway - Intel syntax operand size specifiers
are of no interest there either. The only thing which matters in the
"displacement only" determination is .baseindex.

Of course when masking displacement kinds we should not at the same time
also mask off other attributes.

Furthermore the type mask returned by lex_got() also needs to be
adjusted: The only case where we want Disp32 (rather than Disp32S) is
when dealing with 32-bit addressing mode in 64-bit code.
---
Actually I don't understand why this masking has been dependent on
"displacement only". I would expect the masking to similarly apply to
displacements used with any kind of base/index expression.

I was considering to drop the OPERAND_TYPE_IMM32_32S_* that are being
changed, in favor of using C99 dedicated initializers. But perhaps this
would better be a separate change, dealing with the other similar
constants as well.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -10380,7 +10380,7 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_IMM64, true },
     { STRING_COMMA_LEN ("PLT"),      { BFD_RELOC_386_PLT32,
 				       BFD_RELOC_X86_64_PLT32    },
-      OPERAND_TYPE_IMM32_32S_DISP32, false },
+      OPERAND_TYPE_IMM32_32S_DISP32S, false },
     { STRING_COMMA_LEN ("GOTPLT"),   { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_GOTPLT64 },
       OPERAND_TYPE_IMM64_DISP64, true },
@@ -10389,28 +10389,28 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_IMM64_DISP64, true },
     { STRING_COMMA_LEN ("GOTPCREL"), { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_GOTPCREL },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
     { STRING_COMMA_LEN ("TLSGD"),    { BFD_RELOC_386_TLS_GD,
 				       BFD_RELOC_X86_64_TLSGD    },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
     { STRING_COMMA_LEN ("TLSLDM"),   { BFD_RELOC_386_TLS_LDM,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("TLSLD"),    { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_TLSLD    },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
     { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32,
 				       BFD_RELOC_X86_64_GOTTPOFF },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
     { STRING_COMMA_LEN ("TPOFF"),    { BFD_RELOC_386_TLS_LE_32,
 				       BFD_RELOC_X86_64_TPOFF32  },
-      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
     { STRING_COMMA_LEN ("NTPOFF"),   { BFD_RELOC_386_TLS_LE,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("DTPOFF"),   { BFD_RELOC_386_TLS_LDO_32,
 				       BFD_RELOC_X86_64_DTPOFF32 },
-      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
     { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
@@ -10419,17 +10419,17 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("GOT"),      { BFD_RELOC_386_GOT32,
 				       BFD_RELOC_X86_64_GOT32    },
-      OPERAND_TYPE_IMM32_32S_64_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32S, true },
     { STRING_COMMA_LEN ("TLSDESC"),  { BFD_RELOC_386_TLS_GOTDESC,
 				       BFD_RELOC_X86_64_GOTPC32_TLSDESC },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
     { STRING_COMMA_LEN ("TLSCALL"),  { BFD_RELOC_386_TLS_DESC_CALL,
 				       BFD_RELOC_X86_64_TLSDESC_CALL },
-      OPERAND_TYPE_IMM32_32S_DISP32, true },
+      OPERAND_TYPE_IMM32_32S_DISP32S, true },
 #else /* TE_PE */
     { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL,
 				       BFD_RELOC_32_SECREL },
-      OPERAND_TYPE_IMM32_32S_64_DISP32_64, false },
+      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, false },
 #endif
   };
   char *cp;
@@ -11144,7 +11144,6 @@ static int
 i386_finalize_displacement (segT exp_seg ATTRIBUTE_UNUSED, expressionS *exp,
 			    i386_operand_type types, const char *disp_start)
 {
-  i386_operand_type bigdisp;
   int ret = 1;
 
   /* We do this to make sure that the section symbol is in
@@ -11210,10 +11209,10 @@ i386_finalize_displacement (segT exp_seg
     i.types[this_operand].bitfield.disp8 = 1;
 
   /* Check if this is a displacement only operand.  */
-  bigdisp = operand_type_and_not (i.types[this_operand], anydisp);
-  if (operand_type_all_zero (&bigdisp))
-    i.types[this_operand] = operand_type_and (i.types[this_operand],
-					      types);
+  if (!i.types[this_operand].bitfield.baseindex)
+    i.types[this_operand] =
+      operand_type_or (operand_type_and_not (i.types[this_operand], anydisp),
+		       operand_type_and (i.types[this_operand], types));
 
   return ret;
 }
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -529,14 +529,14 @@ static initializer operand_type_init[] =
     "Imm16|Imm32|Imm32S" },
   { "OPERAND_TYPE_IMM32_64",
     "Imm32|Imm64" },
-  { "OPERAND_TYPE_IMM32_32S_DISP32",
-    "Imm32|Imm32S|Disp32" },
+  { "OPERAND_TYPE_IMM32_32S_DISP32S",
+    "Imm32|Imm32S|Disp32S" },
   { "OPERAND_TYPE_IMM64_DISP64",
     "Imm64|Disp64" },
-  { "OPERAND_TYPE_IMM32_32S_64_DISP32",
-    "Imm32|Imm32S|Imm64|Disp32" },
-  { "OPERAND_TYPE_IMM32_32S_64_DISP32_64",
-    "Imm32|Imm32S|Imm64|Disp32|Disp64" },
+  { "OPERAND_TYPE_IMM32_32S_64_DISP32S",
+    "Imm32|Imm32S|Imm64|Disp32S" },
+  { "OPERAND_TYPE_IMM32_32S_64_DISP32S_64",
+    "Imm32|Imm32S|Imm64|Disp32S|Disp64" },
   { "OPERAND_TYPE_ANYIMM",
     "Imm1|Imm8|Imm8S|Imm16|Imm32|Imm32S|Imm64" },
 };


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] x86: fold Disp32S and Disp32
  2022-06-30 12:06 [PATCH 0/3] gas/x86: displacement handling adjustments Jan Beulich
  2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
  2022-06-30 12:08 ` [PATCH 2/3] x86: restore masking of displacement kinds Jan Beulich
@ 2022-06-30 12:10 ` Jan Beulich
  2022-06-30 22:51   ` H.J. Lu
  2 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2022-06-30 12:10 UTC (permalink / raw)
  To: Binutils

The only case where 64-bit code uses non-sign-extended (can also be
considered zero-extended) displacements is when an address size override
is in place for a memory operand (i.e. particularly excluding
displacements of direct branches, which - if at all - are controlled by
operand size, and then are still sign-extended, just from 16 bits).
Hence the distinction in templates is unnecessary, allowing code to be
simplified in a number of places. The only place where logic becomes
more complicated is when signed-ness of relocations is determined in
output_disp().

The other caveat is that Disp64 cannot be specified anymore in an insn
template at the same time as Disp32. Unlike for non-64-bit mode,
templates don't specify displacements for both possible addressing
modes; the necessary adjustment to the expected ones has already been
done in match_template() anyway (but of course the logic there needs
tweaking now). Hence the single template so far doing so is split.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2142,14 +2142,12 @@ operand_type_check (i386_operand_type t,
       return (t.bitfield.disp8
 	      || t.bitfield.disp16
 	      || t.bitfield.disp32
-	      || t.bitfield.disp32s
 	      || t.bitfield.disp64);
 
     case anymem:
       return (t.bitfield.disp8
 	      || t.bitfield.disp16
 	      || t.bitfield.disp32
-	      || t.bitfield.disp32s
 	      || t.bitfield.disp64
 	      || t.bitfield.baseindex);
 
@@ -2414,8 +2412,7 @@ mode_from_disp_size (i386_operand_type t
   if (t.bitfield.disp8)
     return 1;
   else if (t.bitfield.disp16
-	   || t.bitfield.disp32
-	   || t.bitfield.disp32s)
+	   || t.bitfield.disp32)
     return 2;
   else
     return 0;
@@ -3326,7 +3323,6 @@ const type_names[] =
   { OPERAND_TYPE_DISP8, "d8" },
   { OPERAND_TYPE_DISP16, "d16" },
   { OPERAND_TYPE_DISP32, "d32" },
-  { OPERAND_TYPE_DISP32S, "d32s" },
   { OPERAND_TYPE_DISP64, "d64" },
   { OPERAND_TYPE_INOUTPORTREG, "InOutPortReg" },
   { OPERAND_TYPE_SHIFTCOUNT, "ShiftCount" },
@@ -4990,12 +4986,11 @@ md_assemble (char *line)
 	    continue;
 
 	  /* Since displacement is signed extended to 64bit, don't allow
-	     disp32 and turn off disp32s if they are out of range.  */
-	  i.types[j].bitfield.disp32 = 0;
+	     disp32 if it is out of range.  */
 	  if (fits_in_signed_long (exp->X_add_number))
 	    continue;
 
-	  i.types[j].bitfield.disp32s = 0;
+	  i.types[j].bitfield.disp32 = 0;
 	  if (i.types[j].bitfield.baseindex)
 	    {
 	      char number_buf[128];
@@ -5985,11 +5980,11 @@ optimize_disp (void)
 
 #ifdef BFD64
 	    /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
-	    if ((i.types[op].bitfield.disp32
-		 || (flag_code == CODE_64BIT
-		     && want_disp32 (current_templates->start)
-		     && (!current_templates->start->opcode_modifier.jump
-			 || i.jumpabsolute || i.types[op].bitfield.baseindex)))
+	    if ((flag_code != CODE_64BIT
+		 ? i.types[op].bitfield.disp32
+		 : want_disp32 (current_templates->start)
+		   && (!current_templates->start->opcode_modifier.jump
+		       || i.jumpabsolute || i.types[op].bitfield.baseindex))
 		&& fits_in_unsigned_long (op_disp))
 	      {
 		/* If this operand is at most 32 bits, convert
@@ -6003,11 +5998,10 @@ optimize_disp (void)
 	    if (flag_code == CODE_64BIT && fits_in_signed_long (op_disp))
 	      {
 		i.types[op].bitfield.disp64 = 0;
-		i.types[op].bitfield.disp32s = 1;
+		i.types[op].bitfield.disp32 = 1;
 	      }
 #endif
 	    if ((i.types[op].bitfield.disp32
-		 || i.types[op].bitfield.disp32s
 		 || i.types[op].bitfield.disp16)
 		&& fits_in_disp8 (op_disp))
 	      i.types[op].bitfield.disp8 = 1;
@@ -6673,8 +6667,8 @@ match_template (char mnem_suffix)
 
 	      addr_prefix_disp = j;
 
-	      /* Address size prefix will turn Disp64/Disp32S/Disp32/Disp16
-		 operand into Disp32/Disp32/Disp16/Disp32 operand.  */
+	      /* Address size prefix will turn Disp64 operand into Disp32 and
+		 Disp32/Disp16 one into Disp16/Disp32 respectively.  */
 	      switch (flag_code)
 		{
 		case CODE_16BIT:
@@ -6687,17 +6681,15 @@ match_template (char mnem_suffix)
 		      operand_types[j].bitfield.disp16 = override;
 		      operand_types[j].bitfield.disp32 = !override;
 		    }
-		  operand_types[j].bitfield.disp32s = 0;
-		  operand_types[j].bitfield.disp64 = 0;
+		  gas_assert (!operand_types[j].bitfield.disp64);
 		  break;
 
 		case CODE_64BIT:
-		  if (operand_types[j].bitfield.disp32s
-		      || operand_types[j].bitfield.disp64)
+		  if (operand_types[j].bitfield.disp64)
 		    {
-		      operand_types[j].bitfield.disp64 &= !override;
-		      operand_types[j].bitfield.disp32s &= !override;
+		      gas_assert (!operand_types[j].bitfield.disp32);
 		      operand_types[j].bitfield.disp32 = override;
+		      operand_types[j].bitfield.disp64 = !override;
 		    }
 		  operand_types[j].bitfield.disp16 = 0;
 		  break;
@@ -8378,10 +8370,7 @@ build_modrm_byte (void)
 		  i.sib.base = NO_BASE_REGISTER;
 		  i.sib.scale = i.log2_scale_factor;
 		  i.types[op] = operand_type_and_not (i.types[op], anydisp);
-		  if (want_disp32 (&i.tm))
-		    i.types[op].bitfield.disp32 = 1;
-		  else
-		    i.types[op].bitfield.disp32s = 1;
+		  i.types[op].bitfield.disp32 = 1;
 		}
 
 	      /* Since the mandatory SIB always has index register, so
@@ -8421,10 +8410,7 @@ build_modrm_byte (void)
 		      i.rm.regmem = ESCAPE_TO_TWO_BYTE_ADDRESSING;
 		      i.sib.base = NO_BASE_REGISTER;
 		      i.sib.index = NO_INDEX_REGISTER;
-		      if (want_disp32 (&i.tm))
-			i.types[op].bitfield.disp32 = 1;
-		      else
-			i.types[op].bitfield.disp32s = 1;
+		      i.types[op].bitfield.disp32 = 1;
 		    }
 		  else if ((flag_code == CODE_16BIT)
 			   ^ (i.prefix[ADDR_PREFIX] != 0))
@@ -8449,10 +8435,7 @@ build_modrm_byte (void)
 		  i.sib.scale = i.log2_scale_factor;
 		  i.rm.regmem = ESCAPE_TO_TWO_BYTE_ADDRESSING;
 		  i.types[op] = operand_type_and_not (i.types[op], anydisp);
-		  if (want_disp32 (&i.tm))
-		    i.types[op].bitfield.disp32 = 1;
-		  else
-		    i.types[op].bitfield.disp32s = 1;
+		  i.types[op].bitfield.disp32 = 1;
 		  if ((i.index_reg->reg_flags & RegRex) != 0)
 		    i.rex |= REX_X;
 		}
@@ -8464,8 +8447,7 @@ build_modrm_byte (void)
 	      i.rm.regmem = NO_BASE_REGISTER;
 	      i.types[op].bitfield.disp8 = 0;
 	      i.types[op].bitfield.disp16 = 0;
-	      i.types[op].bitfield.disp32 = 0;
-	      i.types[op].bitfield.disp32s = 1;
+	      i.types[op].bitfield.disp32 = 1;
 	      i.types[op].bitfield.disp64 = 0;
 	      i.flags[op] |= Operand_PCrel;
 	      if (! i.disp_operands)
@@ -8521,16 +8503,7 @@ build_modrm_byte (void)
 		{
 		  i.types[op].bitfield.disp16 = 0;
 		  i.types[op].bitfield.disp64 = 0;
-		  if (!want_disp32 (&i.tm))
-		    {
-		      i.types[op].bitfield.disp32 = 0;
-		      i.types[op].bitfield.disp32s = 1;
-		    }
-		  else
-		    {
-		      i.types[op].bitfield.disp32 = 1;
-		      i.types[op].bitfield.disp32s = 0;
-		    }
+		  i.types[op].bitfield.disp32 = 1;
 		}
 
 	      if (!i.tm.opcode_modifier.sib)
@@ -8799,7 +8772,6 @@ flip_code16 (unsigned int code16)
 
   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;
 }
@@ -10067,8 +10039,12 @@ output_disp (fragS *insn_start_frag, off
 	  else
 	    {
 	      enum bfd_reloc_code_real reloc_type;
-	      int sign = i.types[n].bitfield.disp32s;
-	      int pcrel = (i.flags[n] & Operand_PCrel) != 0;
+	      bool pcrel = (i.flags[n] & Operand_PCrel) != 0;
+	      bool sign = (flag_code == CODE_64BIT && size == 4
+			   && (!want_disp32 (&i.tm)
+			       || (i.tm.opcode_modifier.jump && !i.jumpabsolute
+				   && !i.types[n].bitfield.baseindex)))
+			  || pcrel;
 	      fixS *fixP;
 
 	      /* We can't have 8 bit displacement here.  */
@@ -10380,7 +10356,7 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_IMM64, true },
     { STRING_COMMA_LEN ("PLT"),      { BFD_RELOC_386_PLT32,
 				       BFD_RELOC_X86_64_PLT32    },
-      OPERAND_TYPE_IMM32_32S_DISP32S, false },
+      OPERAND_TYPE_IMM32_32S_DISP32, false },
     { STRING_COMMA_LEN ("GOTPLT"),   { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_GOTPLT64 },
       OPERAND_TYPE_IMM64_DISP64, true },
@@ -10389,28 +10365,28 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_IMM64_DISP64, true },
     { STRING_COMMA_LEN ("GOTPCREL"), { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_GOTPCREL },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
     { STRING_COMMA_LEN ("TLSGD"),    { BFD_RELOC_386_TLS_GD,
 				       BFD_RELOC_X86_64_TLSGD    },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
     { STRING_COMMA_LEN ("TLSLDM"),   { BFD_RELOC_386_TLS_LDM,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("TLSLD"),    { _dummy_first_bfd_reloc_code_real,
 				       BFD_RELOC_X86_64_TLSLD    },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
     { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32,
 				       BFD_RELOC_X86_64_GOTTPOFF },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
     { STRING_COMMA_LEN ("TPOFF"),    { BFD_RELOC_386_TLS_LE_32,
 				       BFD_RELOC_X86_64_TPOFF32  },
-      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
     { STRING_COMMA_LEN ("NTPOFF"),   { BFD_RELOC_386_TLS_LE,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("DTPOFF"),   { BFD_RELOC_386_TLS_LDO_32,
 				       BFD_RELOC_X86_64_DTPOFF32 },
-      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
     { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE,
 				       _dummy_first_bfd_reloc_code_real },
       OPERAND_TYPE_NONE, true },
@@ -10419,17 +10395,17 @@ lex_got (enum bfd_reloc_code_real *rel,
       OPERAND_TYPE_NONE, true },
     { STRING_COMMA_LEN ("GOT"),      { BFD_RELOC_386_GOT32,
 				       BFD_RELOC_X86_64_GOT32    },
-      OPERAND_TYPE_IMM32_32S_64_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_64_DISP32, true },
     { STRING_COMMA_LEN ("TLSDESC"),  { BFD_RELOC_386_TLS_GOTDESC,
 				       BFD_RELOC_X86_64_GOTPC32_TLSDESC },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
     { STRING_COMMA_LEN ("TLSCALL"),  { BFD_RELOC_386_TLS_DESC_CALL,
 				       BFD_RELOC_X86_64_TLSDESC_CALL },
-      OPERAND_TYPE_IMM32_32S_DISP32S, true },
+      OPERAND_TYPE_IMM32_32S_DISP32, true },
 #else /* TE_PE */
     { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL,
 				       BFD_RELOC_32_SECREL },
-      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, false },
+      OPERAND_TYPE_IMM32_32S_64_DISP32_64, false },
 #endif
   };
   char *cp;
@@ -10997,13 +10973,9 @@ i386_displacement (char *disp_start, cha
       override = (i.prefix[ADDR_PREFIX] != 0);
       if (flag_code == CODE_64BIT)
 	{
+	  bigdisp.bitfield.disp32 = 1;
 	  if (!override)
-	    {
-	      bigdisp.bitfield.disp32s = 1;
-	      bigdisp.bitfield.disp64 = 1;
-	    }
-	  else
-	    bigdisp.bitfield.disp32 = 1;
+	    bigdisp.bitfield.disp64 = 1;
 	}
       else if ((flag_code == CODE_16BIT) ^ override)
 	  bigdisp.bitfield.disp16 = 1;
@@ -11042,7 +11014,7 @@ i386_displacement (char *disp_start, cha
 	      && (!intel64 || !has_intel64))
 	    bigdisp.bitfield.disp16 = 1;
 	  else
-	    bigdisp.bitfield.disp32s = 1;
+	    bigdisp.bitfield.disp32 = 1;
 	}
       else
 	{
--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -963,7 +963,6 @@ i386_intel_operand (char *operand_string
 		      i.flags[0] &= ~Operand_Mem;
 		      i.types[0].bitfield.disp16 = 0;
 		      i.types[0].bitfield.disp32 = 0;
-		      i.types[0].bitfield.disp32s = 0;
 		      return 1;
 		    }
 		}
@@ -1011,13 +1010,9 @@ i386_intel_operand (char *operand_string
 
 	  if (flag_code == CODE_64BIT)
 	    {
+	      i.types[this_operand].bitfield.disp32 = 1;
 	      if (!i.prefix[ADDR_PREFIX])
-		{
-		  i.types[this_operand].bitfield.disp64 = 1;
-		  i.types[this_operand].bitfield.disp32s = 1;
-		}
-	      else
-		i.types[this_operand].bitfield.disp32 = 1;
+		i.types[this_operand].bitfield.disp64 = 1;
 	    }
 	  else if (!i.prefix[ADDR_PREFIX] ^ (flag_code == CODE_16BIT))
 	    i.types[this_operand].bitfield.disp32 = 1;
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -475,8 +475,6 @@ static initializer operand_type_init[] =
     "Disp16" },
   { "OPERAND_TYPE_DISP32",
     "Disp32" },
-  { "OPERAND_TYPE_DISP32S",
-    "Disp32S" },
   { "OPERAND_TYPE_DISP64",
     "Disp64" },
   { "OPERAND_TYPE_INOUTPORTREG",
@@ -520,7 +518,7 @@ static initializer operand_type_init[] =
   { "OPERAND_TYPE_DISP16_32",
     "Disp16|Disp32" },
   { "OPERAND_TYPE_ANYDISP",
-    "Disp8|Disp16|Disp32|Disp32S|Disp64" },
+    "Disp8|Disp16|Disp32|Disp64" },
   { "OPERAND_TYPE_IMM16_32",
     "Imm16|Imm32" },
   { "OPERAND_TYPE_IMM16_32S",
@@ -529,14 +527,14 @@ static initializer operand_type_init[] =
     "Imm16|Imm32|Imm32S" },
   { "OPERAND_TYPE_IMM32_64",
     "Imm32|Imm64" },
-  { "OPERAND_TYPE_IMM32_32S_DISP32S",
-    "Imm32|Imm32S|Disp32S" },
+  { "OPERAND_TYPE_IMM32_32S_DISP32",
+    "Imm32|Imm32S|Disp32" },
   { "OPERAND_TYPE_IMM64_DISP64",
     "Imm64|Disp64" },
-  { "OPERAND_TYPE_IMM32_32S_64_DISP32S",
-    "Imm32|Imm32S|Imm64|Disp32S" },
-  { "OPERAND_TYPE_IMM32_32S_64_DISP32S_64",
-    "Imm32|Imm32S|Imm64|Disp32S|Disp64" },
+  { "OPERAND_TYPE_IMM32_32S_64_DISP32",
+    "Imm32|Imm32S|Imm64|Disp32" },
+  { "OPERAND_TYPE_IMM32_32S_64_DISP32_64",
+    "Imm32|Imm32S|Imm64|Disp32|Disp64" },
   { "OPERAND_TYPE_ANYIMM",
     "Imm1|Imm8|Imm8S|Imm16|Imm32|Imm32S|Imm64" },
 };
@@ -786,7 +784,6 @@ static bitfield operand_types[] =
   BITFIELD (Disp8),
   BITFIELD (Disp16),
   BITFIELD (Disp32),
-  BITFIELD (Disp32S),
   BITFIELD (Disp64),
   BITFIELD (Byte),
   BITFIELD (Word),
@@ -1343,10 +1340,7 @@ process_i386_operand_type (FILE *table,
 	  if (!active_cpu_flags.bitfield.cpu64
 	      && !active_cpu_flags.bitfield.cpumpx)
 	    set_bitfield("Disp16", types, 1, ARRAY_SIZE (types), lineno);
-	  if (!active_cpu_flags.bitfield.cpu64)
-	    set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
-	  if (!active_cpu_flags.bitfield.cpuno64)
-	    set_bitfield("Disp32S", types, 1, ARRAY_SIZE (types), lineno);
+	  set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
 	}
     }
   output_operand_type (table, class, instance, types, ARRAY_SIZE (types),
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -832,10 +832,8 @@ enum
   Disp8,
   /* 16 bit displacement */
   Disp16,
-  /* 32 bit displacement */
+  /* 32 bit displacement (64-bit: sign-extended) */
   Disp32,
-  /* 32 bit signed displacement */
-  Disp32S,
   /* 64 bit displacement */
   Disp64,
   /* Register which can be used for base or index in memory operand.  */
@@ -892,7 +890,6 @@ typedef union i386_operand_type
       unsigned int disp8:1;
       unsigned int disp16:1;
       unsigned int disp32:1;
-      unsigned int disp32s:1;
       unsigned int disp64:1;
       unsigned int baseindex:1;
       unsigned int byte:1;
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -127,7 +127,8 @@
 ### MARKER ###
 
 // Move instructions.
-mov, 0xa0, None, 0, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
+mov, 0xa0, None, CpuNo64, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
+mov, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
 movabs, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
 movq, 0xa1, None, Cpu64, D|Size64|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp64|Unspecified|Qword, Acc|Qword }
 mov, 0x88, None, 0, D|W|CheckRegSize|Modrm|No_sSuf|No_ldSuf|HLEPrefixRelease, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
@@ -412,8 +413,8 @@ shrd, 0xfad, None, Cpu386, Modrm|CheckRe
 
 // Control transfer instructions.
 call, 0xe8, None, CpuNo64, JumpDword|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp16|Disp32 }
-call, 0xe8, None, Cpu64, Amd64|JumpDword|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp16|Disp32S }
-call, 0xe8, None, Cpu64, Intel64|JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp32S }
+call, 0xe8, None, Cpu64, Amd64|JumpDword|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp16|Disp32 }
+call, 0xe8, None, Cpu64, Intel64|JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp32 }
 call, 0xff, 2, CpuNo64, Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg32|Unspecified|BaseIndex }
 call, 0xff, 2, Cpu64, Amd64|Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg64|Unspecified|BaseIndex }
 call, 0xff, 2, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg64|Unspecified|BaseIndex }
@@ -425,8 +426,8 @@ lcall, 0x9a, None, CpuNo64, JumpInterSeg
 lcall, 0xff, 3, 0, Amd64|Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex }
 lcall, 0xff, 3, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_sSuf|No_ldSuf, { Unspecified|BaseIndex }
 
-jmp, 0xeb, None, 0, Amd64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32|Disp32S }
-jmp, 0xeb, None, Cpu64, Intel64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp32S }
+jmp, 0xeb, None, 0, Amd64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32 }
+jmp, 0xeb, None, Cpu64, Intel64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp32 }
 jmp, 0xff, 4, CpuNo64, Modrm|JumpAbsolute|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg32|Unspecified|BaseIndex }
 jmp, 0xff, 4, Cpu64, Amd64|Modrm|JumpAbsolute|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg64|Unspecified|BaseIndex }
 jmp, 0xff, 4, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg64|Unspecified|BaseIndex }
@@ -459,7 +460,7 @@ leave, 0xc9, None, Cpu64, DefaultSize|No
          s:8, ns:9, p:a, pe:a, np:b, po:b, l:c, nge:c, nl:d, ge:d, le:e, ng:e, nle:f, g:f>
 
 // Conditional jumps.
-j<cc>, 0x7<cc:opc>, None, 0, Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32|Disp32S }
+j<cc>, 0x7<cc:opc>, None, 0, Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32 }
 
 // jcxz vs. jecxz is chosen on the basis of the address size prefix.
 jcxz, 0xe3, None, CpuNo64, JumpByte|Size16|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
@@ -1911,7 +1912,7 @@ xrelease, 0xf3, None, CpuHLE, No_bSuf|No
 
 // RTM instructions
 xabort, 0xc6f8, None, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8 }
-xbegin, 0xc7f8, None, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Disp32S }
+xbegin, 0xc7f8, None, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32 }
 xend, 0xf01d5, None, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
 xtest, 0xf01d6, None, CpuHLE|CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] x86-64: improve handling of branches to absolute addresses
  2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
@ 2022-06-30 17:50   ` H.J. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2022-06-30 17:50 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Jun 30, 2022 at 5:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> There are two related problems here: The use of "addr32" on a direct
> branch would, besides causing a warning, result in operands to be
> permitted which mistakenly are refused without "addr32". Plus at some
> point not too long ago I'm afraid it may have been me who regressed the
> relocation addends emitted for such branches. Correct both problems,
> adding a testcase to guard against regressing this again.
> ---
> In principle things like "JECXZ <absolute>" should be permitted as well,
> I think. Whether the destination is within reach can only be determined
> by the linker. But that likely being a more intrusive change, I guess we
> can leave this as is until someone really needs it to work.
>
> If "ELF: emit symbol table when there are relocations" (submitted
> earlier) goes in before this change, I'd be inclined to drop the label
> again from the new testcase. The original lack of a label there was how
> I noticed that other issue, so the testcase here could at once serve to
> test that changed behavior as well.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -4975,7 +4975,9 @@ md_assemble (char *line)
>    if (i.imm_operands)
>      optimize_imm ();
>
> -  if (i.disp_operands && !want_disp32 (current_templates->start))
> +  if (i.disp_operands && !want_disp32 (current_templates->start)
> +      && (!current_templates->start->opcode_modifier.jump
> +         || i.jumpabsolute || i.types[0].bitfield.baseindex))
>      {
>        for (j = 0; j < i.operands; ++j)
>         {
> @@ -5985,7 +5987,9 @@ optimize_disp (void)
>             /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
>             if ((i.types[op].bitfield.disp32
>                  || (flag_code == CODE_64BIT
> -                    && want_disp32 (current_templates->start)))
> +                    && want_disp32 (current_templates->start)
> +                    && (!current_templates->start->opcode_modifier.jump
> +                        || i.jumpabsolute || i.types[op].bitfield.baseindex)))
>                 && fits_in_unsigned_long (op_disp))
>               {
>                 /* If this operand is at most 32 bits, convert
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -1314,6 +1314,7 @@ if [gas_64_check] then {
>         run_dump_test "x86-64-branch-3"
>         run_list_test "x86-64-branch-4" "-al -mintel64"
>         run_list_test "x86-64-branch-5" "-al"
> +       run_dump_test "x86-64-branch-6"
>
>         run_dump_test "x86-64-rip-2"
>
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.d
> @@ -0,0 +1,21 @@
> +#objdump: -r
> +#name: x86-64 branch 6
> +#warning_output: x86-64-branch-6.e
> +
> +.*: +file format .*
> +
> +RELOCATION RECORDS FOR \[\.text\]:
> +OFFSET +TYPE +VALUE *
> +0+01 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+11 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+21 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+31 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+07 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+0c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+17 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+1c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+27 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+2c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+37 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+3c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.s
> @@ -0,0 +1,18 @@
> +       .text
> +
> +branch_6:
> +       call    0x87654321
> +       je      0x87654321
> +       jmp     0x87654321
> +
> +       call    0x876543210
> +       je      0x876543210
> +       jmp     0x876543210
> +
> +       addr32 call     0x87654321
> +       addr32 je       0x87654321
> +       addr32 jmp      0x87654321
> +
> +       addr32 call     0x876543210
> +       addr32 je       0x876543210
> +       addr32 jmp      0x876543210
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.e
> @@ -0,0 +1,7 @@
> +.*: Assembler messages:
> +.*:12: Warning: skipping prefixes on `call'
> +.*:13: Warning: skipping prefixes on `je'
> +.*:14: Warning: skipping prefixes on `jmp'
> +.*:16: Warning: skipping prefixes on `call'
> +.*:17: Warning: skipping prefixes on `je'
> +.*:18: Warning: skipping prefixes on `jmp'
>

OK.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] x86: restore masking of displacement kinds
  2022-06-30 12:08 ` [PATCH 2/3] x86: restore masking of displacement kinds Jan Beulich
@ 2022-06-30 22:47   ` H.J. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2022-06-30 22:47 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Jun 30, 2022 at 5:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Commit 7d5e4556a375 rendered the check near the end of what is now
> i386_finalize_displacement() entirely dead for AT&T mode, since for
> operands involving a displacement .unspecified will always be set. But
> the logic there is bogus anyway - Intel syntax operand size specifiers
> are of no interest there either. The only thing which matters in the
> "displacement only" determination is .baseindex.
>
> Of course when masking displacement kinds we should not at the same time
> also mask off other attributes.
>
> Furthermore the type mask returned by lex_got() also needs to be
> adjusted: The only case where we want Disp32 (rather than Disp32S) is
> when dealing with 32-bit addressing mode in 64-bit code.
> ---
> Actually I don't understand why this masking has been dependent on
> "displacement only". I would expect the masking to similarly apply to
> displacements used with any kind of base/index expression.

It has been there for a long time.  Before I added the comment, there was

if (!(i.types[this_operand] & ~Disp))
    i.types[this_operand] &= types;

I guess if the operand is displacement only, we don't change it.

> I was considering to drop the OPERAND_TYPE_IMM32_32S_* that are being
> changed, in favor of using C99 dedicated initializers. But perhaps this
> would better be a separate change, dealing with the other similar
> constants as well.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -10380,7 +10380,7 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_IMM64, true },
>      { STRING_COMMA_LEN ("PLT"),      { BFD_RELOC_386_PLT32,
>                                        BFD_RELOC_X86_64_PLT32    },
> -      OPERAND_TYPE_IMM32_32S_DISP32, false },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, false },
>      { STRING_COMMA_LEN ("GOTPLT"),   { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_GOTPLT64 },
>        OPERAND_TYPE_IMM64_DISP64, true },
> @@ -10389,28 +10389,28 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_IMM64_DISP64, true },
>      { STRING_COMMA_LEN ("GOTPCREL"), { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_GOTPCREL },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>      { STRING_COMMA_LEN ("TLSGD"),    { BFD_RELOC_386_TLS_GD,
>                                        BFD_RELOC_X86_64_TLSGD    },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>      { STRING_COMMA_LEN ("TLSLDM"),   { BFD_RELOC_386_TLS_LDM,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("TLSLD"),    { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_TLSLD    },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>      { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32,
>                                        BFD_RELOC_X86_64_GOTTPOFF },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>      { STRING_COMMA_LEN ("TPOFF"),    { BFD_RELOC_386_TLS_LE_32,
>                                        BFD_RELOC_X86_64_TPOFF32  },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
>      { STRING_COMMA_LEN ("NTPOFF"),   { BFD_RELOC_386_TLS_LE,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("DTPOFF"),   { BFD_RELOC_386_TLS_LDO_32,
>                                        BFD_RELOC_X86_64_DTPOFF32 },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
>      { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
> @@ -10419,17 +10419,17 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("GOT"),      { BFD_RELOC_386_GOT32,
>                                        BFD_RELOC_X86_64_GOT32    },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32S, true },
>      { STRING_COMMA_LEN ("TLSDESC"),  { BFD_RELOC_386_TLS_GOTDESC,
>                                        BFD_RELOC_X86_64_GOTPC32_TLSDESC },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>      { STRING_COMMA_LEN ("TLSCALL"),  { BFD_RELOC_386_TLS_DESC_CALL,
>                                        BFD_RELOC_X86_64_TLSDESC_CALL },
> -      OPERAND_TYPE_IMM32_32S_DISP32, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32S, true },
>  #else /* TE_PE */
>      { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL,
>                                        BFD_RELOC_32_SECREL },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32_64, false },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, false },
>  #endif
>    };
>    char *cp;
> @@ -11144,7 +11144,6 @@ static int
>  i386_finalize_displacement (segT exp_seg ATTRIBUTE_UNUSED, expressionS *exp,
>                             i386_operand_type types, const char *disp_start)
>  {
> -  i386_operand_type bigdisp;
>    int ret = 1;
>
>    /* We do this to make sure that the section symbol is in
> @@ -11210,10 +11209,10 @@ i386_finalize_displacement (segT exp_seg
>      i.types[this_operand].bitfield.disp8 = 1;
>
>    /* Check if this is a displacement only operand.  */
> -  bigdisp = operand_type_and_not (i.types[this_operand], anydisp);
> -  if (operand_type_all_zero (&bigdisp))
> -    i.types[this_operand] = operand_type_and (i.types[this_operand],
> -                                             types);
> +  if (!i.types[this_operand].bitfield.baseindex)
> +    i.types[this_operand] =
> +      operand_type_or (operand_type_and_not (i.types[this_operand], anydisp),
> +                      operand_type_and (i.types[this_operand], types));
>
>    return ret;
>  }
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -529,14 +529,14 @@ static initializer operand_type_init[] =
>      "Imm16|Imm32|Imm32S" },
>    { "OPERAND_TYPE_IMM32_64",
>      "Imm32|Imm64" },
> -  { "OPERAND_TYPE_IMM32_32S_DISP32",
> -    "Imm32|Imm32S|Disp32" },
> +  { "OPERAND_TYPE_IMM32_32S_DISP32S",
> +    "Imm32|Imm32S|Disp32S" },
>    { "OPERAND_TYPE_IMM64_DISP64",
>      "Imm64|Disp64" },
> -  { "OPERAND_TYPE_IMM32_32S_64_DISP32",
> -    "Imm32|Imm32S|Imm64|Disp32" },
> -  { "OPERAND_TYPE_IMM32_32S_64_DISP32_64",
> -    "Imm32|Imm32S|Imm64|Disp32|Disp64" },
> +  { "OPERAND_TYPE_IMM32_32S_64_DISP32S",
> +    "Imm32|Imm32S|Imm64|Disp32S" },
> +  { "OPERAND_TYPE_IMM32_32S_64_DISP32S_64",
> +    "Imm32|Imm32S|Imm64|Disp32S|Disp64" },
>    { "OPERAND_TYPE_ANYIMM",
>      "Imm1|Imm8|Imm8S|Imm16|Imm32|Imm32S|Imm64" },
>  };
>

OK.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] x86: fold Disp32S and Disp32
  2022-06-30 12:10 ` [PATCH 3/3] x86: fold Disp32S and Disp32 Jan Beulich
@ 2022-06-30 22:51   ` H.J. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2022-06-30 22:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Jun 30, 2022 at 5:10 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> The only case where 64-bit code uses non-sign-extended (can also be
> considered zero-extended) displacements is when an address size override
> is in place for a memory operand (i.e. particularly excluding
> displacements of direct branches, which - if at all - are controlled by
> operand size, and then are still sign-extended, just from 16 bits).
> Hence the distinction in templates is unnecessary, allowing code to be
> simplified in a number of places. The only place where logic becomes
> more complicated is when signed-ness of relocations is determined in
> output_disp().
>
> The other caveat is that Disp64 cannot be specified anymore in an insn
> template at the same time as Disp32. Unlike for non-64-bit mode,
> templates don't specify displacements for both possible addressing
> modes; the necessary adjustment to the expected ones has already been
> done in match_template() anyway (but of course the logic there needs
> tweaking now). Hence the single template so far doing so is split.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -2142,14 +2142,12 @@ operand_type_check (i386_operand_type t,
>        return (t.bitfield.disp8
>               || t.bitfield.disp16
>               || t.bitfield.disp32
> -             || t.bitfield.disp32s
>               || t.bitfield.disp64);
>
>      case anymem:
>        return (t.bitfield.disp8
>               || t.bitfield.disp16
>               || t.bitfield.disp32
> -             || t.bitfield.disp32s
>               || t.bitfield.disp64
>               || t.bitfield.baseindex);
>
> @@ -2414,8 +2412,7 @@ mode_from_disp_size (i386_operand_type t
>    if (t.bitfield.disp8)
>      return 1;
>    else if (t.bitfield.disp16
> -          || t.bitfield.disp32
> -          || t.bitfield.disp32s)
> +          || t.bitfield.disp32)
>      return 2;
>    else
>      return 0;
> @@ -3326,7 +3323,6 @@ const type_names[] =
>    { OPERAND_TYPE_DISP8, "d8" },
>    { OPERAND_TYPE_DISP16, "d16" },
>    { OPERAND_TYPE_DISP32, "d32" },
> -  { OPERAND_TYPE_DISP32S, "d32s" },
>    { OPERAND_TYPE_DISP64, "d64" },
>    { OPERAND_TYPE_INOUTPORTREG, "InOutPortReg" },
>    { OPERAND_TYPE_SHIFTCOUNT, "ShiftCount" },
> @@ -4990,12 +4986,11 @@ md_assemble (char *line)
>             continue;
>
>           /* Since displacement is signed extended to 64bit, don't allow
> -            disp32 and turn off disp32s if they are out of range.  */
> -         i.types[j].bitfield.disp32 = 0;
> +            disp32 if it is out of range.  */
>           if (fits_in_signed_long (exp->X_add_number))
>             continue;
>
> -         i.types[j].bitfield.disp32s = 0;
> +         i.types[j].bitfield.disp32 = 0;
>           if (i.types[j].bitfield.baseindex)
>             {
>               char number_buf[128];
> @@ -5985,11 +5980,11 @@ optimize_disp (void)
>
>  #ifdef BFD64
>             /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
> -           if ((i.types[op].bitfield.disp32
> -                || (flag_code == CODE_64BIT
> -                    && want_disp32 (current_templates->start)
> -                    && (!current_templates->start->opcode_modifier.jump
> -                        || i.jumpabsolute || i.types[op].bitfield.baseindex)))
> +           if ((flag_code != CODE_64BIT
> +                ? i.types[op].bitfield.disp32
> +                : want_disp32 (current_templates->start)
> +                  && (!current_templates->start->opcode_modifier.jump
> +                      || i.jumpabsolute || i.types[op].bitfield.baseindex))
>                 && fits_in_unsigned_long (op_disp))
>               {
>                 /* If this operand is at most 32 bits, convert
> @@ -6003,11 +5998,10 @@ optimize_disp (void)
>             if (flag_code == CODE_64BIT && fits_in_signed_long (op_disp))
>               {
>                 i.types[op].bitfield.disp64 = 0;
> -               i.types[op].bitfield.disp32s = 1;
> +               i.types[op].bitfield.disp32 = 1;
>               }
>  #endif
>             if ((i.types[op].bitfield.disp32
> -                || i.types[op].bitfield.disp32s
>                  || i.types[op].bitfield.disp16)
>                 && fits_in_disp8 (op_disp))
>               i.types[op].bitfield.disp8 = 1;
> @@ -6673,8 +6667,8 @@ match_template (char mnem_suffix)
>
>               addr_prefix_disp = j;
>
> -             /* Address size prefix will turn Disp64/Disp32S/Disp32/Disp16
> -                operand into Disp32/Disp32/Disp16/Disp32 operand.  */
> +             /* Address size prefix will turn Disp64 operand into Disp32 and
> +                Disp32/Disp16 one into Disp16/Disp32 respectively.  */
>               switch (flag_code)
>                 {
>                 case CODE_16BIT:
> @@ -6687,17 +6681,15 @@ match_template (char mnem_suffix)
>                       operand_types[j].bitfield.disp16 = override;
>                       operand_types[j].bitfield.disp32 = !override;
>                     }
> -                 operand_types[j].bitfield.disp32s = 0;
> -                 operand_types[j].bitfield.disp64 = 0;
> +                 gas_assert (!operand_types[j].bitfield.disp64);
>                   break;
>
>                 case CODE_64BIT:
> -                 if (operand_types[j].bitfield.disp32s
> -                     || operand_types[j].bitfield.disp64)
> +                 if (operand_types[j].bitfield.disp64)
>                     {
> -                     operand_types[j].bitfield.disp64 &= !override;
> -                     operand_types[j].bitfield.disp32s &= !override;
> +                     gas_assert (!operand_types[j].bitfield.disp32);
>                       operand_types[j].bitfield.disp32 = override;
> +                     operand_types[j].bitfield.disp64 = !override;
>                     }
>                   operand_types[j].bitfield.disp16 = 0;
>                   break;
> @@ -8378,10 +8370,7 @@ build_modrm_byte (void)
>                   i.sib.base = NO_BASE_REGISTER;
>                   i.sib.scale = i.log2_scale_factor;
>                   i.types[op] = operand_type_and_not (i.types[op], anydisp);
> -                 if (want_disp32 (&i.tm))
> -                   i.types[op].bitfield.disp32 = 1;
> -                 else
> -                   i.types[op].bitfield.disp32s = 1;
> +                 i.types[op].bitfield.disp32 = 1;
>                 }
>
>               /* Since the mandatory SIB always has index register, so
> @@ -8421,10 +8410,7 @@ build_modrm_byte (void)
>                       i.rm.regmem = ESCAPE_TO_TWO_BYTE_ADDRESSING;
>                       i.sib.base = NO_BASE_REGISTER;
>                       i.sib.index = NO_INDEX_REGISTER;
> -                     if (want_disp32 (&i.tm))
> -                       i.types[op].bitfield.disp32 = 1;
> -                     else
> -                       i.types[op].bitfield.disp32s = 1;
> +                     i.types[op].bitfield.disp32 = 1;
>                     }
>                   else if ((flag_code == CODE_16BIT)
>                            ^ (i.prefix[ADDR_PREFIX] != 0))
> @@ -8449,10 +8435,7 @@ build_modrm_byte (void)
>                   i.sib.scale = i.log2_scale_factor;
>                   i.rm.regmem = ESCAPE_TO_TWO_BYTE_ADDRESSING;
>                   i.types[op] = operand_type_and_not (i.types[op], anydisp);
> -                 if (want_disp32 (&i.tm))
> -                   i.types[op].bitfield.disp32 = 1;
> -                 else
> -                   i.types[op].bitfield.disp32s = 1;
> +                 i.types[op].bitfield.disp32 = 1;
>                   if ((i.index_reg->reg_flags & RegRex) != 0)
>                     i.rex |= REX_X;
>                 }
> @@ -8464,8 +8447,7 @@ build_modrm_byte (void)
>               i.rm.regmem = NO_BASE_REGISTER;
>               i.types[op].bitfield.disp8 = 0;
>               i.types[op].bitfield.disp16 = 0;
> -             i.types[op].bitfield.disp32 = 0;
> -             i.types[op].bitfield.disp32s = 1;
> +             i.types[op].bitfield.disp32 = 1;
>               i.types[op].bitfield.disp64 = 0;
>               i.flags[op] |= Operand_PCrel;
>               if (! i.disp_operands)
> @@ -8521,16 +8503,7 @@ build_modrm_byte (void)
>                 {
>                   i.types[op].bitfield.disp16 = 0;
>                   i.types[op].bitfield.disp64 = 0;
> -                 if (!want_disp32 (&i.tm))
> -                   {
> -                     i.types[op].bitfield.disp32 = 0;
> -                     i.types[op].bitfield.disp32s = 1;
> -                   }
> -                 else
> -                   {
> -                     i.types[op].bitfield.disp32 = 1;
> -                     i.types[op].bitfield.disp32s = 0;
> -                   }
> +                 i.types[op].bitfield.disp32 = 1;
>                 }
>
>               if (!i.tm.opcode_modifier.sib)
> @@ -8799,7 +8772,6 @@ flip_code16 (unsigned int code16)
>
>    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;
>  }
> @@ -10067,8 +10039,12 @@ output_disp (fragS *insn_start_frag, off
>           else
>             {
>               enum bfd_reloc_code_real reloc_type;
> -             int sign = i.types[n].bitfield.disp32s;
> -             int pcrel = (i.flags[n] & Operand_PCrel) != 0;
> +             bool pcrel = (i.flags[n] & Operand_PCrel) != 0;
> +             bool sign = (flag_code == CODE_64BIT && size == 4
> +                          && (!want_disp32 (&i.tm)
> +                              || (i.tm.opcode_modifier.jump && !i.jumpabsolute
> +                                  && !i.types[n].bitfield.baseindex)))
> +                         || pcrel;
>               fixS *fixP;
>
>               /* We can't have 8 bit displacement here.  */
> @@ -10380,7 +10356,7 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_IMM64, true },
>      { STRING_COMMA_LEN ("PLT"),      { BFD_RELOC_386_PLT32,
>                                        BFD_RELOC_X86_64_PLT32    },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, false },
> +      OPERAND_TYPE_IMM32_32S_DISP32, false },
>      { STRING_COMMA_LEN ("GOTPLT"),   { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_GOTPLT64 },
>        OPERAND_TYPE_IMM64_DISP64, true },
> @@ -10389,28 +10365,28 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_IMM64_DISP64, true },
>      { STRING_COMMA_LEN ("GOTPCREL"), { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_GOTPCREL },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>      { STRING_COMMA_LEN ("TLSGD"),    { BFD_RELOC_386_TLS_GD,
>                                        BFD_RELOC_X86_64_TLSGD    },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>      { STRING_COMMA_LEN ("TLSLDM"),   { BFD_RELOC_386_TLS_LDM,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("TLSLD"),    { _dummy_first_bfd_reloc_code_real,
>                                        BFD_RELOC_X86_64_TLSLD    },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>      { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32,
>                                        BFD_RELOC_X86_64_GOTTPOFF },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>      { STRING_COMMA_LEN ("TPOFF"),    { BFD_RELOC_386_TLS_LE_32,
>                                        BFD_RELOC_X86_64_TPOFF32  },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
>      { STRING_COMMA_LEN ("NTPOFF"),   { BFD_RELOC_386_TLS_LE,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("DTPOFF"),   { BFD_RELOC_386_TLS_LDO_32,
>                                        BFD_RELOC_X86_64_DTPOFF32 },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
>      { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE,
>                                        _dummy_first_bfd_reloc_code_real },
>        OPERAND_TYPE_NONE, true },
> @@ -10419,17 +10395,17 @@ lex_got (enum bfd_reloc_code_real *rel,
>        OPERAND_TYPE_NONE, true },
>      { STRING_COMMA_LEN ("GOT"),      { BFD_RELOC_386_GOT32,
>                                        BFD_RELOC_X86_64_GOT32    },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32, true },
>      { STRING_COMMA_LEN ("TLSDESC"),  { BFD_RELOC_386_TLS_GOTDESC,
>                                        BFD_RELOC_X86_64_GOTPC32_TLSDESC },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>      { STRING_COMMA_LEN ("TLSCALL"),  { BFD_RELOC_386_TLS_DESC_CALL,
>                                        BFD_RELOC_X86_64_TLSDESC_CALL },
> -      OPERAND_TYPE_IMM32_32S_DISP32S, true },
> +      OPERAND_TYPE_IMM32_32S_DISP32, true },
>  #else /* TE_PE */
>      { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL,
>                                        BFD_RELOC_32_SECREL },
> -      OPERAND_TYPE_IMM32_32S_64_DISP32S_64, false },
> +      OPERAND_TYPE_IMM32_32S_64_DISP32_64, false },
>  #endif
>    };
>    char *cp;
> @@ -10997,13 +10973,9 @@ i386_displacement (char *disp_start, cha
>        override = (i.prefix[ADDR_PREFIX] != 0);
>        if (flag_code == CODE_64BIT)
>         {
> +         bigdisp.bitfield.disp32 = 1;
>           if (!override)
> -           {
> -             bigdisp.bitfield.disp32s = 1;
> -             bigdisp.bitfield.disp64 = 1;
> -           }
> -         else
> -           bigdisp.bitfield.disp32 = 1;
> +           bigdisp.bitfield.disp64 = 1;
>         }
>        else if ((flag_code == CODE_16BIT) ^ override)
>           bigdisp.bitfield.disp16 = 1;
> @@ -11042,7 +11014,7 @@ i386_displacement (char *disp_start, cha
>               && (!intel64 || !has_intel64))
>             bigdisp.bitfield.disp16 = 1;
>           else
> -           bigdisp.bitfield.disp32s = 1;
> +           bigdisp.bitfield.disp32 = 1;
>         }
>        else
>         {
> --- a/gas/config/tc-i386-intel.c
> +++ b/gas/config/tc-i386-intel.c
> @@ -963,7 +963,6 @@ i386_intel_operand (char *operand_string
>                       i.flags[0] &= ~Operand_Mem;
>                       i.types[0].bitfield.disp16 = 0;
>                       i.types[0].bitfield.disp32 = 0;
> -                     i.types[0].bitfield.disp32s = 0;
>                       return 1;
>                     }
>                 }
> @@ -1011,13 +1010,9 @@ i386_intel_operand (char *operand_string
>
>           if (flag_code == CODE_64BIT)
>             {
> +             i.types[this_operand].bitfield.disp32 = 1;
>               if (!i.prefix[ADDR_PREFIX])
> -               {
> -                 i.types[this_operand].bitfield.disp64 = 1;
> -                 i.types[this_operand].bitfield.disp32s = 1;
> -               }
> -             else
> -               i.types[this_operand].bitfield.disp32 = 1;
> +               i.types[this_operand].bitfield.disp64 = 1;
>             }
>           else if (!i.prefix[ADDR_PREFIX] ^ (flag_code == CODE_16BIT))
>             i.types[this_operand].bitfield.disp32 = 1;
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -475,8 +475,6 @@ static initializer operand_type_init[] =
>      "Disp16" },
>    { "OPERAND_TYPE_DISP32",
>      "Disp32" },
> -  { "OPERAND_TYPE_DISP32S",
> -    "Disp32S" },
>    { "OPERAND_TYPE_DISP64",
>      "Disp64" },
>    { "OPERAND_TYPE_INOUTPORTREG",
> @@ -520,7 +518,7 @@ static initializer operand_type_init[] =
>    { "OPERAND_TYPE_DISP16_32",
>      "Disp16|Disp32" },
>    { "OPERAND_TYPE_ANYDISP",
> -    "Disp8|Disp16|Disp32|Disp32S|Disp64" },
> +    "Disp8|Disp16|Disp32|Disp64" },
>    { "OPERAND_TYPE_IMM16_32",
>      "Imm16|Imm32" },
>    { "OPERAND_TYPE_IMM16_32S",
> @@ -529,14 +527,14 @@ static initializer operand_type_init[] =
>      "Imm16|Imm32|Imm32S" },
>    { "OPERAND_TYPE_IMM32_64",
>      "Imm32|Imm64" },
> -  { "OPERAND_TYPE_IMM32_32S_DISP32S",
> -    "Imm32|Imm32S|Disp32S" },
> +  { "OPERAND_TYPE_IMM32_32S_DISP32",
> +    "Imm32|Imm32S|Disp32" },
>    { "OPERAND_TYPE_IMM64_DISP64",
>      "Imm64|Disp64" },
> -  { "OPERAND_TYPE_IMM32_32S_64_DISP32S",
> -    "Imm32|Imm32S|Imm64|Disp32S" },
> -  { "OPERAND_TYPE_IMM32_32S_64_DISP32S_64",
> -    "Imm32|Imm32S|Imm64|Disp32S|Disp64" },
> +  { "OPERAND_TYPE_IMM32_32S_64_DISP32",
> +    "Imm32|Imm32S|Imm64|Disp32" },
> +  { "OPERAND_TYPE_IMM32_32S_64_DISP32_64",
> +    "Imm32|Imm32S|Imm64|Disp32|Disp64" },
>    { "OPERAND_TYPE_ANYIMM",
>      "Imm1|Imm8|Imm8S|Imm16|Imm32|Imm32S|Imm64" },
>  };
> @@ -786,7 +784,6 @@ static bitfield operand_types[] =
>    BITFIELD (Disp8),
>    BITFIELD (Disp16),
>    BITFIELD (Disp32),
> -  BITFIELD (Disp32S),
>    BITFIELD (Disp64),
>    BITFIELD (Byte),
>    BITFIELD (Word),
> @@ -1343,10 +1340,7 @@ process_i386_operand_type (FILE *table,
>           if (!active_cpu_flags.bitfield.cpu64
>               && !active_cpu_flags.bitfield.cpumpx)
>             set_bitfield("Disp16", types, 1, ARRAY_SIZE (types), lineno);
> -         if (!active_cpu_flags.bitfield.cpu64)
> -           set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
> -         if (!active_cpu_flags.bitfield.cpuno64)
> -           set_bitfield("Disp32S", types, 1, ARRAY_SIZE (types), lineno);
> +         set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
>         }
>      }
>    output_operand_type (table, class, instance, types, ARRAY_SIZE (types),
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -832,10 +832,8 @@ enum
>    Disp8,
>    /* 16 bit displacement */
>    Disp16,
> -  /* 32 bit displacement */
> +  /* 32 bit displacement (64-bit: sign-extended) */
>    Disp32,
> -  /* 32 bit signed displacement */
> -  Disp32S,
>    /* 64 bit displacement */
>    Disp64,
>    /* Register which can be used for base or index in memory operand.  */
> @@ -892,7 +890,6 @@ typedef union i386_operand_type
>        unsigned int disp8:1;
>        unsigned int disp16:1;
>        unsigned int disp32:1;
> -      unsigned int disp32s:1;
>        unsigned int disp64:1;
>        unsigned int baseindex:1;
>        unsigned int byte:1;
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -127,7 +127,8 @@
>  ### MARKER ###
>
>  // Move instructions.
> -mov, 0xa0, None, 0, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
> +mov, 0xa0, None, CpuNo64, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
> +mov, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
>  movabs, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
>  movq, 0xa1, None, Cpu64, D|Size64|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp64|Unspecified|Qword, Acc|Qword }
>  mov, 0x88, None, 0, D|W|CheckRegSize|Modrm|No_sSuf|No_ldSuf|HLEPrefixRelease, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
> @@ -412,8 +413,8 @@ shrd, 0xfad, None, Cpu386, Modrm|CheckRe
>
>  // Control transfer instructions.
>  call, 0xe8, None, CpuNo64, JumpDword|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp16|Disp32 }
> -call, 0xe8, None, Cpu64, Amd64|JumpDword|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp16|Disp32S }
> -call, 0xe8, None, Cpu64, Intel64|JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp32S }
> +call, 0xe8, None, Cpu64, Amd64|JumpDword|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp16|Disp32 }
> +call, 0xe8, None, Cpu64, Intel64|JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk, { Disp32 }
>  call, 0xff, 2, CpuNo64, Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg32|Unspecified|BaseIndex }
>  call, 0xff, 2, Cpu64, Amd64|Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg64|Unspecified|BaseIndex }
>  call, 0xff, 2, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg64|Unspecified|BaseIndex }
> @@ -425,8 +426,8 @@ lcall, 0x9a, None, CpuNo64, JumpInterSeg
>  lcall, 0xff, 3, 0, Amd64|Modrm|JumpAbsolute|DefaultSize|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex }
>  lcall, 0xff, 3, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_sSuf|No_ldSuf, { Unspecified|BaseIndex }
>
> -jmp, 0xeb, None, 0, Amd64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32|Disp32S }
> -jmp, 0xeb, None, Cpu64, Intel64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp32S }
> +jmp, 0xeb, None, 0, Amd64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32 }
> +jmp, 0xeb, None, Cpu64, Intel64|Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp32 }
>  jmp, 0xff, 4, CpuNo64, Modrm|JumpAbsolute|No_bSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg32|Unspecified|BaseIndex }
>  jmp, 0xff, 4, Cpu64, Amd64|Modrm|JumpAbsolute|No_bSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg16|Reg64|Unspecified|BaseIndex }
>  jmp, 0xff, 4, Cpu64, Intel64|Modrm|JumpAbsolute|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64|BNDPrefixOk|NoTrackPrefixOk, { Reg64|Unspecified|BaseIndex }
> @@ -459,7 +460,7 @@ leave, 0xc9, None, Cpu64, DefaultSize|No
>           s:8, ns:9, p:a, pe:a, np:b, po:b, l:c, nge:c, nl:d, ge:d, le:e, ng:e, nle:f, g:f>
>
>  // Conditional jumps.
> -j<cc>, 0x7<cc:opc>, None, 0, Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32|Disp32S }
> +j<cc>, 0x7<cc:opc>, None, 0, Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32 }
>
>  // jcxz vs. jecxz is chosen on the basis of the address size prefix.
>  jcxz, 0xe3, None, CpuNo64, JumpByte|Size16|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
> @@ -1911,7 +1912,7 @@ xrelease, 0xf3, None, CpuHLE, No_bSuf|No
>
>  // RTM instructions
>  xabort, 0xc6f8, None, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8 }
> -xbegin, 0xc7f8, None, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Disp32S }
> +xbegin, 0xc7f8, None, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32 }
>  xend, 0xf01d5, None, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
>  xtest, 0xf01d6, None, CpuHLE|CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
>
>

OK.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-30 22:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-30 12:06 [PATCH 0/3] gas/x86: displacement handling adjustments Jan Beulich
2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
2022-06-30 17:50   ` H.J. Lu
2022-06-30 12:08 ` [PATCH 2/3] x86: restore masking of displacement kinds Jan Beulich
2022-06-30 22:47   ` H.J. Lu
2022-06-30 12:10 ` [PATCH 3/3] x86: fold Disp32S and Disp32 Jan Beulich
2022-06-30 22:51   ` H.J. Lu

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).