Unhandled Exception: Recursive Factorial in assembly (MASM)

Question

I've written a program in assembly language (MASM) to allow students to practice calculating combinations. The program calculates the factorials recursively. I use a procedure called combinations which receives n and r, then gets (n=r)!, n!, and r! by calling a procedure called factorial. Factorial is recursive.

.data
n       DWORD   ?
r       DWORD   ?
result  DWORD   ?
answer            DWORD ?
divisor DWORD   ?

.code
MAIN    PROC

(some preliminary procedure calls)

push OFFSET divisor     ;ebp+20
push n              ;ebp+16
push r              ;ebp+12
push OFFSET result      ;ebp+8
call    combinations

;*************************************************
; combinations calls factorial (3 times) to calculate n!, r!, and (n-r)!.
; combinations calculates n!/(r!(n-r)!) , and stores the value in result.
; receives:  accepts n and r by value and result by address.
; returns: none
; preconditions: none
; registers changed:  eax, ebx, edx
;*************************************************
combinations    PROC
push        ebp
mov     ebp,esp

mov     eax, [ebp+16]   ;find (n-r)!
sub     eax, [ebp+12]
mov     ebx, eax
push        ebx
call        factorial
pop     ebx
mov     edx,[ebp+20]    ;move (n-r)! into result
mov     [edx],eax

mov     ebx, [ebp+12]        ;find r!
push        ebx
call        factorial
pop     ebx

mov     edx,[ebp+20]
mov     ebx, [edx]
mul     ebx         ;r!*(n-r)!, store product in eax
mov     ebx, [ebp+20]
mov     [ebx], eax          ;store product in divisor variable

mov     ebx, [ebp+16]   ;find n!
push        ebx
call        factorial
pop     ebx
mov     edx,[ebp+20]            
mov     ebx,[edx]           ;move value of divisor into ebx

mov     edx, 0
div     ebx         ;divide n! by divisor (r!*(n-r)!)
mov     ebx, [ebp+8]
mov     [ebx],eax                 ;move quotient into result

pop     ebp
ret     16
combinations    ENDP

;*************************************************
; calculates factorial recursively
; receives: 
; returns: factorial solution in eax
; preconditions: none
; registers changed: eax
;*************************************************
factorial   PROC
mov eax,dword ptr [esp+4]
cmp eax,1
jle endRecursive
dec eax
push eax
call factorial
mov esi,dword ptr [esp+4]
mul esi
endRecursive:
    ret 4
factorial   ENDP

All goes as expected and I'm getting the values needed. However, when all the calculations are done and the program reaches the statement "ret 16" at the end of the combinations procedure, I get the following exception:

Unhandled exception at 0x76f915de in Project.exe: 0xC0000005: Access violation.

I've run it through debugger and tried changing the return statement in the event that I miscalculated, but nothing I've tried thus far has worked. Maybe it's a case of having stared at this too long, but any ideas would be appreciated. Thanks in advance.

Update: Just tracking ebp and esp in debugger: it looks like when the program comes out of the last factorial call, esp is +12 of ebp, so it's adding +4 with each call of factorial. As a result, when the program hits pop ebp, ebp is pointing at r instead of where it should. Any suggestions for how to fix this?

Answer 1

You need to remove the three pop ebx instructions that appear after the three top-level calls tofactorial. Since factorial already pops its argument off the stack (through the use of ret 4), trying to remove the argument again by doing pop ebx messes up the stack pointer.