Can anyone tell me what I'm doing wrong here/what causes the crash?

What I'm trying to do:
Push a 0 or a 1 onto the stack depending on which radio button is checked. Then in the Convert proc pop that value into eax and then either multiply or divide.

			invoke IsDlgButtonChecked,hWin,IDC_RBNEUROSEK			;get checked state of Euro->SEK radio button
.if eax==BST_CHECKED
mov edx,0
push edx
call Convert
.endif
invoke IsDlgButtonChecked,hWin,IDC_RBNSEKEURO ;get checked state of SEK->Euro radio button
.if eax==BST_CHECKED
mov edx,1
push edx
call Convert
.endif


Convert proc

invoke StrToFloat, addr buffer, addr inputvalue
fld inputvalue
pop eax
.if eax==0
fmul SEKvalue
.endif
.if eax==1
fdiv SEKvalue
.endif
fstp inputvalue
invoke FloatToStr2, inputvalue, addr buffer
ret

Convert endp


The Convert proc used to work without the pop/if statements in it and with just a single fdiv or fmul.

Thanks.
Posted on 2007-04-06 17:28:57 by Ixje
It would work if you didn't use the proc and endp macros which modify the stack. See, if you call something then it's return address gets pushed onto the stack. Here, you pop that return address into eax inside your convert proc. Just change your Convert proc as shown below and all should be fine.


Convert proc myflag:DWORD

invoke StrToFloat, addr buffer, addr inputvalue
fld inputvalue
.if myflag==0
fmul SEKvalue
.else
fdiv SEKvalue
.endif
fstp inputvalue
invoke FloatToStr2, inputvalue, addr buffer
ret

Convert endp


A good exercise would be to debug starting from push eax | call Convert and see how exactly everything works ;)
Posted on 2007-04-06 17:45:23 by JimmyClif
push edx       ;0000
call Convert   ;0030 0000

So now the return address 0030 is on the top of the stack
and:

pop eax  ;eax gets the return address 0030
ret         ;gets the push edx of 0000...and ----->BOOM :-)

Posted on 2007-04-06 18:29:16 by eek

push edx      ;0000
call Convert  ;0030 0000

So now the return address 0030 is on the top of the stack
and:

pop eax  ;eax gets the return address 0030
ret        ;gets the push edx of 0000...and ----->BOOM :-)




Anything that gets PUSHed outside of a CALL should to be POPed outside of the CALL to keep things clean. This is the standard for CDECL (LibC) and FASTCALL (64-bit replacement for Win32 STDCALL, which in itself is actually more like CDECL.) However, if you don't need the value that you PUSHed, simply ADD to ESP.


invoke IsDlgButtonChecked,hWin,IDC_RBNEUROSEK ;get checked state of Euro->SEK radio button
.if eax==BST_CHECKED
mov edx,0
push edx
call Convert
add esp,4    ;***** Compensate for 4-byte PUSH of EDX prior to CALL *****
.endif
invoke IsDlgButtonChecked,hWin,IDC_RBNSEKEURO ;get checked state of SEK->Euro radio button
.if eax==BST_CHECKED
mov edx,1
push edx
call Convert
add esp,4    ;***** Compensate for 4-byte PUSH of EDX prior to CALL *****
.endif


Also, to access values on the stack, use ESP/EBP...


Convert proc

invoke StrToFloat, addr buffer, addr inputvalue
fld inputvalue
mov eax,DWORD ;***** EIP takes up closest 4 bytes ATM, go beyond it *****
.if eax==0
fmul SEKvalue
.endif
.if eax==1
fdiv SEKvalue
.endif
fstp inputvalue
invoke FloatToStr2, inputvalue, addr buffer
ret

Convert endp


HtH.
Posted on 2007-04-06 18:44:06 by SpooK
Thanks all for the good and clear feedback. Guess next time I should use the debugger first :oops:
Posted on 2007-04-07 05:13:12 by Ixje

Thanks all for the good and clear feedback. Guess next time I should use the debugger first :oops:


Just keep in mind that with the stack, First In Last Out (FILO) ;)
Posted on 2007-04-07 09:42:29 by SpooK

Just keep in mind that with the stack, First In Last Out (FILO) ;)


I did keep the FILO in mind. But what I didn't think about was that the return address also gets pushed to the stack.
Posted on 2007-04-07 10:33:44 by Ixje