Make manual size overrides on effective addresses only work if legal;

otherwise the override is ignored and a warning is output, so correct
code is always generated.

Reported by: vclaudepierre@tiscali.fr
Bugzilla Bug: 40

* x86expr.c (x86_checkea_calc_displen): Output warnings for noreg case;
reorder such that displacement length calculation is done if the warning
is output.
* x86bc.c (x86_bc_insn_resolve): If length was forced but checkea had to
override it, save it always (to avoid multiple warnings).

* addrop.hex: Match corrected code generation.
* addrop.errwarn: Include new warning message.

* ea-warn.asm, ea-warn.errwarn, ea-warn.hex: Test new functionality more
in depth (code thanks to bug reporter).

svn path=/trunk/yasm/; revision=1165

diff --git a/modules/arch/x86/tests/Makefile.inc b/modules/arch/x86/tests/Makefile.inc
index 2355abbbf404e9ce16ce88c643e645ba18939ab5..19e1b99dc7e0b007ec2102115530d36b82f798d8 100644 (file)
--- a/modules/arch/x86/tests/Makefile.inc
+++ b/modules/arch/x86/tests/Makefile.inc
@@ -24,6 +24,9 @@ EXTRA_DIST += modules/arch/x86/tests/div-err.errwarn
 EXTRA_DIST += modules/arch/x86/tests/ea-over.asm
 EXTRA_DIST += modules/arch/x86/tests/ea-over.errwarn
 EXTRA_DIST += modules/arch/x86/tests/ea-over.hex
+EXTRA_DIST += modules/arch/x86/tests/ea-warn.asm
+EXTRA_DIST += modules/arch/x86/tests/ea-warn.errwarn
+EXTRA_DIST += modules/arch/x86/tests/ea-warn.hex
 EXTRA_DIST += modules/arch/x86/tests/ebpindex.asm
 EXTRA_DIST += modules/arch/x86/tests/ebpindex.errwarn
 EXTRA_DIST += modules/arch/x86/tests/ebpindex.hex

diff --git a/modules/arch/x86/tests/addrop.errwarn b/modules/arch/x86/tests/addrop.errwarn
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..6bccc3a212154d9353fb7d8e864ea0ec873f549f 100644 (file)
--- a/modules/arch/x86/tests/addrop.errwarn
+++ b/modules/arch/x86/tests/addrop.errwarn
@@ -0,0 +1 @@
+-:22: warning: invalid displacement size; fixed

diff --git a/modules/arch/x86/tests/addrop.hex b/modules/arch/x86/tests/addrop.hex
index fb47944fcff2449e62c0c0a5a03bfdeefeb57b3b..da61c8737bb162b2ab14a6a7b1af94b399bd3b4b 100644 (file)
--- a/modules/arch/x86/tests/addrop.hex
+++ b/modules/arch/x86/tests/addrop.hex
@@ -76,6 +76,7 @@ f7
 f7 
 3e 
 05 
+00 
 26 
 67 
 f7 

diff --git a/modules/arch/x86/tests/ea-warn.asm b/modules/arch/x86/tests/ea-warn.asm
new file mode 100644 (file)
index 0000000..62e9444
--- /dev/null
+++ b/modules/arch/x86/tests/ea-warn.asm
@@ -0,0 +1,18 @@
+[bits 32]
+add [byte  ebp*8+06h],ecx ;db 01,0c,0ed,06 probably wrong
+dd 90909090h
+add [dword ebp*8+06h],ecx ;db 01,0c,0ed,06,0,0,0  OK
+dd 90909090h
+add ecx,[byte  ebp*8+06h] ;db 03,0c,0ed,06 probably wrong
+dd 90909090h
+add ecx,[dword ebp*8+06h]
+dd 90909090h
+add ecx,[ebp*8+06h]
+dd 90909090h
+add ecx,[byte ebx*8+06h]  ;db 03,0c,0dd,06 probably wrong
+dd 90909090h
+add ecx,[dword ebx*8+06h]
+dd 90909090h
+add ecx,[ebx*8+06h]
+dd 90909090h
+

diff --git a/modules/arch/x86/tests/ea-warn.errwarn b/modules/arch/x86/tests/ea-warn.errwarn
new file mode 100644 (file)
index 0000000..d805738
--- /dev/null
+++ b/modules/arch/x86/tests/ea-warn.errwarn
@@ -0,0 +1,3 @@
+-:2: warning: invalid displacement size; fixed
+-:6: warning: invalid displacement size; fixed
+-:12: warning: invalid displacement size; fixed

diff --git a/modules/arch/x86/tests/ea-warn.hex b/modules/arch/x86/tests/ea-warn.hex
new file mode 100644 (file)
index 0000000..b9c2b02
--- /dev/null
+++ b/modules/arch/x86/tests/ea-warn.hex
@@ -0,0 +1,88 @@
+01 
+0c 
+ed 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+01 
+0c 
+ed 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+ed 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+ed 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+ed 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+dd 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+dd 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 
+03 
+0c 
+dd 
+06 
+00 
+00 
+00 
+90 
+90 
+90 
+90 

diff --git a/modules/arch/x86/x86bc.c b/modules/arch/x86/x86bc.c
index 34dc54943f6b4e401c2eae42dbe7e41d6fb74a95..02439fc7e12eaa83a7f4071fa3082e4663d41388 100644 (file)
--- a/modules/arch/x86/x86bc.c
+++ b/modules/arch/x86/x86bc.c
@@ -698,6 +698,10 @@ x86_bc_insn_resolve(yasm_bytecode *bc, int save,
                    displen = (insn->addrsize == 16) ? 2U : 4U;
            }
 
+           /* If we had forced ea->len but had to override, save it now */
+           if (ea->len != 0 && ea->len != displen)
+               ea->len = displen;
+
            if (save) {
                *x86_ea = eat;  /* structure copy */
                ea->len = displen;

diff --git a/modules/arch/x86/x86expr.c b/modules/arch/x86/x86expr.c
index f37bf56fc4b74e9f97a6fcf0efd39417741b3c12..4bea638bcd502c8c0304e9678064a31003aafef2 100644 (file)
--- a/modules/arch/x86/x86expr.c
+++ b/modules/arch/x86/x86expr.c
@@ -419,86 +419,17 @@ x86_checkea_calc_displen(yasm_expr **ep, unsigned int wordsize, int noreg,
 
     switch (*displen) {
        case 0:
-           /* the displacement length hasn't been forced, try to
-            * determine what it is.
-            */
-           if (noreg) {
-               /* no register in ModRM expression, so it must be disp16/32,
-                * and as the Mod bits are set to 0 by the caller, we're done
-                * with the ModRM byte.
-                */
-               *displen = wordsize;
-               *v_modrm = 1;
-               break;
-           } else if (dispreq) {
-               /* for BP/EBP, there *must* be a displacement value, but we
-                * may not know the size (8 or 16/32) for sure right now.
-                * We can't leave displen at 0, because that just means
-                * unknown displacement, including none.
-                */
-               *displen = 0xff;
-           }
-
-           intn = yasm_expr_get_intnum(ep, NULL);
-           if (!intn) {
-               /* expr still has unknown values: assume 16/32-bit disp */
-               *displen = wordsize;
-               *modrm |= 0200;
-               *v_modrm = 1;
-               break;
-           }   
-
-           /* don't try to find out what size displacement we have if
-            * displen is known.
-            */
-           if (*displen != 0 && *displen != 0xff) {
-               if (*displen == 1)
-                   *modrm |= 0100;
-               else
-                   *modrm |= 0200;
-               *v_modrm = 1;
-               break;
-           }
-
-           dispval = yasm_intnum_get_int(intn);
-
-           /* Figure out what size displacement we will have. */
-           if (*displen != 0xff && dispval == 0) {
-               /* if we know that the displacement is 0 right now,
-                * go ahead and delete the expr (making it so no
-                * displacement value is included in the output).
-                * The Mod bits of ModRM are set to 0 above, and
-                * we're done with the ModRM byte!
-                *
-                * Don't do this if we came from dispreq check above, so
-                * check *displen.
-                */
-               yasm_expr_destroy(e);
-               *ep = (yasm_expr *)NULL;
-           } else if (dispval >= -128 && dispval <= 127) {
-               /* It fits into a signed byte */
-               *displen = 1;
-               *modrm |= 0100;
-           } else {
-               /* It's a 16/32-bit displacement */
-               *displen = wordsize;
-               *modrm |= 0200;
-           }
-           *v_modrm = 1;       /* We're done with ModRM */
-
            break;
-
        /* If not 0, the displacement length was forced; set the Mod bits
-        * appropriately and we're done with the ModRM byte.  We assume
-        * that the user knows what they're doing if they do an explicit
-        * override, so we don't check for overflow (we'll just truncate
-        * when we output).
+        * appropriately and we're done with the ModRM byte.
         */
        case 1:
-           /* TODO: Add optional warning here about byte not being valid
-            * override in noreg case.
-            */
-           if (!noreg)
+           /* Byte is not valid override in noreg case; fix it. */
+           if (noreg) {
+               *displen = 0;
+               yasm__warning(YASM_WARN_GENERAL, e->line,
+                             N_("invalid displacement size; fixed"));
+           } else
                *modrm |= 0100;
            *v_modrm = 1;
            break;
@@ -509,10 +440,13 @@ x86_checkea_calc_displen(yasm_expr **ep, unsigned int wordsize, int noreg,
                    N_("invalid effective address (displacement size)"));
                return 1;
            }
-           /* TODO: Add optional warning here about 2/4 not being valid
-            * override in noreg case.
-            */
-           if (!noreg)
+           /* 2/4 is not valid override in noreg case; fix it. */
+           if (noreg) {
+               if (wordsize != *displen)
+                   yasm__warning(YASM_WARN_GENERAL, e->line,
+                                 N_("invalid displacement size; fixed"));
+               *displen = 0;
+           } else
                *modrm |= 0200;
            *v_modrm = 1;
            break;
@@ -521,6 +455,75 @@ x86_checkea_calc_displen(yasm_expr **ep, unsigned int wordsize, int noreg,
            yasm_internal_error(N_("strange EA displacement size"));
     }
 
+    if (*displen == 0) {
+       /* the displacement length hasn't been forced (or the forcing wasn't
+        * valid), try to determine what it is.
+        */
+       if (noreg) {
+           /* no register in ModRM expression, so it must be disp16/32,
+            * and as the Mod bits are set to 0 by the caller, we're done
+            * with the ModRM byte.
+            */
+           *displen = wordsize;
+           *v_modrm = 1;
+           return 0;
+       } else if (dispreq) {
+           /* for BP/EBP, there *must* be a displacement value, but we
+            * may not know the size (8 or 16/32) for sure right now.
+            * We can't leave displen at 0, because that just means
+            * unknown displacement, including none.
+            */
+           *displen = 0xff;
+       }
+
+       intn = yasm_expr_get_intnum(ep, NULL);
+       if (!intn) {
+           /* expr still has unknown values: assume 16/32-bit disp */
+           *displen = wordsize;
+           *modrm |= 0200;
+           *v_modrm = 1;
+           return 0;
+       }       
+
+       /* don't try to find out what size displacement we have if
+        * displen is known.
+        */
+       if (*displen != 0 && *displen != 0xff) {
+           if (*displen == 1)
+               *modrm |= 0100;
+           else
+               *modrm |= 0200;
+           *v_modrm = 1;
+           return 0;
+       }
+
+       dispval = yasm_intnum_get_int(intn);
+
+       /* Figure out what size displacement we will have. */
+       if (*displen != 0xff && dispval == 0) {
+           /* if we know that the displacement is 0 right now,
+            * go ahead and delete the expr (making it so no
+            * displacement value is included in the output).
+            * The Mod bits of ModRM are set to 0 above, and
+            * we're done with the ModRM byte!
+            *
+            * Don't do this if we came from dispreq check above, so
+            * check *displen.
+            */
+           yasm_expr_destroy(e);
+           *ep = (yasm_expr *)NULL;
+       } else if (dispval >= -128 && dispval <= 127) {
+           /* It fits into a signed byte */
+           *displen = 1;
+           *modrm |= 0100;
+       } else {
+           /* It's a 16/32-bit displacement */
+           *displen = wordsize;
+           *modrm |= 0200;
+       }
+       *v_modrm = 1;   /* We're done with ModRM */
+    }
+
     return 0;
 }
 /*@=nullstate@*/