|
From: Bryan M. <om...@br...> - 2006-02-17 23:34:59
|
Chaps,
finally got some time to work on Omega again and found what stops it
from working with optimised code (debug code is fine for some reason):
Index: priv/guest-x86/toIR.c
===================================================================
--- priv/guest-x86/toIR.c (revision 1574)
+++ priv/guest-x86/toIR.c (working copy)
@@ -11330,8 +11330,8 @@
t1 = newTemp(szToITy(sz)); t2 = newTemp(Ity_I32);
assign(t2, getIReg(4, R_ESP));
assign(t1, loadLE(szToITy(sz),mkexpr(t2)));
+ putIReg(sz, opc-0x58, mkexpr(t1));
putIReg(4, R_ESP, binop(Iop_Add32, mkexpr(t2), mkU32(sz)));
- putIReg(sz, opc-0x58, mkexpr(t1));
DIP("pop%c %s\n", nameISize(sz), nameIReg(sz,opc-0x58));
break;
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
Is this an acceptable change? Is there another way of doing this? Does
this break anyone else?
The reason I need this for omega is that the value is read from the
stack but the stack content is invalidated (causing the "last pointer"
that was on the stack to die, generating a false leak report) before the
"last pointer" becomes visible in a client register. This way around,
the "last pointer" gets a new reference (a client register) before the
old one on the stack dies.
I would also need this for the other architectures but only have x86 to
play on at home and won't be at work until Monday.
Comments welcome!
Bryan "Brain Murders" Meredith
|
|
From: Bryan M. <om...@br...> - 2006-02-18 00:22:48
|
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.
>
|