Callback Function

[RotateRight]

Maybe an extra set of eyes might help me
solve this.

Posted below is what I wanted to be a callback
function.  I am using it while enumerating
child windows.  Most of this is generated and
is a disassembly.  But forgiving the FPU
instructions, here's my problem.

The value 1.0 is stored in memory.
The idea was to have this value returned in eax.
So far so good.  The problem is that ESI is
getting "clobbered" on the last POP.  When
that happens, windows "bombs".

Any ideas on what might cause the bad ESI?

Code Select Expand


004031BB  /. 55             PUSH EBP
004031BC  |. 89E5           MOV EBP,ESP
004031BE  |. 53             PUSH EBX
004031BF  |. 57             PUSH EDI
004031C0  |. 56             PUSH ESI
004031C1  |. 81EC 04000000  SUB ESP,4
004031C7  |. FF35 FAB34000  PUSH DWORD PTR DS:[40B3FA]
004031CD  |. FF35 F6B34000  PUSH DWORD PTR DS:[40B3F6]
004031D3  |. 8F05 65B04000  POP DWORD PTR DS:[40B065]
004031D9  |. 8F05 69B04000  POP DWORD PTR DS:[40B069]
004031DF  |. FF35 69B04000  PUSH DWORD PTR DS:[40B069]
004031E5  |. FF35 65B04000  PUSH DWORD PTR DS:[40B065]
004031EB  |. E8 764E0300    CALL <JMP.&msvcrt.floor>
004031F0  |. 81C4 08000000  ADD ESP,8
004031F6  |. DB1D 65B04000  FISTP DWORD PTR DS:[40B065]
004031FC  |. FF35 65B04000  PUSH DWORD PTR DS:[40B065]
00403202  |. 8F05 5DB04000  POP DWORD PTR DS:[40B05D]
00403208  |. 9B             WAIT
00403209  |. DBE3           FINIT
0040320B  |. DB05 5DB04000  FILD DWORD PTR DS:[40B05D]
00403211  |. DB55 FC        FIST DWORD PTR SS:[EBP-4]
00403214  |. 8B45 FC        MOV EAX,DWORD PTR SS:[EBP-4]
00403217  |. 5E             POP ESI  <-- Different than the Push.
00403218  |. 5F             POP EDI
00403219  |. 5B             POP EBX
0040321A  |. 89EC           MOV ESP,EBP
0040321C  |. 5D             POP EBP
0040321D  \. C2 0800        RETN 8


[hutch--]

A quick look says the pushes match the pops so it should be a balanced stack. It looks like C code disassembled, if its asm code, could you post the code so we know what the DATA section references are ?

[RotateRight]

It seems balanced to me as well.
I watched the esp value as the
call to floor went though.  It
remained unchanged and the next
instruction adjusted by eight for
the previous two pushes.

Much thanks for taking a look!
The code came from the KoolB
Basic compiler.

Here's the generated code.
I ask forgiveness in that it's
not Masm.

Code Select Expand


global _SETCHILDFONTS
_SETCHILDFONTS:
PUSH EBP
MOV EBP,ESP
PUSH EBX
PUSH EDI
PUSH ESI
SUB ESP,4
PUSH dword[Number_4+4]
PUSH dword[Number_4]
POP dword[TempQWord2]
POP dword[TempQWord2+4]
ccall floor,dword[TempQWord2],dword[TempQWord2+4]
FISTP dword[TempQWord2]
PUSH dword[TempQWord2]
POP dword[TempQWord1]FINIT
FILD dword[TempQWord1]
FIST dword[EBP-4]
MOV EAX,dword[EBP-4]
POP ESI
POP EDI
POP EBX
MOV ESP,EBP
POP EBP
RET 8


And the data section.

Code Select Expand


section .data
Number_4 dq 1.0
TempQWord1 dq 0.0
TempQWord2 dq 0.0


[Mark Jones]

Let's look at this in terms of relative stack pointer:

Code Select Expand


004031BB  /. 55             PUSH EBP                    ; esp-4
004031BC  |. 89E5           MOV EBP,ESP
004031BE  |. 53             PUSH EBX                    ; esp-8
004031BF  |. 57             PUSH EDI                    ; esp-12
004031C0  |. 56             PUSH ESI                    ; esp-16
004031C1  |. 81EC 04000000  SUB ESP,4                   ; esp-20
004031C7  |. FF35 FAB34000  PUSH DWORD PTR DS:[40B3FA]  ; esp-24
004031CD  |. FF35 F6B34000  PUSH DWORD PTR DS:[40B3F6]  ; esp-28
004031D3  |. 8F05 65B04000  POP DWORD PTR DS:[40B065]   ; esp-24
004031D9  |. 8F05 69B04000  POP DWORD PTR DS:[40B069]   ; esp-20
004031DF  |. FF35 69B04000  PUSH DWORD PTR DS:[40B069]  ; esp-24
004031E5  |. FF35 65B04000  PUSH DWORD PTR DS:[40B065]  ; esp-28
004031EB  |. E8 764E0300    CALL <JMP.&msvcrt.floor>
004031F0  |. 81C4 08000000  ADD ESP,8                   ; esp-20
004031F6  |. DB1D 65B04000  FISTP DWORD PTR DS:[40B065]
004031FC  |. FF35 65B04000  PUSH DWORD PTR DS:[40B065]  ; esp-24
00403202  |. 8F05 5DB04000  POP DWORD PTR DS:[40B05D]   ; esp-20
00403208  |. 9B             WAIT
00403209  |. DBE3           FINIT
0040320B  |. DB05 5DB04000  FILD DWORD PTR DS:[40B05D]
00403211  |. DB55 FC        FIST DWORD PTR SS:[EBP-4]
00403214  |. 8B45 FC        MOV EAX,DWORD PTR SS:[EBP-4]
00403217  |. 5E             POP ESI                     ; esp-16
00403218  |. 5F             POP EDI                     ; esp-12
00403219  |. 5B             POP EBX                     ; esp-8
0040321A  |. 89EC           MOV ESP,EBP
0040321C  |. 5D             POP EBP                     ; esp-4
0040321D  \. C2 0800        RETN 8


Hmm... have you tried putting a breakpoint on the call to MSVCRT and step over it, then check to make sure the value at ESP-4 hasn't been modified? (and not just ESP.) Maybe there is a bug with the call? The rest of the code is so basic it doesn't seem like that could be the problem.

[RotateRight]

I think the balancing act from Hutch and Mark
is getting me closer to a solution.

The SUB ESP,4 was allocation for the return variable
called "Result".

Function SetChildFonts(hControl As Integer, lParam As Integer) As Integer
'SendMessage(....
  Result = 1
End Function

After getting the return value in EAX,
I added the line ADD ESP,4

MOV EAX,dword[EBP-4]
ADD ESP,4
POP ESI
POP EDI
POP EBX
MOV ESP,EBP
POP EBP
RET 8

Things seem much better.

Any comments are welcome as to if this is
correct.  A better way?   

Thanks Guys.

Mark

[MichaelW]

The

SUB ESP, 4

Is not in the normal location. If it is allocating space for a local variable it would normally, immediately follow the:

MOV EBP, ESP

That way it would not affect the stack operations that follow, and the:

MOV ESP, EBP

At the end would effectively remove the local variable from the stack.

[zooba]

I agree with Michael's last comment. The SUB ESP,4 is not balanced before the registers are popped. It appears that it should be immediately after the MOV EBP,ESP, since it is being used as a local variable.

Cheers,

Zooba :U

[RotateRight]

Thanks for everyone's response.

I spent alot of time on this one.
This has definitely been a balancing act.
Thanks Hutch.

Mark, you made my Ollydbg skills much better.
(So that's what the little box in the bottom
right corner is for. :bg)

Although the ADD worked, Michael's approach
has got to be the better method.

And with a vote from zooba, I know which way
to go.

Perhaps this may have a little to do with why
languages ask that "local variables" be declared
(or dim) at the top of functions.

You could learn "heaps" hanging out with you guys.

Thanks again,

Mark

[zooba]

Perhaps this may have a little to do with why
languages ask that "local variables" be declared
(or dim) at the top of functions.

It does, in that it saves time scanning the function to see how much space is required. Newer/higher-level languages scan the entire function, but still allocate the space at the start.

Cheers,

Zooba :U