public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86-64: correct segment override prefix generation
@ 2012-07-24 15:28 Jan Beulich
  2012-07-30 17:04 ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-07-24 15:28 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Despite them being ignored by the CPU, gas issues segment override
prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
should clearly do this correctly. Determining the default segment,
however, requires to take into consideration RegRex (so far, RSP, RBP,
R12, and R13 were all treated equally here).

2012-07-24  Jan Beulich <jbeulich@suse.com>

	* config/tc-i386-intel.c (build_modrm_byte): Split determining
	default segment from figuring out encoding. Honor RegRex for
	the former.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5733,18 +5733,14 @@ build_modrm_byte (void)
 	      i.sib.base = i.base_reg->reg_num;
 	      /* x86-64 ignores REX prefix bit here to avoid decoder
 		 complications.  */
-	      if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
-		{
+	      if (!(i.base_reg->reg_flags & RegRex)
+		  && (i.base_reg->reg_num == EBP_REG_NUM
+		   || i.base_reg->reg_num == ESP_REG_NUM))
 		  default_seg = &ss;
-		  if (i.disp_operands == 0)
-		    {
-		      fake_zero_displacement = 1;
-		      i.types[op].bitfield.disp8 = 1;
-		    }
-		}
-	      else if (i.base_reg->reg_num == ESP_REG_NUM)
+	      if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
 		{
-		  default_seg = &ss;
+		  fake_zero_displacement = 1;
+		  i.types[op].bitfield.disp8 = 1;
 		}
 	      i.sib.scale = i.log2_scale_factor;
 	      if (i.index_reg == 0)




[-- Attachment #2: binutils-mainline-x86_64-default-seg.patch --]
[-- Type: text/plain, Size: 1417 bytes --]

Despite them being ignored by the CPU, gas issues segment override
prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
should clearly do this correctly. Determining the default segment,
however, requires to take into consideration RegRex (so far, RSP, RBP,
R12, and R13 were all treated equally here).

2012-07-24  Jan Beulich <jbeulich@suse.com>

	* config/tc-i386-intel.c (build_modrm_byte): Split determining
	default segment from figuring out encoding. Honor RegRex for
	the former.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5733,18 +5733,14 @@ build_modrm_byte (void)
 	      i.sib.base = i.base_reg->reg_num;
 	      /* x86-64 ignores REX prefix bit here to avoid decoder
 		 complications.  */
-	      if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
-		{
+	      if (!(i.base_reg->reg_flags & RegRex)
+		  && (i.base_reg->reg_num == EBP_REG_NUM
+		   || i.base_reg->reg_num == ESP_REG_NUM))
 		  default_seg = &ss;
-		  if (i.disp_operands == 0)
-		    {
-		      fake_zero_displacement = 1;
-		      i.types[op].bitfield.disp8 = 1;
-		    }
-		}
-	      else if (i.base_reg->reg_num == ESP_REG_NUM)
+	      if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
 		{
-		  default_seg = &ss;
+		  fake_zero_displacement = 1;
+		  i.types[op].bitfield.disp8 = 1;
 		}
 	      i.sib.scale = i.log2_scale_factor;
 	      if (i.index_reg == 0)

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

* Re: [PATCH] x86-64: correct segment override prefix generation
  2012-07-24 15:28 [PATCH] x86-64: correct segment override prefix generation Jan Beulich
@ 2012-07-30 17:04 ` H.J. Lu
  2012-08-07 10:42   ` [PATCH, v2] " Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2012-07-30 17:04 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Tue, Jul 24, 2012 at 8:28 AM, Jan Beulich <JBeulich@suse.com> wrote:
> Despite them being ignored by the CPU, gas issues segment override
> prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
> should clearly do this correctly. Determining the default segment,
> however, requires to take into consideration RegRex (so far, RSP, RBP,
> R12, and R13 were all treated equally here).
>
> 2012-07-24  Jan Beulich <jbeulich@suse.com>
>
>         * config/tc-i386-intel.c (build_modrm_byte): Split determining
>         default segment from figuring out encoding. Honor RegRex for
>         the former.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -5733,18 +5733,14 @@ build_modrm_byte (void)
>               i.sib.base = i.base_reg->reg_num;
>               /* x86-64 ignores REX prefix bit here to avoid decoder
>                  complications.  */
> -             if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
> -               {
> +             if (!(i.base_reg->reg_flags & RegRex)
> +                 && (i.base_reg->reg_num == EBP_REG_NUM
> +                  || i.base_reg->reg_num == ESP_REG_NUM))
>                   default_seg = &ss;
> -                 if (i.disp_operands == 0)
> -                   {
> -                     fake_zero_displacement = 1;
> -                     i.types[op].bitfield.disp8 = 1;
> -                   }
> -               }
> -             else if (i.base_reg->reg_num == ESP_REG_NUM)
> +             if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
>                 {
> -                 default_seg = &ss;
> +                 fake_zero_displacement = 1;
> +                 i.types[op].bitfield.disp8 = 1;
>                 }
>               i.sib.scale = i.log2_scale_factor;
>               if (i.index_reg == 0)
>

Please provide a testcase to show the correct behavior.

Thanks.

-- 
H.J.

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

* [PATCH, v2] x86-64: correct segment override prefix generation
  2012-07-30 17:04 ` H.J. Lu
@ 2012-08-07 10:42   ` Jan Beulich
  2012-08-07 13:46     ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-08-07 10:42 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

[-- Attachment #1: Type: text/plain, Size: 4855 bytes --]

>>> On 30.07.12 at 19:03, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> Please provide a testcase to show the correct behavior.

Here you go.

Jan

Despite them being ignored by the CPU, gas issues segment override
prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
should clearly do this correctly. Determining the default segment,
however, requires to take into consideration RegRex (so far, RSP, RBP,
R12, and R13 were all treated equally here).

gas/
2012-08-07  Jan Beulich <jbeulich@suse.com>
	* config/tc-i386-intel.c (build_modrm_byte): Split determining
	default segment from figuring out encoding. Honor RegRex for
	the former.

gas/testsuite/
2012-08-07  Jan Beulich <jbeulich@suse.com>

	* gas/i386/x86-64-segovr.{s,l}: New.
	* gas/i386/i386.exp: Run new test.

--- 2012-08-07/gas/config/tc-i386.c	2012-07-31 09:45:03.000000000 +0200
+++ 2012-08-07/gas/config/tc-i386.c	2012-08-07 12:13:39.000000000 +0200
@@ -5729,18 +5729,14 @@ build_modrm_byte (void)
 	      i.sib.base = i.base_reg->reg_num;
 	      /* x86-64 ignores REX prefix bit here to avoid decoder
 		 complications.  */
-	      if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
-		{
+	      if (!(i.base_reg->reg_flags & RegRex)
+		  && (i.base_reg->reg_num == EBP_REG_NUM
+		   || i.base_reg->reg_num == ESP_REG_NUM))
 		  default_seg = &ss;
-		  if (i.disp_operands == 0)
-		    {
-		      fake_zero_displacement = 1;
-		      i.types[op].bitfield.disp8 = 1;
-		    }
-		}
-	      else if (i.base_reg->reg_num == ESP_REG_NUM)
+	      if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
 		{
-		  default_seg = &ss;
+		  fake_zero_displacement = 1;
+		  i.types[op].bitfield.disp8 = 1;
 		}
 	      i.sib.scale = i.log2_scale_factor;
 	      if (i.index_reg == 0)
--- 2012-08-07/gas/testsuite/gas/i386/i386.exp	2012-07-24 14:52:59.000000000 +0200
+++ 2012-08-07/gas/testsuite/gas/i386/i386.exp	2012-08-07 12:13:39.000000000 +0200
@@ -311,6 +311,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     run_dump_test "x86-64-stack-suffix"
     run_list_test "x86-64-inval" "-al"
     run_list_test "x86-64-segment" "-al"
+    run_dump_test "x86-64-segovr"
     run_list_test "x86-64-inval-seg" "-al"
     run_dump_test "x86-64-branch"
     run_dump_test "x86-64-relax-1"
--- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d	1970-01-01 01:00:00.000000000 +0100
+++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d	2012-08-03 14:06:44.000000000 +0200
@@ -0,0 +1,41 @@
+#objdump: -dw
+#name: x86-64 segment overrides
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <segovr>:
+[ 	]*[a-f0-9]+:	8b 00[ 	]+mov[ 	]+\(%rax\),%eax
+[ 	]*[a-f0-9]+:	8b 01[ 	]+mov[ 	]+\(%rcx\),%eax
+[ 	]*[a-f0-9]+:	8b 02[ 	]+mov[ 	]+\(%rdx\),%eax
+[ 	]*[a-f0-9]+:	8b 03[ 	]+mov[ 	]+\(%rbx\),%eax
+[ 	]*[a-f0-9]+:	3e 8b 04 24[ 	]+mov[ 	]+%ds:\(%rsp\),%eax
+[ 	]*[a-f0-9]+:	3e 8b 45 00[ 	]+mov[ 	]+%ds:((0x)?0)?\(%rbp\),%eax
+[ 	]*[a-f0-9]+:	8b 06[ 	]+mov[ 	]+\(%rsi\),%eax
+[ 	]*[a-f0-9]+:	8b 07[ 	]+mov[ 	]+\(%rdi\),%eax
+[ 	]*[a-f0-9]+:	41 8b 00[ 	]+mov[ 	]+\(%r8\),%eax
+[ 	]*[a-f0-9]+:	41 8b 01[ 	]+mov[ 	]+\(%r9\),%eax
+[ 	]*[a-f0-9]+:	41 8b 02[ 	]+mov[ 	]+\(%r10\),%eax
+[ 	]*[a-f0-9]+:	41 8b 03[ 	]+mov[ 	]+\(%r11\),%eax
+[ 	]*[a-f0-9]+:	41 8b 04 24[ 	]+mov[ 	]+\(%r12\),%eax
+[ 	]*[a-f0-9]+:	41 8b 45 00[ 	]+mov[ 	]+((0x)?0)?\(%r13\),%eax
+[ 	]*[a-f0-9]+:	41 8b 06[ 	]+mov[ 	]+\(%r14\),%eax
+[ 	]*[a-f0-9]+:	41 8b 07[ 	]+mov[ 	]+\(%r15\),%eax
+[ 	]*[a-f0-9]+:	36 8b 00[ 	]+mov[ 	]+%ss:\(%rax\),%eax
+[ 	]*[a-f0-9]+:	36 8b 01[ 	]+mov[ 	]+%ss:\(%rcx\),%eax
+[ 	]*[a-f0-9]+:	36 8b 02[ 	]+mov[ 	]+%ss:\(%rdx\),%eax
+[ 	]*[a-f0-9]+:	36 8b 03[ 	]+mov[ 	]+%ss:\(%rbx\),%eax
+[ 	]*[a-f0-9]+:	8b 04 24[ 	]+mov[ 	]+\(%rsp\),%eax
+[ 	]*[a-f0-9]+:	8b 45 00[ 	]+mov[ 	]+((0x)?0)?\(%rbp\),%eax
+[ 	]*[a-f0-9]+:	36 8b 06[ 	]+mov[ 	]+%ss:\(%rsi\),%eax
+[ 	]*[a-f0-9]+:	36 8b 07[ 	]+mov[ 	]+%ss:\(%rdi\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 00[ 	]+mov[ 	]+%ss:\(%r8\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 01[ 	]+mov[ 	]+%ss:\(%r9\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 02[ 	]+mov[ 	]+%ss:\(%r10\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 03[ 	]+mov[ 	]+%ss:\(%r11\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 04 24[ 	]+mov[ 	]+%ss:\(%r12\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 45 00[ 	]+mov[ 	]+%ss:((0x)?0)?\(%r13\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 06[ 	]+mov[ 	]+%ss:\(%r14\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 07[ 	]+mov[ 	]+%ss:\(%r15\),%eax
+#pass
--- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s	1970-01-01 01:00:00.000000000 +0100
+++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s	2012-08-03 13:54:08.000000000 +0200
@@ -0,0 +1,9 @@
+# 64bit segment overrides
+
+	.text
+segovr:
+.irp seg, ds, ss
+ .irp reg, ax, cx, dx, bx, sp, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15
+	mov	%\seg:(%r\reg), %eax
+ .endr
+.endr



[-- Attachment #2: binutils-mainline-x86_64-default-seg.patch --]
[-- Type: text/plain, Size: 4701 bytes --]

Despite them being ignored by the CPU, gas issues segment override
prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
should clearly do this correctly. Determining the default segment,
however, requires to take into consideration RegRex (so far, RSP, RBP,
R12, and R13 were all treated equally here).

gas/
2012-08-07  Jan Beulich <jbeulich@suse.com>
	* config/tc-i386-intel.c (build_modrm_byte): Split determining
	default segment from figuring out encoding. Honor RegRex for
	the former.

gas/testsuite/
2012-08-07  Jan Beulich <jbeulich@suse.com>

	* gas/i386/x86-64-segovr.{s,l}: New.
	* gas/i386/i386.exp: Run new test.

--- 2012-08-07/gas/config/tc-i386.c	2012-07-31 09:45:03.000000000 +0200
+++ 2012-08-07/gas/config/tc-i386.c	2012-08-07 12:13:39.000000000 +0200
@@ -5729,18 +5729,14 @@ build_modrm_byte (void)
 	      i.sib.base = i.base_reg->reg_num;
 	      /* x86-64 ignores REX prefix bit here to avoid decoder
 		 complications.  */
-	      if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
-		{
+	      if (!(i.base_reg->reg_flags & RegRex)
+		  && (i.base_reg->reg_num == EBP_REG_NUM
+		   || i.base_reg->reg_num == ESP_REG_NUM))
 		  default_seg = &ss;
-		  if (i.disp_operands == 0)
-		    {
-		      fake_zero_displacement = 1;
-		      i.types[op].bitfield.disp8 = 1;
-		    }
-		}
-	      else if (i.base_reg->reg_num == ESP_REG_NUM)
+	      if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
 		{
-		  default_seg = &ss;
+		  fake_zero_displacement = 1;
+		  i.types[op].bitfield.disp8 = 1;
 		}
 	      i.sib.scale = i.log2_scale_factor;
 	      if (i.index_reg == 0)
--- 2012-08-07/gas/testsuite/gas/i386/i386.exp	2012-07-24 14:52:59.000000000 +0200
+++ 2012-08-07/gas/testsuite/gas/i386/i386.exp	2012-08-07 12:13:39.000000000 +0200
@@ -311,6 +311,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     run_dump_test "x86-64-stack-suffix"
     run_list_test "x86-64-inval" "-al"
     run_list_test "x86-64-segment" "-al"
+    run_dump_test "x86-64-segovr"
     run_list_test "x86-64-inval-seg" "-al"
     run_dump_test "x86-64-branch"
     run_dump_test "x86-64-relax-1"
--- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d	1970-01-01 01:00:00.000000000 +0100
+++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d	2012-08-03 14:06:44.000000000 +0200
@@ -0,0 +1,41 @@
+#objdump: -dw
+#name: x86-64 segment overrides
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <segovr>:
+[ 	]*[a-f0-9]+:	8b 00[ 	]+mov[ 	]+\(%rax\),%eax
+[ 	]*[a-f0-9]+:	8b 01[ 	]+mov[ 	]+\(%rcx\),%eax
+[ 	]*[a-f0-9]+:	8b 02[ 	]+mov[ 	]+\(%rdx\),%eax
+[ 	]*[a-f0-9]+:	8b 03[ 	]+mov[ 	]+\(%rbx\),%eax
+[ 	]*[a-f0-9]+:	3e 8b 04 24[ 	]+mov[ 	]+%ds:\(%rsp\),%eax
+[ 	]*[a-f0-9]+:	3e 8b 45 00[ 	]+mov[ 	]+%ds:((0x)?0)?\(%rbp\),%eax
+[ 	]*[a-f0-9]+:	8b 06[ 	]+mov[ 	]+\(%rsi\),%eax
+[ 	]*[a-f0-9]+:	8b 07[ 	]+mov[ 	]+\(%rdi\),%eax
+[ 	]*[a-f0-9]+:	41 8b 00[ 	]+mov[ 	]+\(%r8\),%eax
+[ 	]*[a-f0-9]+:	41 8b 01[ 	]+mov[ 	]+\(%r9\),%eax
+[ 	]*[a-f0-9]+:	41 8b 02[ 	]+mov[ 	]+\(%r10\),%eax
+[ 	]*[a-f0-9]+:	41 8b 03[ 	]+mov[ 	]+\(%r11\),%eax
+[ 	]*[a-f0-9]+:	41 8b 04 24[ 	]+mov[ 	]+\(%r12\),%eax
+[ 	]*[a-f0-9]+:	41 8b 45 00[ 	]+mov[ 	]+((0x)?0)?\(%r13\),%eax
+[ 	]*[a-f0-9]+:	41 8b 06[ 	]+mov[ 	]+\(%r14\),%eax
+[ 	]*[a-f0-9]+:	41 8b 07[ 	]+mov[ 	]+\(%r15\),%eax
+[ 	]*[a-f0-9]+:	36 8b 00[ 	]+mov[ 	]+%ss:\(%rax\),%eax
+[ 	]*[a-f0-9]+:	36 8b 01[ 	]+mov[ 	]+%ss:\(%rcx\),%eax
+[ 	]*[a-f0-9]+:	36 8b 02[ 	]+mov[ 	]+%ss:\(%rdx\),%eax
+[ 	]*[a-f0-9]+:	36 8b 03[ 	]+mov[ 	]+%ss:\(%rbx\),%eax
+[ 	]*[a-f0-9]+:	8b 04 24[ 	]+mov[ 	]+\(%rsp\),%eax
+[ 	]*[a-f0-9]+:	8b 45 00[ 	]+mov[ 	]+((0x)?0)?\(%rbp\),%eax
+[ 	]*[a-f0-9]+:	36 8b 06[ 	]+mov[ 	]+%ss:\(%rsi\),%eax
+[ 	]*[a-f0-9]+:	36 8b 07[ 	]+mov[ 	]+%ss:\(%rdi\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 00[ 	]+mov[ 	]+%ss:\(%r8\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 01[ 	]+mov[ 	]+%ss:\(%r9\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 02[ 	]+mov[ 	]+%ss:\(%r10\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 03[ 	]+mov[ 	]+%ss:\(%r11\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 04 24[ 	]+mov[ 	]+%ss:\(%r12\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 45 00[ 	]+mov[ 	]+%ss:((0x)?0)?\(%r13\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 06[ 	]+mov[ 	]+%ss:\(%r14\),%eax
+[ 	]*[a-f0-9]+:	36 41 8b 07[ 	]+mov[ 	]+%ss:\(%r15\),%eax
+#pass
--- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s	1970-01-01 01:00:00.000000000 +0100
+++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s	2012-08-03 13:54:08.000000000 +0200
@@ -0,0 +1,9 @@
+# 64bit segment overrides
+
+	.text
+segovr:
+.irp seg, ds, ss
+ .irp reg, ax, cx, dx, bx, sp, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15
+	mov	%\seg:(%r\reg), %eax
+ .endr
+.endr

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

* Re: [PATCH, v2] x86-64: correct segment override prefix generation
  2012-08-07 10:42   ` [PATCH, v2] " Jan Beulich
@ 2012-08-07 13:46     ` H.J. Lu
  2012-08-07 13:56       ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2012-08-07 13:46 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Tue, Aug 7, 2012 at 3:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 30.07.12 at 19:03, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> Please provide a testcase to show the correct behavior.
>
> Here you go.
>
> Jan
>
> Despite them being ignored by the CPU, gas issues segment override
> prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
> should clearly do this correctly. Determining the default segment,
> however, requires to take into consideration RegRex (so far, RSP, RBP,
> R12, and R13 were all treated equally here).
>
> gas/
> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>         * config/tc-i386-intel.c (build_modrm_byte): Split determining
>         default segment from figuring out encoding. Honor RegRex for
>         the former.
>
> gas/testsuite/
> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>
>         * gas/i386/x86-64-segovr.{s,l}: New.
>         * gas/i386/i386.exp: Run new test.
>
> --- 2012-08-07/gas/config/tc-i386.c     2012-07-31 09:45:03.000000000 +0200
> +++ 2012-08-07/gas/config/tc-i386.c     2012-08-07 12:13:39.000000000 +0200
> @@ -5729,18 +5729,14 @@ build_modrm_byte (void)
>               i.sib.base = i.base_reg->reg_num;
>               /* x86-64 ignores REX prefix bit here to avoid decoder
>                  complications.  */
> -             if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
> -               {
> +             if (!(i.base_reg->reg_flags & RegRex)
> +                 && (i.base_reg->reg_num == EBP_REG_NUM
> +                  || i.base_reg->reg_num == ESP_REG_NUM))
>                   default_seg = &ss;
> -                 if (i.disp_operands == 0)
> -                   {
> -                     fake_zero_displacement = 1;
> -                     i.types[op].bitfield.disp8 = 1;
> -                   }
> -               }
> -             else if (i.base_reg->reg_num == ESP_REG_NUM)
> +             if (i.base_reg->reg_num == 5 && i.disp_operands == 0)

Please use EBP_REG_NUM instead 5 here.

>                 {
> -                 default_seg = &ss;
> +                 fake_zero_displacement = 1;
> +                 i.types[op].bitfield.disp8 = 1;
>                 }
>               i.sib.scale = i.log2_scale_factor;
>               if (i.index_reg == 0)
> --- 2012-08-07/gas/testsuite/gas/i386/i386.exp  2012-07-24 14:52:59.000000000 +0200
> +++ 2012-08-07/gas/testsuite/gas/i386/i386.exp  2012-08-07 12:13:39.000000000 +0200
> @@ -311,6 +311,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
>      run_dump_test "x86-64-stack-suffix"
>      run_list_test "x86-64-inval" "-al"
>      run_list_test "x86-64-segment" "-al"
> +    run_dump_test "x86-64-segovr"
>      run_list_test "x86-64-inval-seg" "-al"
>      run_dump_test "x86-64-branch"
>      run_dump_test "x86-64-relax-1"
> --- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d   1970-01-01 01:00:00.000000000 +0100
> +++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.d   2012-08-03 14:06:44.000000000 +0200
> @@ -0,0 +1,41 @@
> +#objdump: -dw
> +#name: x86-64 segment overrides
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <segovr>:
> +[      ]*[a-f0-9]+:    8b 00[  ]+mov[  ]+\(%rax\),%eax
> +[      ]*[a-f0-9]+:    8b 01[  ]+mov[  ]+\(%rcx\),%eax
> +[      ]*[a-f0-9]+:    8b 02[  ]+mov[  ]+\(%rdx\),%eax
> +[      ]*[a-f0-9]+:    8b 03[  ]+mov[  ]+\(%rbx\),%eax
> +[      ]*[a-f0-9]+:    3e 8b 04 24[    ]+mov[  ]+%ds:\(%rsp\),%eax
> +[      ]*[a-f0-9]+:    3e 8b 45 00[    ]+mov[  ]+%ds:((0x)?0)?\(%rbp\),%eax
> +[      ]*[a-f0-9]+:    8b 06[  ]+mov[  ]+\(%rsi\),%eax
> +[      ]*[a-f0-9]+:    8b 07[  ]+mov[  ]+\(%rdi\),%eax
> +[      ]*[a-f0-9]+:    41 8b 00[       ]+mov[  ]+\(%r8\),%eax
> +[      ]*[a-f0-9]+:    41 8b 01[       ]+mov[  ]+\(%r9\),%eax
> +[      ]*[a-f0-9]+:    41 8b 02[       ]+mov[  ]+\(%r10\),%eax
> +[      ]*[a-f0-9]+:    41 8b 03[       ]+mov[  ]+\(%r11\),%eax
> +[      ]*[a-f0-9]+:    41 8b 04 24[    ]+mov[  ]+\(%r12\),%eax
> +[      ]*[a-f0-9]+:    41 8b 45 00[    ]+mov[  ]+((0x)?0)?\(%r13\),%eax
> +[      ]*[a-f0-9]+:    41 8b 06[       ]+mov[  ]+\(%r14\),%eax
> +[      ]*[a-f0-9]+:    41 8b 07[       ]+mov[  ]+\(%r15\),%eax
> +[      ]*[a-f0-9]+:    36 8b 00[       ]+mov[  ]+%ss:\(%rax\),%eax
> +[      ]*[a-f0-9]+:    36 8b 01[       ]+mov[  ]+%ss:\(%rcx\),%eax
> +[      ]*[a-f0-9]+:    36 8b 02[       ]+mov[  ]+%ss:\(%rdx\),%eax
> +[      ]*[a-f0-9]+:    36 8b 03[       ]+mov[  ]+%ss:\(%rbx\),%eax
> +[      ]*[a-f0-9]+:    8b 04 24[       ]+mov[  ]+\(%rsp\),%eax
> +[      ]*[a-f0-9]+:    8b 45 00[       ]+mov[  ]+((0x)?0)?\(%rbp\),%eax
> +[      ]*[a-f0-9]+:    36 8b 06[       ]+mov[  ]+%ss:\(%rsi\),%eax
> +[      ]*[a-f0-9]+:    36 8b 07[       ]+mov[  ]+%ss:\(%rdi\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 00[    ]+mov[  ]+%ss:\(%r8\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 01[    ]+mov[  ]+%ss:\(%r9\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 02[    ]+mov[  ]+%ss:\(%r10\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 03[    ]+mov[  ]+%ss:\(%r11\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 04 24[         ]+mov[  ]+%ss:\(%r12\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 45 00[         ]+mov[  ]+%ss:((0x)?0)?\(%r13\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 06[    ]+mov[  ]+%ss:\(%r14\),%eax
> +[      ]*[a-f0-9]+:    36 41 8b 07[    ]+mov[  ]+%ss:\(%r15\),%eax
> +#pass
> --- 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s   1970-01-01 01:00:00.000000000 +0100
> +++ 2012-08-07/gas/testsuite/gas/i386/x86-64-segovr.s   2012-08-03 13:54:08.000000000 +0200
> @@ -0,0 +1,9 @@
> +# 64bit segment overrides
> +
> +       .text
> +segovr:
> +.irp seg, ds, ss
> + .irp reg, ax, cx, dx, bx, sp, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15
> +       mov     %\seg:(%r\reg), %eax
> + .endr
> +.endr
>
>

OK with the EBP_REG_NUM change above if Linux x86-64 kernel
compiles and runs.

Thanks.

-- 
H.J.

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

* Re: [PATCH, v2] x86-64: correct segment override prefix generation
  2012-08-07 13:46     ` H.J. Lu
@ 2012-08-07 13:56       ` Jan Beulich
  2012-08-07 14:03         ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-08-07 13:56 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

>>> On 07.08.12 at 15:44, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> On Tue, Aug 7, 2012 at 3:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 30.07.12 at 19:03, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>> Please provide a testcase to show the correct behavior.
>>
>> Here you go.
>>
>> Jan
>>
>> Despite them being ignored by the CPU, gas issues segment override
>> prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
>> should clearly do this correctly. Determining the default segment,
>> however, requires to take into consideration RegRex (so far, RSP, RBP,
>> R12, and R13 were all treated equally here).
>>
>> gas/
>> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>>         * config/tc-i386-intel.c (build_modrm_byte): Split determining
>>         default segment from figuring out encoding. Honor RegRex for
>>         the former.
>>
>> gas/testsuite/
>> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>>
>>         * gas/i386/x86-64-segovr.{s,l}: New.
>>         * gas/i386/i386.exp: Run new test.
>>
>> --- 2012-08-07/gas/config/tc-i386.c     2012-07-31 09:45:03.000000000 +0200
>> +++ 2012-08-07/gas/config/tc-i386.c     2012-08-07 12:13:39.000000000 +0200
>> @@ -5729,18 +5729,14 @@ build_modrm_byte (void)
>>               i.sib.base = i.base_reg->reg_num;
>>               /* x86-64 ignores REX prefix bit here to avoid decoder
>>                  complications.  */
>> -             if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
>> -               {
>> +             if (!(i.base_reg->reg_flags & RegRex)
>> +                 && (i.base_reg->reg_num == EBP_REG_NUM
>> +                  || i.base_reg->reg_num == ESP_REG_NUM))
>>                   default_seg = &ss;
>> -                 if (i.disp_operands == 0)
>> -                   {
>> -                     fake_zero_displacement = 1;
>> -                     i.types[op].bitfield.disp8 = 1;
>> -                   }
>> -               }
>> -             else if (i.base_reg->reg_num == ESP_REG_NUM)
>> +             if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
> 
> Please use EBP_REG_NUM instead 5 here.

But that change was intentional - we're _not_ looking for EBP here,
we're looking for "EBP or R13". The previous use of EBP_REG_NUM
was part of why this was broken imo.

> OK with the EBP_REG_NUM change above if Linux x86-64 kernel
> compiles and runs.

Sure, that has been the case for many weeks already.

Jan

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

* Re: [PATCH, v2] x86-64: correct segment override prefix generation
  2012-08-07 13:56       ` Jan Beulich
@ 2012-08-07 14:03         ` H.J. Lu
  0 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2012-08-07 14:03 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Tue, Aug 7, 2012 at 6:47 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.08.12 at 15:44, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> On Tue, Aug 7, 2012 at 3:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 30.07.12 at 19:03, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>>> Please provide a testcase to show the correct behavior.
>>>
>>> Here you go.
>>>
>>> Jan
>>>
>>> Despite them being ignored by the CPU, gas issues segment override
>>> prefixes for other than FS/GS in 64-bit mode. If doing so at all, it
>>> should clearly do this correctly. Determining the default segment,
>>> however, requires to take into consideration RegRex (so far, RSP, RBP,
>>> R12, and R13 were all treated equally here).
>>>
>>> gas/
>>> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>>>         * config/tc-i386-intel.c (build_modrm_byte): Split determining
>>>         default segment from figuring out encoding. Honor RegRex for
>>>         the former.
>>>
>>> gas/testsuite/
>>> 2012-08-07  Jan Beulich <jbeulich@suse.com>
>>>
>>>         * gas/i386/x86-64-segovr.{s,l}: New.
>>>         * gas/i386/i386.exp: Run new test.
>>>
>>> --- 2012-08-07/gas/config/tc-i386.c     2012-07-31 09:45:03.000000000 +0200
>>> +++ 2012-08-07/gas/config/tc-i386.c     2012-08-07 12:13:39.000000000 +0200
>>> @@ -5729,18 +5729,14 @@ build_modrm_byte (void)
>>>               i.sib.base = i.base_reg->reg_num;
>>>               /* x86-64 ignores REX prefix bit here to avoid decoder
>>>                  complications.  */
>>> -             if ((i.base_reg->reg_num & 7) == EBP_REG_NUM)
>>> -               {
>>> +             if (!(i.base_reg->reg_flags & RegRex)
>>> +                 && (i.base_reg->reg_num == EBP_REG_NUM
>>> +                  || i.base_reg->reg_num == ESP_REG_NUM))
>>>                   default_seg = &ss;
>>> -                 if (i.disp_operands == 0)
>>> -                   {
>>> -                     fake_zero_displacement = 1;
>>> -                     i.types[op].bitfield.disp8 = 1;
>>> -                   }
>>> -               }
>>> -             else if (i.base_reg->reg_num == ESP_REG_NUM)
>>> +             if (i.base_reg->reg_num == 5 && i.disp_operands == 0)
>>
>> Please use EBP_REG_NUM instead 5 here.
>
> But that change was intentional - we're _not_ looking for EBP here,
> we're looking for "EBP or R13". The previous use of EBP_REG_NUM
> was part of why this was broken imo.
>
>> OK with the EBP_REG_NUM change above if Linux x86-64 kernel
>> compiles and runs.
>
> Sure, that has been the case for many weeks already.
>

OK then.

Thanks.


-- 
H.J.

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

end of thread, other threads:[~2012-08-07 14:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-24 15:28 [PATCH] x86-64: correct segment override prefix generation Jan Beulich
2012-07-30 17:04 ` H.J. Lu
2012-08-07 10:42   ` [PATCH, v2] " Jan Beulich
2012-08-07 13:46     ` H.J. Lu
2012-08-07 13:56       ` Jan Beulich
2012-08-07 14:03         ` 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).