Re: [Valgrind-developers] Omega - found my problem...

[Bryan M. <om...@br...>]

John,

good spot - it does need a check now to see if the popped to register is 
the ESP.

thanks.

Index: priv/guest-x86/toIR.c
===================================================================
--- priv/guest-x86/toIR.c       (revision 1574)
+++ priv/guest-x86/toIR.c       (working copy)
@@ -11330,8 +11330,9 @@
        t1 = newTemp(szToITy(sz)); t2 = newTemp(Ity_I32);
        assign(t2, getIReg(4, R_ESP));
        assign(t1, loadLE(szToITy(sz),mkexpr(t2)));
-      putIReg(4, R_ESP, binop(Iop_Add32, mkexpr(t2), mkU32(sz)));
        putIReg(sz, opc-0x58, mkexpr(t1));
+      if(opc != 0x5C)
+       putIReg(4, R_ESP, binop(Iop_Add32, mkexpr(t2), mkU32(sz)));
        DIP("pop%c %s\n", nameISize(sz), nameIReg(sz,opc-0x58));
        break;

Bryan


John Reiser wrote:
>> That little patch reverses the order of assignments to sim cpu registers
>> so that when you do a pop this happens:
>>
>> ESP -> temp2
>> popped value -> temp1
>> temp1 -> sim cpu register
>> (calc new stack value from temp 2) -> ESP
>>
>> instead of:
>>
>> ESP -> temp2
>> popped value -> temp1
>> (calc new stack value from temp 2) -> ESP
>> temp1 -> sim cpu register
> 
> The special case to check is popping the stack pointer itself:
> -----foo.S
> _start: .globl _start
>         nop; int3
>         pushl $0
>         popl %esp
>         nop
>         int3
> -----
> $ gcc -o foo -nostarfiles -nostdlib foo.S
> $ gdb foo
> (gdb)run
> (gdb) x/i $pc
> 0x8048076 <_start+2>:   push   $0x0
> (gdb) stepi
> 0x08048078 in _start ()
> 0x8048078 <_start+4>:   pop    %esp
> (gdb) p $esp
> $1 = (void *) 0xbf9bed1c
> (gdb) stepi
> 0x08048079 in _start ()
> 0x8048079 <_start+5>:   nop
> (gdb) p $esp
> $2 = (void *) 0x0  ## not 4 + 0xbf9bed1c
> 
> So the final value of %esp is the contents that was popped from memory,
> not (4 + old_value).  It looks like the code change introduced a bug.
>