Always add the segment base when an override prefix is specified in segmented
mode. Otherwise the generated address is calculated incorrectly when an override
prefix is specified and HF_ADDSEG_MASK is not set.

Fixes: 55a15eb3504 ("target/i386/tcg: simplify effective address calculation")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/i386/tcg/translate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


Paolo/Richard: I'm not sure if this patch is exactly correct, however it fixes
a regression when launching the OpenWatcom installer under MS-DOS. Current
git master aborts the installer with a GPF error in the guest caused by this
instruction:

0x0041e0b5:  66 2e 8e 1d 0c 00 00 00  movw     %cs:0xc, %ds

where def_seg == 3, ovr_seg == 1 and ADDSEG(s) is 0. Without this fix the target
address is calculated as 0xc instead of 0x41e00c.
 
 
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 192cf43215e..323beec656b 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -600,12 +600,15 @@ static void gen_lea_v_seg_dest(DisasContext *s, MemOp aflag, TCGv dest, TCGv a0,
 {
     int easize;
     bool has_base;
+    bool addseg = ADDSEG(s);
 
     if (ovr_seg < 0) {
         ovr_seg = def_seg;
+    } else if (!CODE64(s)) {
+        addseg = true;
     }
 
-    has_base = ovr_seg >= R_FS || (ovr_seg >= 0 && ADDSEG(s));
+    has_base = ovr_seg >= R_FS || (ovr_seg >= 0 && addseg);
     easize = CODE64(s) ? MO_64 : MO_32;
 
     if (has_base) {
-- 
2.47.3

On 9/26/26 00:27, Mark Cave-Ayland wrote:
> Always add the segment base when an override prefix is specified in segmented
> mode. Otherwise the generated address is calculated incorrectly when an override
> prefix is specified and HF_ADDSEG_MASK is not set.
> 
> Fixes: 55a15eb3504 ("target/i386/tcg: simplify effective address calculation")
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/i386/tcg/translate.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 
> Paolo/Richard: I'm not sure if this patch is exactly correct, however it fixes
> a regression when launching the OpenWatcom installer under MS-DOS. Current
> git master aborts the installer with a GPF error in the guest caused by this
> instruction:
> 
> 0x0041e0b5:  66 2e 8e 1d 0c 00 00 00  movw     %cs:0xc, %ds
> 
> where def_seg == 3, ovr_seg == 1 and ADDSEG(s) is 0. Without this fix the target
> address is calculated as 0xc instead of 0x41e00c.

It isn't the root cause, no.  Here we go:

x86_update_hflags:
>             hflags |= ((env->segs[R_DS].base | env->segs[R_ES].base |
>                         env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT;

cpu_x86_load_seg_cache:
>             new_hflags |= ((env->segs[R_DS].base |
>                             env->segs[R_ES].base |
>                             env->segs[R_SS].base) != 0) <<
>                 HF_ADDSEG_SHIFT;

So we've been incorrectly including CS in ADDSEG since 55a15eb3504.

Thus

-    has_base = ovr_seg >= R_FS || (ovr_seg >= 0 && ADDSEG(s));
     switch (ovr_seg) {
     case R_DS:
     case R_ES:
     case R_SS:
         has_base = ADDSEG(s);
         break;
     case R_CS:
     case R_FS:
     case R_GS:
         has_base = true;
         break;
     default:
         assert(ovr_seg < 0);
         has_base = false;
         break;
     }

might be the most legible way to fix the above expression.


r~

Or better,

>      switch (ovr_seg) {
>      case R_DS:
>      case R_ES:
>      case R_SS:
>          has_base = ADDSEG(s);
>          break;
>      case R_CS:

            has_base = !CODE64(s);
            break;

>      case R_FS:
>      case R_GS:
>          has_base = true;
>          break;
>      default:
>          assert(ovr_seg < 0);
>          has_base = false;
>          break;
>      }

r~