Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly in Intel syntax

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly in Intel syntax

Jan Beulich
While I agree on the subject, I slightly disagree on the approach you took: The added flags shouldn't go on the instructions, but on their operands (otherwise you'll likely end up creating more special case code namely for movzx/movsx, but perhaps also elsewhere): Just like for registers, memory operands should properly specify what sizes are acceptable (basically, operand type and operand size should probably be decoupled). Jan

>>> "H.J. Lu" <[hidden email]> 01/02/08 9:54 PM >>>
If an instruction is marked with IgnoreSize, we don't check for
memory size in Intel mode. I am checking in this patch to create
the infrastructure to handle it properly. I will fix movq first
and work on others later. Eventually, the x86 assembler will check
memory size for all instructions in Intel mode.


H.J.
----
gas/

2008-01-02  H.J. Lu  <[hidden email]>

        PR gas/5534
        * config/tc-i386.c (match_template): Handle XMMWORD_MNEM_SUFFIX.
        Check memory size in Intel mode.
        (process_suffix): Handle XMMWORD_MNEM_SUFFIX.
        (intel_e09): Likewise.

        * config/tc-i386.h (XMMWORD_MNEM_SUFFIX): New.

gas/testsuite/

2008-01-02  H.J. Lu  <[hidden email]>

        PR gas/5534
        * gas/i386/intel.s: Use QWORD on movq instead of DWORD.

        * gas/i386/inval.s: Add tests for movq.
        * gas/i386/x86-64-inval.s: Likewise.

        * gas/i386/inval.l: Updated.
        * gas/i386/x86-64-inval.l: Likewise.

opcodes/

2008-01-02  H.J. Lu  <[hidden email]>

        PR gas/5534
        * i386-gen.c (opcode_modifiers): Add No_xSuf, CheckSize,
        Byte, Word, Dword, QWord and Xmmword.

        * i386-opc.h (No_xSuf): New.
        (CheckSize): Likewise.
        (Byte): Likewise.
        (Word): Likewise.
        (Dword): Likewise.
        (QWord): Likewise.
        (Xmmword): Likewise.
        (FWait): Updated.
        (i386_opcode_modifier): Add No_xSuf, CheckSize, Byte, Word,
        Dword, QWord and Xmmword.

        * i386-opc.tbl: Add CheckSize|QWord to movq if IgnoreSize is
        used.
        * i386-tbl.h: Regenerated.

--- binutils/gas/config/tc-i386.c.ptr 2007-12-31 10:53:14.000000000 -0800
+++ binutils/gas/config/tc-i386.c 2008-01-02 12:24:58.000000000 -0800
@@ -3047,6 +3047,8 @@ match_template (void)
     suffix_check.no_qsuf = 1;
   else if (i.suffix == LONG_DOUBLE_MNEM_SUFFIX)
     suffix_check.no_ldsuf = 1;
+  else if (i.suffix == XMMWORD_MNEM_SUFFIX)
+    suffix_check.no_xsuf = 1;
 
   for (t = current_templates->start; t < current_templates->end; t++)
     {
@@ -3077,6 +3079,18 @@ match_template (void)
       || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf)))
  continue;
 
+      /* Check memory size in Intel mode if needed when it is provided
+ and isn't ignored.  */
+      if (intel_syntax
+  && (i.suffix || !t->opcode_modifier.ignoresize)
+  && t->opcode_modifier.checksize
+  && !((t->opcode_modifier.byte && suffix_check.no_bsuf)
+       || (t->opcode_modifier.word && suffix_check.no_wsuf)
+       || (t->opcode_modifier.dword && suffix_check.no_lsuf)
+       || (t->opcode_modifier.qword && suffix_check.no_qsuf)
+       || (t->opcode_modifier.xmmword && suffix_check.no_xsuf)))
+ continue;
+
       for (j = 0; j < MAX_OPERANDS; j++)
  operand_types [j] = t->operand_types [j];
 
@@ -3453,6 +3467,11 @@ process_suffix (void)
   if (!check_word_reg ())
     return 0;
  }
+      else if (i.suffix == XMMWORD_MNEM_SUFFIX)
+ {
+  /* Skip if the instruction has x suffix.  match_template
+     should check if it is a valid suffix.  */
+ }
       else if (intel_syntax && i.tm.opcode_modifier.ignoresize)
  /* Do nothing if the instruction is going to ignore the prefix.  */
  ;
@@ -3535,7 +3554,9 @@ process_suffix (void)
   /* Change the opcode based on the operand size given by i.suffix;
      We don't need to change things for byte insns.  */
 
-  if (i.suffix && i.suffix != BYTE_MNEM_SUFFIX)
+  if (i.suffix
+      && i.suffix != BYTE_MNEM_SUFFIX
+      && i.suffix != XMMWORD_MNEM_SUFFIX)
     {
       /* It's not a byte, select word/dword operation.  */
       if (i.tm.opcode_modifier.w)
@@ -8166,8 +8187,7 @@ intel_e09 (void)
 
   else if (prev_token.code == T_XMMWORD)
     {
-      /* XXX ignored for now, but accepted since gcc uses it */
-      suffix = 0;
+      suffix = XMMWORD_MNEM_SUFFIX;
     }
 
   else
--- binutils/gas/config/tc-i386.h.ptr 2007-11-01 11:48:52.000000000 -0700
+++ binutils/gas/config/tc-i386.h 2008-01-02 10:40:23.000000000 -0800
@@ -116,12 +116,14 @@ extern const char *i386_comment_chars;
 #define IMMEDIATE_PREFIX '$'
 #define ABSOLUTE_PREFIX '*'
 
-/* these are the instruction mnemonic suffixes.  */
+/* these are the instruction mnemonic suffixes in AT&T syntax or
+   memory operand size in Intel syntax.  */
 #define WORD_MNEM_SUFFIX  'w'
 #define BYTE_MNEM_SUFFIX  'b'
 #define SHORT_MNEM_SUFFIX 's'
 #define LONG_MNEM_SUFFIX  'l'
 #define QWORD_MNEM_SUFFIX  'q'
+#define XMMWORD_MNEM_SUFFIX  'x'
 /* Intel Syntax.  Use a non-ascii letter since since it never appears
    in instructions.  */
 #define LONG_DOUBLE_MNEM_SUFFIX '\1'
--- binutils/gas/testsuite/gas/i386/intel.s.ptr 2006-12-29 13:48:59.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/intel.s 2008-01-02 11:25:07.000000000 -0800
@@ -601,7 +601,7 @@ rot5:
 
 1:
  jne 1b
- movq mm6, [DWORD PTR .LC5+40]
+ movq mm6, [QWORD PTR .LC5+40]
  add edi, dword ptr [ebx+8*eax]
  movd mm0, dword ptr [ebx+8*eax+4]
  add edi, dword ptr [ebx+8*ecx+((4095+1)*8)]
--- binutils/gas/testsuite/gas/i386/inval.l.ptr 2007-12-31 10:53:14.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/inval.l 2008-01-02 12:26:49.000000000 -0800
@@ -53,6 +53,14 @@
 .*:56: Error: .*
 .*:57: Error: .*
 .*:58: Error: .*
+.*:59: Error: .*
+.*:60: Error: .*
+.*:61: Error: .*
+.*:62: Error: .*
+.*:63: Error: .*
+.*:64: Error: .*
+.*:65: Error: .*
+.*:66: Error: .*
 GAS LISTING .*
 
 
@@ -117,3 +125,11 @@ GAS LISTING .*
 
 
 [ ]*58[ ]+cvtsi2sdq xmm1,QWORD PTR \[eax\]
+[ ]*59[ ]+movq xmm1, XMMWORD PTR \[esp\]
+[ ]*60[ ]+movq xmm1, DWORD PTR \[esp\]
+[ ]*61[ ]+movq xmm1, WORD PTR \[esp\]
+[ ]*62[ ]+movq xmm1, BYTE PTR \[esp\]
+[ ]*63[ ]+movq XMMWORD PTR \[esp\],xmm1
+[ ]*64[ ]+movq DWORD PTR \[esp\],xmm1
+[ ]*65[ ]+movq WORD PTR \[esp\],xmm1
+[ ]*66[ ]+movq BYTE PTR \[esp\],xmm1
--- binutils/gas/testsuite/gas/i386/inval.s.ptr 2007-12-31 10:53:14.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/inval.s 2008-01-02 12:02:28.000000000 -0800
@@ -56,3 +56,11 @@ foo: jaw foo
  cvtsi2sd xmm1,QWORD PTR [eax]
  cvtsi2ssq xmm1,QWORD PTR [eax]
  cvtsi2sdq xmm1,QWORD PTR [eax]
+ movq xmm1, XMMWORD PTR [esp]
+ movq xmm1, DWORD PTR [esp]
+ movq xmm1, WORD PTR [esp]
+ movq xmm1, BYTE PTR [esp]
+ movq XMMWORD PTR [esp],xmm1
+ movq DWORD PTR [esp],xmm1
+ movq WORD PTR [esp],xmm1
+ movq BYTE PTR [esp],xmm1
--- binutils/gas/testsuite/gas/i386/x86-64-inval.l.ptr 2006-12-15 05:22:44.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/x86-64-inval.l 2008-01-02 12:27:17.000000000 -0800
@@ -50,60 +50,79 @@
 .*:51: Error: .*
 .*:52: Error: .*
 .*:54: Error: .*
+.*:55: Error: .*
+.*:56: Error: .*
+.*:57: Error: .*
+.*:58: Error: .*
+.*:59: Error: .*
+.*:60: Error: .*
+.*:61: Error: .*
+.*:62: Error: .*
 GAS LISTING .*
 
 
-   1 [ ]*.text
-   2 [ ]*# All the following should be illegal for x86-64
-   3 [ ]*aaa # illegal
-   4 [ ]*aad # illegal
-   5 [ ]*aam # illegal
-   6 [ ]*aas # illegal
-   7 [ ]*arpl %ax,%ax # illegal
-   8 [ ]*bound %eax,\(%rax\) # illegal
-   9 [ ]*calll \*%eax # 32-bit data size not allowed
-  10 [ ]*calll \*\(%ax\) # 32-bit data size not allowed
-  11 [ ]*calll \*\(%eax\) # 32-bit data size not allowed
-  12 [ ]*calll \*\(%r8\) # 32-bit data size not allowed
-  13 [ ]*calll \*\(%rax\) # 32-bit data size not allowed
-  14 [ ]*callq \*\(%ax\) # 32-bit data size not allowed
-  15 [ ]*callw \*\(%ax\) # no 16-bit addressing
-  16 [ ]*daa # illegal
-  17 [ ]*das # illegal
-  18 [ ]*enterl \$0,\$0 # can't have 32-bit stack operands
-  19 [ ]*into # illegal
-  20 [ ]*foo:[ ]*jcxz foo # No prefix exists to select CX as a counter
-  21 [ ]*jmpl \*%eax # 32-bit data size not allowed
-  22 [ ]*jmpl \*\(%rax\) # 32-bit data size not allowed
-  23 [ ]*lcalll \$0,\$0 # illegal
-  24 [ ]*lcallq \$0,\$0 # illegal
-  25 [ ]*ldsl %eax,\(%rax\) # illegal
-  26 [ ]*ldsq %rax,\(%rax\) # illegal
-  27 [ ]*lesl %eax,\(%rax\) # illegal
-  28 [ ]*lesq %rax,\(%rax\) # illegal
-  29 [ ]*ljmpl \$0,\$0 # illegal
-  30 [ ]*ljmpq \$0,\$0 # illegal
-  31 [ ]*ljmpq \*\(%rax\) # 64-bit data size not allowed
-  32 [ ]*loopw foo # No prefix exists to select CX as a counter
-  33 [ ]*loopew foo # No prefix exists to select CX as a counter
-  34 [ ]*loopnew foo # No prefix exists to select CX as a counter
-  35 [ ]*loopnzw foo # No prefix exists to select CX as a counter
-  36 [ ]*loopzw foo # No prefix exists to select CX as a counter
-  37 [ ]*leavel # can't have 32-bit stack operands
-  38 [ ]*pop %ds # illegal
-  39 [ ]*pop %es # illegal
-  40 [ ]*pop %ss # illegal
-  41 [ ]*popa # illegal
-  42 [ ]*popl %eax # can't have 32-bit stack operands
-  43 [ ]*push %cs # illegal
-  44 [ ]*push %ds # illegal
-  45 [ ]*push %es # illegal
-  46 [ ]*push %ss # illegal
-  47 [ ]*pusha # illegal
-  48 [ ]*pushl %eax # can't have 32-bit stack operands
-  49 [ ]*pushfl # can't have 32-bit stack operands
-  50 [ ]*popfl # can't have 32-bit stack operands
-  51 [ ]*retl # can't have 32-bit stack operands
-  52 [ ]*insertq \$4,\$2,%xmm2,%ebx # The last operand must be XMM register.
-  53 [ ]*.intel_syntax noprefix
-  54 [ ]*cmpxchg16b dword ptr \[rax\] # Must be oword
+[ ]*1[ ]+\.text
+[ ]*2[ ]+\# All the following should be illegal for x86-64
+[ ]*3[ ]+aaa \# illegal
+[ ]*4[ ]+aad \# illegal
+[ ]*5[ ]+aam \# illegal
+[ ]*6[ ]+aas \# illegal
+[ ]*7[ ]+arpl %ax,%ax \# illegal
+[ ]*8[ ]+bound %eax,\(%rax\) \# illegal
+[ ]*9[ ]+calll \*%eax \# 32-bit data size not allowed
+[ ]*10[ ]+calll \*\(%ax\) \# 32-bit data size not allowed
+[ ]*11[ ]+calll \*\(%eax\) \# 32-bit data size not allowed
+[ ]*12[ ]+calll \*\(%r8\) \# 32-bit data size not allowed
+[ ]*13[ ]+calll \*\(%rax\) \# 32-bit data size not allowed
+[ ]*14[ ]+callq \*\(%ax\) \# 32-bit data size not allowed
+[ ]*15[ ]+callw \*\(%ax\) \# no 16-bit addressing
+[ ]*16[ ]+daa \# illegal
+[ ]*17[ ]+das \# illegal
+[ ]*18[ ]+enterl \$0,\$0 \# can't have 32-bit stack operands
+[ ]*19[ ]+into \# illegal
+[ ]*20[ ]+foo: jcxz foo \# No prefix exists to select CX as a counter
+[ ]*21[ ]+jmpl \*%eax \# 32-bit data size not allowed
+[ ]*22[ ]+jmpl \*\(%rax\) \# 32-bit data size not allowed
+[ ]*23[ ]+lcalll \$0,\$0 \# illegal
+[ ]*24[ ]+lcallq \$0,\$0 \# illegal
+[ ]*25[ ]+ldsl %eax,\(%rax\) \# illegal
+[ ]*26[ ]+ldsq %rax,\(%rax\) \# illegal
+[ ]*27[ ]+lesl %eax,\(%rax\) \# illegal
+[ ]*28[ ]+lesq %rax,\(%rax\) \# illegal
+[ ]*29[ ]+ljmpl \$0,\$0 \# illegal
+[ ]*30[ ]+ljmpq \$0,\$0 \# illegal
+[ ]*31[ ]+ljmpq \*\(%rax\) \# 64-bit data size not allowed
+[ ]*32[ ]+loopw foo \# No prefix exists to select CX as a counter
+[ ]*33[ ]+loopew foo \# No prefix exists to select CX as a counter
+[ ]*34[ ]+loopnew foo \# No prefix exists to select CX as a counter
+[ ]*35[ ]+loopnzw foo \# No prefix exists to select CX as a counter
+[ ]*36[ ]+loopzw foo \# No prefix exists to select CX as a counter
+[ ]*37[ ]+leavel \# can't have 32-bit stack operands
+[ ]*38[ ]+pop %ds \# illegal
+[ ]*39[ ]+pop %es \# illegal
+[ ]*40[ ]+pop %ss \# illegal
+[ ]*41[ ]+popa \# illegal
+[ ]*42[ ]+popl %eax \# can't have 32-bit stack operands
+[ ]*43[ ]+push %cs \# illegal
+[ ]*44[ ]+push %ds \# illegal
+[ ]*45[ ]+push %es \# illegal
+[ ]*46[ ]+push %ss \# illegal
+[ ]*47[ ]+pusha \# illegal
+[ ]*48[ ]+pushl %eax \# can't have 32-bit stack operands
+[ ]*49[ ]+pushfl \# can't have 32-bit stack operands
+[ ]*50[ ]+popfl \# can't have 32-bit stack operands
+[ ]*51[ ]+retl \# can't have 32-bit stack operands
+[ ]*52[ ]+insertq \$4,\$2,%xmm2,%ebx \# The last operand must be XMM register\.
+[ ]*53[ ]+\.intel_syntax noprefix
+[ ]*54[ ]+cmpxchg16b dword ptr \[rax\] \# Must be oword
+[ ]*55[ ]+movq xmm1, XMMWORD PTR \[rsp\]
+[ ]*56[ ]+movq xmm1, DWORD PTR \[rsp\]
+[ ]*57[ ]+movq xmm1, WORD PTR \[rsp\]
+ GAS LISTING .*
+
+
+[ ]*58[ ]+movq xmm1, BYTE PTR \[rsp\]
+[ ]*59[ ]+movq XMMWORD PTR \[rsp\],xmm1
+[ ]*60[ ]+movq DWORD PTR \[rsp\],xmm1
+[ ]*61[ ]+movq WORD PTR \[rsp\],xmm1
+[ ]*62[ ]+movq BYTE PTR \[rsp\],xmm1
--- binutils/gas/testsuite/gas/i386/x86-64-inval.s.ptr 2006-12-15 05:22:44.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/x86-64-inval.s 2008-01-02 12:03:13.000000000 -0800
@@ -52,3 +52,11 @@ foo: jcxz foo # No prefix exists to sele
  insertq $4,$2,%xmm2,%ebx # The last operand must be XMM register.
  .intel_syntax noprefix
  cmpxchg16b dword ptr [rax] # Must be oword
+ movq xmm1, XMMWORD PTR [rsp]
+ movq xmm1, DWORD PTR [rsp]
+ movq xmm1, WORD PTR [rsp]
+ movq xmm1, BYTE PTR [rsp]
+ movq XMMWORD PTR [rsp],xmm1
+ movq DWORD PTR [rsp],xmm1
+ movq WORD PTR [rsp],xmm1
+ movq BYTE PTR [rsp],xmm1
--- binutils/opcodes/i386-gen.c.ptr 2007-12-28 11:43:06.000000000 -0800
+++ binutils/opcodes/i386-gen.c 2008-01-02 11:06:11.000000000 -0800
@@ -274,6 +274,13 @@ static bitfield opcode_modifiers[] =
   BITFIELD (No_sSuf),
   BITFIELD (No_qSuf),
   BITFIELD (No_ldSuf),
+  BITFIELD (No_xSuf),
+  BITFIELD (CheckSize),
+  BITFIELD (Byte),
+  BITFIELD (Word),
+  BITFIELD (Dword),
+  BITFIELD (QWord),
+  BITFIELD (Xmmword),
   BITFIELD (FWait),
   BITFIELD (IsString),
   BITFIELD (RegKludge),
--- binutils/opcodes/i386-opc.h.ptr 2007-12-28 08:59:03.000000000 -0800
+++ binutils/opcodes/i386-opc.h 2008-01-02 11:06:53.000000000 -0800
@@ -191,8 +191,22 @@ typedef union i386_cpu_flags
 #define No_qSuf (No_sSuf + 1)
 /* long double suffix on instruction illegal */
 #define No_ldSuf (No_qSuf + 1)
+/* x suffix on instruction illegal */
+#define No_xSuf (No_ldSuf + 1)
+/* check PTR size on instruction */
+#define CheckSize (No_xSuf + 1)
+/* BYTE PTR on instruction */
+#define Byte (CheckSize + 1)
+/* WORD PTR on instruction */
+#define Word (Byte + 1)
+/* DWORD PTR on instruction */
+#define Dword (Word + 1)
+/* QWORD PTR on instruction */
+#define QWord (Dword + 1)
+/* XMMWORD PTR on instruction */
+#define Xmmword (QWord + 1)
 /* instruction needs FWAIT */
-#define FWait (No_ldSuf + 1)
+#define FWait (Xmmword + 1)
 /* quick test for string instructions */
 #define IsString (FWait + 1)
 /* fake an extra reg operand for clr, imul and special register
@@ -256,6 +270,13 @@ typedef struct i386_opcode_modifier
   unsigned int no_ssuf:1;
   unsigned int no_qsuf:1;
   unsigned int no_ldsuf:1;
+  unsigned int no_xsuf:1;
+  unsigned int checksize:1;
+  unsigned int byte:1;
+  unsigned int word:1;
+  unsigned int dword:1;
+  unsigned int qword:1;
+  unsigned int xmmword:1;
   unsigned int fwait:1;
   unsigned int isstring:1;
   unsigned int regkludge:1;
--- binutils/opcodes/i386-opc.tbl.ptr 2007-12-31 10:53:13.000000000 -0800
+++ binutils/opcodes/i386-opc.tbl 2008-01-02 11:00:16.000000000 -0800
@@ -907,14 +907,14 @@ movd, 2, 0x660f6e, None, 2, CpuSSE2, Mod
 movd, 2, 0x660f7e, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM, Reg32|Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
 // In the 64bit mode the short form mov immediate is redefined to have
 // 64bit displacement value.
-movq, 2, 0xf6f, None, 2, CpuMMX, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegMMX, RegMMX }
-movq, 2, 0xf7f, None, 2, CpuMMX, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { RegMMX, BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegMMX }
-movq, 2, 0xf30f7e, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM, RegXMM }
-movq, 2, 0x660fd6, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { RegXMM, BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM }
-movq, 2, 0xf6e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S, RegMMX }
-movq, 2, 0xf7e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegMMX, Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
-movq, 2, 0x660f6e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S, RegXMM }
-movq, 2, 0x660f7e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM, Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
+movq, 2, 0xf6f, None, 2, CpuMMX, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord|NoRex64, { BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegMMX, RegMMX }
+movq, 2, 0xf7f, None, 2, CpuMMX, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord|NoRex64, { RegMMX, BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegMMX }
+movq, 2, 0xf30f7e, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord|NoRex64, { BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM, RegXMM }
+movq, 2, 0x660fd6, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord|NoRex64, { RegXMM, BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM }
+movq, 2, 0xf6e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|CheckSize|QWord|No_ldSuf, { Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S, RegMMX }
+movq, 2, 0xf7e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord, { RegMMX, Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
+movq, 2, 0x660f6e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord, { Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S, RegXMM }
+movq, 2, 0x660f7e, None, 2, Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|CheckSize|QWord, { RegXMM, Reg64|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
 // We put the 64bit displacement first and we only mark constants
 // larger than 32bit as Disp64.
 movq, 2, 0xa0, None, 1, Cpu64, D|W|Size64|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp64, Acc }

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly in Intel syntax

H.J. Lu-27
On Fri, Jan 04, 2008 at 08:09:44AM +0000, Jan Beulich wrote:
> While I agree on the subject, I slightly disagree on the approach you took: The added flags shouldn't go on the instructions, but on their operands (otherwise you'll likely end up creating more special case code namely for movzx/movsx, but perhaps also elsewhere): Just like for registers, memory operands should properly specify what sizes are acceptable (basically, operand type and operand size should probably be decoupled). Jan

Another problem is suffix.  I don't like using "suffix" for both
mnemonic suffix in AT&T/Intel modes and operand size in Intel mode.
I like your suggestion. We now have enough bits on operand to do it.
But it is a major work and it can't be done one instruction at a time.



H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly in Intel syntax

H.J. Lu-27
On Fri, Jan 04, 2008 at 06:28:20AM -0800, H.J. Lu wrote:
> On Fri, Jan 04, 2008 at 08:09:44AM +0000, Jan Beulich wrote:
> > While I agree on the subject, I slightly disagree on the approach you took: The added flags shouldn't go on the instructions, but on their operands (otherwise you'll likely end up creating more special case code namely for movzx/movsx, but perhaps also elsewhere): Just like for registers, memory operands should properly specify what sizes are acceptable (basically, operand type and operand size should probably be decoupled). Jan
>
> Another problem is suffix.  I don't like using "suffix" for both
> mnemonic suffix in AT&T/Intel modes and operand size in Intel mode.
> I like your suggestion. We now have enough bits on operand to do it.
> But it is a major work and it can't be done one instruction at a time.
>

Here is the initial patch. Any comments?

Thanks.

H.J.
---
gas/testsuite/

2008-01-10  H.J. Lu  <[hidden email]>

        PR gas/5534
        * gas/i386/inval.s: Add tests for fnstsw and fstsw.
        * gas/i386/i386.s: Likewise.
        * gas/i386/x86_64.s: Likewise.

        * gas/i386/x86-64-inval.s: Add tests for fnstsw, fstsw, in
        and out.

        * gas/i386/prefix.s: Remove invalid fstsw.

        * gas/i386/inval.l: Updated.
        * gas/i386/i386.d: Likewise.
        * gas/i386/x86_64.d: Likewise.
        * gas/i386/x86-64-inval.l: Likewise.
        * gas/i386/prefix.d: Updated.

gas/

2008-01-10  H.J. Lu  <[hidden email]>

        PR gas/5534
        * config/tc-i386.c (_i386_insn): Update comment.
        (operand_type_register_match): Likewise.
        (match_template): Check memory and accumulator operand size.
        (intel_e09): Set operand size for "XXX PTR".

opcodes/

2008-01-10  H.J. Lu  <[hidden email]>

        PR gas/5534
        * i386-gen.c (operand_type_init): Add Dword to
        OPERAND_TYPE_ACC32.  Add Qword to OPERAND_TYPE_ACC64.
        (opcode_modifiers): Remove CheckSize, Byte, Word, Dword,
        Qword and Xmmword.
        (operand_types): Add Byte, Word, Dword, Qword, Xmmword and
        Unspecified.

        * i386-opc.h (CheckSize): Removed.
        (Byte): Updated.
        (Word): Likewise.
        (Dword): Likewise.
        (Qword): Likewise.
        (Xmmword): Likewise.
        (FWait): Updated.
        (OTMax): Likewise.
        (i386_opcode_modifier): Remove checksize, byte, word, dword,
        qword and xmmword.
        (Unspecified): New.
        (i386_operand_type): Add checksize, byte, word, dword, qword,
        xmmword and unspecified.

        * i386-opc.tbl: Updated to use Byte, Word, Dword, Qword,
        Xmmword and Unspecified.

        * i386-reg.tbl: Add size for accumulator.

        * i386-init.h: Regenerated.
        * i386-tbl.h: Likewise.

binutils-size-1.diff.bz2 (42K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly in Intelsyntax

H.J. Lu-27
On Fri, Jan 11, 2008 at 10:42:22AM +0000, Jan Beulich wrote:

> >>> "H.J. Lu" <[hidden email]> 11.01.08 01:03 >>>
> >On Fri, Jan 04, 2008 at 06:28:20AM -0800, H.J. Lu wrote:
> >> On Fri, Jan 04, 2008 at 08:09:44AM +0000, Jan Beulich wrote:
> >> > While I agree on the subject, I slightly disagree on the approach you took: The added flags shouldn't go on the instructions, but
> >on their operands (otherwise you'll likely end up creating more special case code namely for movzx/movsx, but perhaps also
> >elsewhere): Just like for registers, memory operands should properly specify what sizes are acceptable (basically, operand type
> >and operand size should probably be decoupled). Jan
> >>
> >> Another problem is suffix.  I don't like using "suffix" for both
> >> mnemonic suffix in AT&T/Intel modes and operand size in Intel mode.
> >> I like your suggestion. We now have enough bits on operand to do it.
> >> But it is a major work and it can't be done one instruction at a time.
> >>
> >
> >Here is the initial patch. Any comments?
>
> I first thought I understand what Unspecified means, but since it's being
> used on movzx/movsx as well as instructions where the respective
> operand's size cannot be derived from another operand, I don't think I
> really understand the concept.
>
Unspecified means size can be unspecified. AnySize means any size
is ok. Here is the updated patch. Now it reports error on all lines
in intelbad.s.



H.J.
----
gas/testsuite/

2008-01-11  H.J. Lu  <[hidden email]>

        PR gas/5534
        * gas/i386/i386.s: Add tests for fnstsw and fstsw.
        * gas/i386/inval.s: Likewise.
        * gas/i386/x86_64.s: Likewise.

        * gas/i386/intel.s: Use word instead of dword on ss.

        * gas/i386/x86-64-inval.s: Add tests for fnstsw, fstsw, in
        and out.

        * gas/i386/prefix.s: Remove invalid fstsw.

        * gas/i386/inval.l: Updated.
        * gas/i386/intelbad.l: Likewise.
        * gas/i386/i386.d: Likewise.
        * gas/i386/x86_64.d: Likewise.
        * gas/i386/x86-64-inval.l: Likewise.
        * gas/i386/prefix.d: Updated.

gas/

2008-01-11  H.J. Lu  <[hidden email]>

        PR gas/5534
        * config/tc-i386.c (_i386_insn): Update comment.
        (operand_type_match): Also clear unspecified.
        (operand_type_register_match): Likewise.
        (parse_operands): Initialize unspecified.
        (i386_intel_operand): Likewise.
        (match_template): Check memory and accumulator operand size.
        (i386_att_operand): Clear unspecified on register operand.
        (intel_e11): Likewise.
        (intel_e09): Set operand size and clean unspecified for
        "XXX PTR".

opcodes/

2008-01-11  H.J. Lu  <[hidden email]>

        PR gas/5534
        * i386-gen.c (operand_type_init): Add Dword to
        OPERAND_TYPE_ACC32.  Add Qword to OPERAND_TYPE_ACC64.
        (opcode_modifiers): Remove CheckSize, Byte, Word, Dword,
        Qword and Xmmword.
        (operand_types): Add Byte, Word, Dword, Fword, Qword, Tbyte,
        Xmmword, Unspecified and Anysize.
        (set_bitfield): Make Mmword an alias of Qword.  Make Oword
        an alias of Xmmword.

        * i386-opc.h (CheckSize): Removed.
        (Byte): Updated.
        (Word): Likewise.
        (Dword): Likewise.
        (Qword): Likewise.
        (Xmmword): Likewise.
        (FWait): Updated.
        (OTMax): Likewise.
        (i386_opcode_modifier): Remove checksize, byte, word, dword,
        qword and xmmword.
        (Fword): New.
        (TBYTE): Likewise.
        (Unspecified): Likewise.
        (Anysize): Likewise.
        (i386_operand_type): Add byte, word, dword, fword, qword,
        tbyte xmmword, unspecified and anysize.

        * i386-opc.tbl: Updated to use Byte, Word, Dword, Fword, Qword,
        Tbyte, Xmmword, Unspecified and Anysize.

        * i386-reg.tbl: Add size for accumulator.

        * i386-init.h: Regenerated.
        * i386-tbl.h: Likewise.

binutils-size-2.diff.bz2 (44K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly inIntelsyntax

Jan Beulich
>Unspecified means size can be unspecified. AnySize means any size
>is ok.

But you continue to have Unspecified in places where a size specification
is a requirement (at least in Intel syntax; I never got any clarification on
whether AT&T syntax silently enforcing a certain size if none was
specified is really intended behavior).

>Here is the updated patch. Now it reports error on all lines
>in intelbad.s.

Looks fine, and errors on all lines of intelbad.s was always the goal.

Jan

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properly inIntelsyntax

H.J. Lu-27
On Mon, Jan 14, 2008 at 08:59:29AM +0000, Jan Beulich wrote:
> >Unspecified means size can be unspecified. AnySize means any size
> >is ok.
>
> But you continue to have Unspecified in places where a size specification
> is a requirement (at least in Intel syntax; I never got any clarification on
> whether AT&T syntax silently enforcing a certain size if none was
> specified is really intended behavior).
>

As you mentioned above, since for some instructions, the old assembler
selected a default size even in Intel mode when size isn't provided, I
used Unspecified on them. Otherwise, the new assembler will be
incompatible with the old one. I will enforce the operand size on
new instructions for both Intel and AT&T modes when more than 1
size are available.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properlyinIntelsyntax

Jan Beulich
>>> "H.J. Lu" <[hidden email]> 14.01.08 14:55 >>>
>On Mon, Jan 14, 2008 at 08:59:29AM +0000, Jan Beulich wrote:
>> >Unspecified means size can be unspecified. AnySize means any size
>> >is ok.
>>
>> But you continue to have Unspecified in places where a size specification
>> is a requirement (at least in Intel syntax; I never got any clarification on
>> whether AT&T syntax silently enforcing a certain size if none was
>> specified is really intended behavior).
>>
>
>As you mentioned above, since for some instructions, the old assembler
>selected a default size even in Intel mode when size isn't provided, I

I don't think I said anything like that. Quite a while back, I made ambiguous
operand sizes an error in Intel mode, and I'm not aware that I missed any
cases.

>used Unspecified on them. Otherwise, the new assembler will be
>incompatible with the old one. I will enforce the operand size on

I think it should (in AT&T mode) at least warn in that case (providing the
option to make this an error in a couple years time); I'm asking to make
this an error unconditionally in Intel mode (as it's an error in MASM).

>new instructions for both Intel and AT&T modes when more than 1
>size are available.

Jan

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checked properlyinIntelsyntax

H.J. Lu-27
On Mon, Jan 14, 2008 at 02:17:05PM +0000, Jan Beulich wrote:

> >>> "H.J. Lu" <[hidden email]> 14.01.08 14:55 >>>
> >On Mon, Jan 14, 2008 at 08:59:29AM +0000, Jan Beulich wrote:
> >> >Unspecified means size can be unspecified. AnySize means any size
> >> >is ok.
> >>
> >> But you continue to have Unspecified in places where a size specification
> >> is a requirement (at least in Intel syntax; I never got any clarification on
> >> whether AT&T syntax silently enforcing a certain size if none was
> >> specified is really intended behavior).
> >>
> >
> >As you mentioned above, since for some instructions, the old assembler
> >selected a default size even in Intel mode when size isn't provided, I
>
> I don't think I said anything like that. Quite a while back, I made ambiguous
> operand sizes an error in Intel mode, and I'm not aware that I missed any
> cases.

There are 2 cases when Unspecified is used with more than one sizes:

1. Only one size is allowed given the second operand, like

movw (%eax), %ax
movl (%eax), %eax
mov ax, WORD PTR [ebx]
mov eax, DWORD PTR [ebx]

2. 64bit extension adds a new size and we don't want to disallow the 32bit
default version:

bash-3.2$ cat x.s
        .intel_syntax noprefix
cvtsi2sd xmm1, [rax]
cvtsi2sd xmm1,DWORD PTR [rax]
cvtsi2sd xmm1,QWORD PTR [rax]
bash-3.2$ /usr/bin/as -o x.o x.s
bash-3.2$ objdump -dw x.o

x.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <.text>:
   0:   f2 0f 2a 08             cvtsi2sdl (%rax),%xmm1
   4:   f2 0f 2a 08             cvtsi2sdl (%rax),%xmm1
   8:   f2 48 0f 2a 08          cvtsi2sdq (%rax),%xmm1
bash-3.2$

If you can find a case where Unspecified is used with more than 1 sizes
in a different way, please let me know.

>
> >used Unspecified on them. Otherwise, the new assembler will be
> >incompatible with the old one. I will enforce the operand size on
>
> I think it should (in AT&T mode) at least warn in that case (providing the
> option to make this an error in a couple years time); I'm asking to make
> this an error unconditionally in Intel mode (as it's an error in MASM).
>

I will generate an error in AT&T mode if there is no such an instruction.
I disallowed "fnstsw %eax".  I don't want to issue an warning for
default size in AT&T mode since it is well understood and may generate
too many warnings.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checkedproperlyinIntelsyntax

Jan Beulich
>There are 2 cases when Unspecified is used with more than one sizes:
>
>1. Only one size is allowed given the second operand, like
>
>movw (%eax), %ax
>movl (%eax), %eax
>mov ax, WORD PTR [ebx]
>mov eax, DWORD PTR [ebx]
>
>2. 64bit extension adds a new size and we don't want to disallow the 32bit
>default version:
>
>bash-3.2$ cat x.s
>        .intel_syntax noprefix
>cvtsi2sd xmm1, [rax]
>cvtsi2sd xmm1,DWORD PTR [rax]
>cvtsi2sd xmm1,QWORD PTR [rax]

I don't think this is desirable - if 64bit specifies two sizes, programmers
should be explicit in selecting them. I'm pretty certain MASM enforces
this, too, so again I'd want it to work that way at least in Intel mode.

>If you can find a case where Unspecified is used with more than 1 sizes
>in a different way, please let me know.

- base opcode 0xC6 (the immediate doesn't allow deriving a size)
- movsx
- movzx
- push (base opcode 0xFF)
- pop (base opcode 0x8F)
- base opcodes 0x80 and 0x83
- inc/dec (base opcode 0xFE)

etc (I stopped here, as the problem seems to be too wide-spread). The
only exception that I can see being useful are operations that act on the
stack (push, pop, call) as well as branches through a memory operand:
defaulting these to the native size may be useful, but strictly speaking
even there the programmer ought to be required to specify an explicit
size (and again I think MASM enforces this).

Jan

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn't checkedproperlyinIntelsyntax

H.J. Lu-27
On Mon, Jan 14, 2008 at 03:54:13PM +0000, Jan Beulich wrote:

> >2. 64bit extension adds a new size and we don't want to disallow the 32bit
> >default version:
> >
> >bash-3.2$ cat x.s
> >        .intel_syntax noprefix
> >cvtsi2sd xmm1, [rax]
> >cvtsi2sd xmm1,DWORD PTR [rax]
> >cvtsi2sd xmm1,QWORD PTR [rax]
>
> I don't think this is desirable - if 64bit specifies two sizes, programmers
> should be explicit in selecting them. I'm pretty certain MASM enforces
> this, too, so again I'd want it to work that way at least in Intel mode.

That is not a problem. I will enforce it.

>
> >If you can find a case where Unspecified is used with more than 1 sizes
> >in a different way, please let me know.
>
> - base opcode 0xC6 (the immediate doesn't allow deriving a size)
> - movsx
> - movzx
> - push (base opcode 0xFF)
> - pop (base opcode 0x8F)
> - base opcodes 0x80 and 0x83
> - inc/dec (base opcode 0xFE)

I didn't see it at least for movsx,

movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg16|Reg32|Reg64 }
movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg32|Reg64 }
movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg64 }

One one size and Unspecified are allowed. What is wrong with that?



H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn'tcheckedproperlyinIntelsyntax

Jan Beulich
>> >If you can find a case where Unspecified is used with more than 1 sizes
>> >in a different way, please let me know.
>>
>> - base opcode 0xC6 (the immediate doesn't allow deriving a size)
>> - movsx
>> - movzx
>> - push (base opcode 0xFF)
>> - pop (base opcode 0x8F)
>> - base opcodes 0x80 and 0x83
>> - inc/dec (base opcode 0xFE)
>
>I didn't see it at least for movsx,
>
>movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg16|Reg32|Reg64 }
>movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg32|Reg64 }
>movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg64 }
>
>One one size and Unspecified are allowed. What is wrong with that?

The fact this (according to your description) allows

        movsx eax, [eax]

which is ambiguous.

Jan

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn'tcheckedproperlyinIntelsyntax

H.J. Lu-27
On Mon, Jan 14, 2008 at 04:41:04PM +0000, Jan Beulich wrote:

> >> >If you can find a case where Unspecified is used with more than 1 sizes
> >> >in a different way, please let me know.
> >>
> >> - base opcode 0xC6 (the immediate doesn't allow deriving a size)
> >> - movsx
> >> - movzx
> >> - push (base opcode 0xFF)
> >> - pop (base opcode 0x8F)
> >> - base opcodes 0x80 and 0x83
> >> - inc/dec (base opcode 0xFE)
> >
> >I didn't see it at least for movsx,
> >
> >movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg16|Reg32|Reg64 }
> >movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg32|Reg64 }
> >movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg64 }
> >
> >One one size and Unspecified are allowed. What is wrong with that?
>
> The fact this (according to your description) allows
>
> movsx eax, [eax]
>
> which is ambiguous.

This is checked by process_suffix. I am planning to clean up
process_suffix. Then we will need to update the opcode table. I
will see what I can do in the meantime. In any case, I will
leave AT&T syntax alone and enforce size for Intel mode.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR gas/5534: "XXX PTR" isn'tcheckedproperlyinIntelsyntax

H.J. Lu-27
On Mon, Jan 14, 2008 at 10:16:19AM -0800, H.J. Lu wrote:

> On Mon, Jan 14, 2008 at 04:41:04PM +0000, Jan Beulich wrote:
> > >> >If you can find a case where Unspecified is used with more than 1 sizes
> > >> >in a different way, please let me know.
> > >>
> > >> - base opcode 0xC6 (the immediate doesn't allow deriving a size)
> > >> - movsx
> > >> - movzx
> > >> - push (base opcode 0xFF)
> > >> - pop (base opcode 0x8F)
> > >> - base opcodes 0x80 and 0x83
> > >> - inc/dec (base opcode 0xFE)
> > >
> > >I didn't see it at least for movsx,
> > >
> > >movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg16|Reg32|Reg64 }
> > >movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg32|Reg64 }
> > >movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg64 }
> > >
> > >One one size and Unspecified are allowed. What is wrong with that?
> >
> > The fact this (according to your description) allows
> >
> > movsx eax, [eax]
> >
> > which is ambiguous.
>
> This is checked by process_suffix. I am planning to clean up
> process_suffix. Then we will need to update the opcode table. I
> will see what I can do in the meantime. In any case, I will
> leave AT&T syntax alone and enforce size for Intel mode.
>
>
This is the patch I checked in. I left most of Unspecified alone
since I don't want to break the existing code in Intel syntax.
Please feel free to remove those Unspecified with more than one
size. But please make sure that AT&T syntax still works.

The x86 assembler needs some cleanup. My change should help
the cleanup process.


H.J.
---
gas/

2008-01-14  H.J. Lu  <[hidden email]>

        * config/tc-i386.c (match_reg_size): New.
        (match_mem_size): Likewise.
        (operand_size_match): Likewise.
        (operand_type_match): Also clear all size fields.
        (match_template): Skip Intel syntax when in AT&T syntax.
        Call operand_size_match to check operand size.

        (i386_att_operand): Set the mem field to 1 for memory
        operand.
        (i386_intel_operand): Likewise.

gas/testsuite/

2008-01-14  H.J. Lu  <[hidden email]>

        * gas/i386/i386.s: Add tests for movsx, movzx and movnti.
        * gas/i386/inval.s: Likewise.
        * gas/i386/x86_64.s: Likewise.
        * gas/i386/x86-64-inval.s: Likewise.

        * gas/i386/i386.d: Updated.
        * gas/i386/inval.l: Likewise.
        * gas/i386/x86_64.d: Likewise.
        * gas/i386/x86-64-inval.l: Likewise.

opcodes/

2008-01-14  H.J. Lu  <[hidden email]>

        * i386-gen.c (opcode_modifiers): Add IntelSyntax.
        (operand_types): Add Mem.

        * i386-opc.h (IntelSyntax): New.
        * i386-opc.h (Mem): New.
        (Byte): Updated.
        (Opcode_Modifier_Max): Updated.
        (i386_opcode_modifier): Add intelsyntax.
        (i386_operand_type): Add mem.

        * i386-opc.tbl: Remove Reg16 from movnti.  Add sizes to more
        instructions.

        * i386-reg.tbl: Add size for accumulator.

        * i386-init.h: Regenerated.
        * i386-tbl.h: Likewise.



binutils-mem-2.diff.bz2 (26K) Download Attachment