This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
Re: PATCH: Fix i386 disassembler with index == 0x4 in SIB (Re: objdump bug-report)
- From: "H. J. Lu" <hjl at lucon dot org>
- To: "Allan B. Cruse" <cruse at cs dot usfca dot edu>,binutils at sources dot redhat dot com, gcc at gcc dot gnu dot org,GNU C Library <libc-alpha at sources dot redhat dot com>
- Date: Fri, 14 Jan 2005 13:48:14 -0800
- Subject: Re: PATCH: Fix i386 disassembler with index == 0x4 in SIB (Re: objdump bug-report)
- References: <20050111210753.0C8CB219E0@nexus.cs.usfca.edu> <20050112191052.GA12463@lucon.org> <20050113034440.GG30985@bubble.modra.org> <20050113170849.GA30644@lucon.org> <20050114000528.GA3408@bubble.modra.org> <20050114002659.GA4491@lucon.org> <20050114005918.GD3408@bubble.modra.org>
On Fri, Jan 14, 2005 at 11:29:18AM +1030, Alan Modra wrote:
> On Thu, Jan 13, 2005 at 04:26:59PM -0800, H. J. Lu wrote:
> > If it is an optimization, there shouldn't be a warning.
>
> No, whether we warn or not is an entirely separate matter to whether we
> optimize.
>
> > I think it
> > may be useful to turn "leal 0xf(%eax,1), %eax" into "8d 44 20 0f"
> > Gcc/ld use
> >
> > leal foo(%reg), %eax; call ___tls_get_addr; nop
> >
> > today for TLS optimization. With the change, we can use
> >
> > leal foo(%reg,1), %eax; call ___tls_get_addr;
>
> Hmm. So that you generate a larger instruction on purpose? Wanted for
> the space needed with some of the tls transformations, I expect.
>
> OK, that is a valid reason to support encoding of the instruction
> that way. You still should warn for scale factors other than 1,
> because it's easy to forget the comma in (,%reg,2) where you really
> do want the register to be scaled.
>
> > Then it should display
> >
> > 8b 04 23 mov (%ebx,1),%eax
>
I decided to use (%ebx,,1) for this and adjusted assembler to accept
it. You will get warnings for (%ebx,1) as before. It will be easier
to check if assembler takes (%ebx,,1).
BTW, I didn't change the Intel syntax since I don't know enough
about it.
H.J.
-----
gas/
2005-01-14 H.J. Lu <hongjiu.lu@intel.com>
PR 658
* config/tc-i386.c (SCALE1_WHEN_NO_INDEX): Removed.
(_i386_insn): Add empty_index_reg.
(build_modrm_byte): Use SIB if empty_index_reg is not 0.
(i386_scale): Don't warn scale factor without index register if
empty_index_reg is not 0.
(i386_operand): Set empty_index_reg 1 if the index register is
"".
gas/testsuite/
2005-01-14 H.J. Lu <hongjiu.lu@intel.com>
PR 658
* gas/i386/sib.d: Updated.
* gas/i386/sib.s: Likewise.
* gas/i386/ssemmx2.d: Likewise.
ld/testsuite/
2005-01-14 H.J. Lu <hongjiu.lu@intel.com>
PR 658
* ld-i386/tlsbin.dd: Updated.
opcodes/
2005-01-14 H.J. Lu <hongjiu.lu@intel.com>
PR 658
* 386-dis.c (OP_E): Undo the 2005-01-12 change. Display scale
for SIB with INDEX == 4.
--- binutils/gas/config/tc-i386.c.sib 2005-01-14 11:27:06.000000000 -0800
+++ binutils/gas/config/tc-i386.c 2005-01-14 12:25:45.624584751 -0800
@@ -43,14 +43,6 @@
#define INFER_ADDR_PREFIX 1
#endif
-#ifndef SCALE1_WHEN_NO_INDEX
-/* Specifying a scale factor besides 1 when there is no index is
- futile. eg. `mov (%ebx,2),%al' does exactly the same as
- `mov (%ebx),%al'. To slavishly follow what the programmer
- specified, set SCALE1_WHEN_NO_INDEX to 0. */
-#define SCALE1_WHEN_NO_INDEX 1
-#endif
-
#ifndef DEFAULT_ARCH
#define DEFAULT_ARCH "i386"
#endif
@@ -162,6 +154,8 @@ struct _i386_insn
const reg_entry *index_reg;
unsigned int log2_scale_factor;
+ int empty_index_reg;
+
/* SEG gives the seg_entries of this insn. They are zero unless
explicit segment overrides are given. */
const seg_entry *seg[2];
@@ -3006,11 +3000,9 @@ build_modrm_byte ()
Any base register besides %esp will not use the
extra modrm byte. */
i.sib.index = NO_INDEX_REGISTER;
-#if !SCALE1_WHEN_NO_INDEX
/* Another case where we force the second modrm byte. */
- if (i.log2_scale_factor)
+ if (i.empty_index_reg)
i.rm.regmem = ESCAPE_TO_TWO_BYTE_ADDRESSING;
-#endif
}
else
{
@@ -3970,13 +3962,13 @@ i386_scale (scale)
input_line_pointer = save;
return NULL;
}
- if (i.log2_scale_factor != 0 && i.index_reg == 0)
+ if (i.log2_scale_factor != 0
+ && i.index_reg == 0
+ && i.empty_index_reg == 0)
{
as_warn (_("scale factor of %d without an index register"),
1 << i.log2_scale_factor);
-#if SCALE1_WHEN_NO_INDEX
i.log2_scale_factor = 0;
-#endif
}
scale = input_line_pointer;
input_line_pointer = save;
@@ -4430,6 +4422,12 @@ i386_operand (operand_string)
as_bad (_("bad register name `%s'"), base_string);
return 0;
}
+ else if (*base_string == ',' && i.base_reg)
+ {
+ /* Check for empty index reg. */
+ base_string++;
+ i.empty_index_reg = 1;
+ }
/* Check for scale factor. */
if (*base_string != ')')
--- binutils/gas/testsuite/gas/i386/sib.d.sib 2005-01-12 11:12:51.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/sib.d 2005-01-14 12:43:29.131911163 -0800
@@ -6,10 +6,14 @@
Disassembly of section .text:
0+000 <foo>:
- 0: 8b 04 23 [ ]*mov [ ]*\(%ebx\),%eax
- 3: 8b 04 63 [ ]*mov [ ]*\(%ebx\),%eax
- 6: 8b 04 a3 [ ]*mov [ ]*\(%ebx\),%eax
- 9: 8b 04 e3 [ ]*mov [ ]*\(%ebx\),%eax
- c: 90 [ ]*nop [ ]*
- d: 90 [ ]*nop [ ]*
- ...
+ 0: 8b 03 [ ]*mov [ ]*\(%ebx\),%eax
+ 2: 8b 04 23 [ ]*mov [ ]*\(%ebx,,1\),%eax
+ 5: 8b 04 63 [ ]*mov [ ]*\(%ebx,,2\),%eax
+ 8: 8b 04 a3 [ ]*mov [ ]*\(%ebx,,4\),%eax
+ b: 8b 04 e3 [ ]*mov [ ]*\(%ebx,,8\),%eax
+ e: 8b 04 24 [ ]*mov [ ]*\(%esp\),%eax
+ 11: 8b 04 24 [ ]*mov [ ]*\(%esp\),%eax
+ 14: 8b 04 64 [ ]*mov [ ]*\(%esp,,2\),%eax
+ 17: 8b 04 a4 [ ]*mov [ ]*\(%esp,,4\),%eax
+ 1a: 8b 04 e4 [ ]*mov [ ]*\(%esp,,8\),%eax
+ 1d: 8d 76 00 [ ]*lea [ ]*0x0\(%esi\),%esi
--- binutils/gas/testsuite/gas/i386/sib.s.sib 2005-01-12 11:12:51.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/sib.s 2005-01-14 13:33:54.696308591 -0800
@@ -2,10 +2,14 @@
.text
foo:
- .byte 0x8B, 0x04, 0x23 # effect is: movl (%ebx), %eax
- .byte 0x8B, 0x04, 0x63 # effect is: movl (%ebx), %eax
- .byte 0x8B, 0x04, 0xA3 # effect is: movl (%ebx), %eax
- .byte 0x8B, 0x04, 0xE3 # effect is: movl (%ebx), %eax
- nop
- nop
- .p2align 4,0
+ mov (%ebx),%eax
+ mov (%ebx,,1),%eax
+ mov (%ebx,,2),%eax
+ mov (%ebx,,4),%eax
+ mov (%ebx,,8),%eax
+ mov (%esp),%eax
+ mov (%esp,,1),%eax
+ mov (%esp,,2),%eax
+ mov (%esp,,4),%eax
+ mov (%esp,,8),%eax
+ .p2align 4
--- binutils/gas/testsuite/gas/i386/ssemmx2.d.sib 2004-01-18 15:13:35.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/ssemmx2.d 2005-01-14 11:11:57.000000000 -0800
@@ -85,4 +85,4 @@ Disassembly of section .text:
1f1: 66 0f fc 90 90 90 90 90 paddb[ ]+0x90909090\(%eax\),%xmm2
1f9: 66 0f fd 90 90 90 90 90 paddw[ ]+0x90909090\(%eax\),%xmm2
201: 66 0f fe 90 90 90 90 90 paddd[ ]+0x90909090\(%eax\),%xmm2
- 209: 8d b4 26 00 00 00 00 lea[ ]+0x0\(%esi\),%esi
+ 209: 8d b4 26 00 00 00 00 lea[ ]+0x0\(%esi,,1\),%esi
--- binutils/ld/testsuite/ld-i386/tlsbin.dd.sib 2004-05-11 10:08:36.000000000 -0700
+++ binutils/ld/testsuite/ld-i386/tlsbin.dd 2005-01-14 11:14:25.000000000 -0800
@@ -92,7 +92,7 @@ Disassembly of section .text:
# LD -> LE
8049085: 65 a1 00 00 00 00[ ]+mov %gs:0x0,%eax
804908b: 90[ ]+nop *
- 804908c: 8d 74 26 00[ ]+lea 0x0\(%esi\),%esi
+ 804908c: 8d 74 26 00[ ]+lea 0x0\(%esi,,1\),%esi
8049090: 90[ ]+nop *
8049091: 90[ ]+nop *
8049092: 8d 90 20 f0 ff ff[ ]+lea 0xfffff020\(%eax\),%edx
@@ -108,7 +108,7 @@ Disassembly of section .text:
# LD -> LE against hidden variables
80490a4: 65 a1 00 00 00 00[ ]+mov %gs:0x0,%eax
80490aa: 90[ ]+nop *
- 80490ab: 8d 74 26 00[ ]+lea 0x0\(%esi\),%esi
+ 80490ab: 8d 74 26 00[ ]+lea 0x0\(%esi,,1\),%esi
80490af: 90[ ]+nop *
80490b0: 90[ ]+nop *
80490b1: 8d 90 40 f0 ff ff[ ]+lea 0xfffff040\(%eax\),%edx
--- binutils/opcodes/i386-dis.c.sib 2005-01-13 09:41:31.000000000 -0800
+++ binutils/opcodes/i386-dis.c 2005-01-14 13:33:42.625890827 -0800
@@ -3191,10 +3191,8 @@ OP_E (int bytemode, int sizeflag)
{
havesib = 1;
FETCH_DATA (the_info, codep + 1);
+ scale = (*codep >> 6) & 3;
index = (*codep >> 3) & 7;
- if (mode_64bit || index != 0x4)
- /* When INDEX == 0x4 in 32 bit mode, SCALE is ignored. */
- scale = (*codep >> 6) & 3;
base = *codep & 7;
USED_REX (REX_EXTY);
USED_REX (REX_EXTZ);
@@ -3316,7 +3314,20 @@ OP_E (int bytemode, int sizeflag)
oappend (mode_64bit && (sizeflag & AFLAG)
? names64[index] : names32[index]);
}
- if (scale != 0 || (!intel_syntax && index != 4))
+ else if (!intel_syntax
+ && havebase
+ && (scale != 0
+ || ((base & 7) != 4
+ && (base & 7) != 5)))
+ {
+ *obufp++ = separator_char;
+ *obufp = '\0';
+ }
+ if (scale != 0
+ || (!intel_syntax && index != 4)
+ || (index == 4
+ && (base & 7) != 4
+ && (base & 7) != 5))
{
*obufp++ = scale_char;
*obufp = '\0';