[Qemu-devel] Final call for help: x86: enter instruction bug

[Stefan Kisdaroczi]

Hi all,

i would like to use qemu on a x86 in System Emulation mode, simulating a x86 
system. Thats all.
My System, which runs on a real x86, with Bochs and with Virtual PC without 
Problems, does crash in some Apps (throwing Exceptions) with qemu.

The first try i gave qemu in august:
  It crashed on OS-Initialization, but using other configurations, I could log 
in, and start an App. Next Crash. It looked like there are two Bugs,
  one with the Bound instruction and one with the LES instruction [1].

After seeing that Fabrice did fix the Bound-Instruction[2], i gave qemu a 
second change in october:
  The OS now started with the Default-Configuration, and I could log in, but 
all my Apps are still crashing.
  After the Bound-Fix, it seemed that the second Problem is not a bug with LES, 
it is a bug with ENTER (Stack-Values and ESP) [3].

I made a small program with the same compiler as the other apps to reproduce 
the bug. An it crashed.
My prog uses enter 2,1 , enter 2,2 and enter 2,3 ( enter has two parameters : 
esp_addend,level )
The bug depends on the level parameter, if level is 0, everything is fine.

I singlestepped the program under a real x86 and qemu. I have noted ESP and 
EBP, and all values on the Stack before each ENTER, and after each ENTER.
Already after the first enter, ESP was wrong, and there was value too much on 
the Stack.

I made this patch [5], applied to the source [4] and tested again. ENTER x,1 
was now working as expected, ESP ok, Values on Stack ok.
But the program still crashed after enter x,2. ESP was ok after the patch, but 
a value on the Stack was wrong, same with enter x,3.

Finally, im sure the bug is generated by the marked line :
        while (level--) {
            gen_op_addl_A0_im(-opsize);
            gen_op_addl_T0_im(-opsize);
***         gen_op_st_T0_A0[ot + s->mem_index]();
        }

With enter x,3 the stack should be generated like this :

push ebp
push [ebp+opsize]
push [ebp+(2*opsize)]
push [ebp+(3*opsize)]
push ebp

But qemu makes this :

push ebp
push ebp+opsize
push ebp+(2*opsize)
push ebp+(3*opsize)
push ebp

--> It should copy the values from the old stack-frame, but qemu "pushes" the 
adresses.

I tried to fix it myself, but how to copy from stack to stack ? I looked at 
other instructions how to do, T0 and T1 are already used, what else can I use ?
Finally I didnt succeed to make a one-line-patch, so I hoped that someone can 
help me[3].

I invested some time to debug this, and I will invest time to check it, I still 
have my test-app, but im stuck here. So please. Can someone help ?????

The enter instruction seems to be seldom used with levels > 0, but it depends 
on the compiler and the structure of the program, so I am pretty sure that there 
are
crashes in other apps caused by this bug.

I cant send you a Test-App/Image, because you dont have this OS, and I cant 
send it to you. ( OS and Compiler are from Intel and not free )

Merci beaucoup and thank you very much for reading until here ;-)

greetings kisda


[1] http://lists.gnu.org/archive/html/qemu-devel/2004-08/msg00140.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2004-08/msg00220.html
[3] http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00162.html
[4] :
static void gen_enter(DisasContext *s, int esp_addend, int level)
{
    int ot, level1, addend, opsize;

    ot = s->dflag + OT_WORD;
    level &= 0x1f;
    level1 = level;
    opsize = 2 << s->dflag;

    gen_op_movl_A0_ESP();
    gen_op_addl_A0_im(-opsize);
    if (!s->ss32)
        gen_op_andl_A0_ffff();
    gen_op_movl_T1_A0();
    if (s->addseg)
        gen_op_addl_A0_seg(offsetof(CPUX86State,segs[R_SS].base));
    /* push bp */
    gen_op_mov_TN_reg[OT_LONG][0][R_EBP]();
    gen_op_st_T0_A0[ot + s->mem_index]();
    if (level) {
        while (level--) {
            gen_op_addl_A0_im(-opsize);
            gen_op_addl_T0_im(-opsize);
            gen_op_st_T0_A0[ot + s->mem_index]();
        }
        gen_op_addl_A0_im(-opsize);
        gen_op_st_T1_A0[ot + s->mem_index]();
    }
    gen_op_mov_reg_T1[ot][R_EBP]();
    addend = -esp_addend;
    if (level1)
        addend -= opsize * (level1 + 1);
    gen_op_addl_T1_im(addend);
    gen_op_mov_reg_T1[ot][R_ESP]();
}

[5] :
--- enter.old   2004-10-29 19:27:20.000000000 +0200
+++ enter.new   2004-10-29 19:27:46.000000000 +0200
@@ -18,7 +18,7 @@ static void gen_enter(DisasContext *s, i
     gen_op_mov_TN_reg[OT_LONG][0][R_EBP]();
     gen_op_st_T0_A0[ot + s->mem_index]();
     if (level) {
-        while (level--) {
+        while (--level) {
             gen_op_addl_A0_im(-opsize);
             gen_op_addl_T0_im(-opsize);
             gen_op_st_T0_A0[ot + s->mem_index]();
@@ -29,7 +29,7 @@ static void gen_enter(DisasContext *s, i
     gen_op_mov_reg_T1[ot][R_EBP]();
     addend = -esp_addend;
     if (level1)
-        addend -= opsize * (level1 + 1);
+        addend -= opsize * level1;
     gen_op_addl_T1_im(addend);
     gen_op_mov_reg_T1[ot][R_ESP]();
 }