Assembly GCD Coding Infinite Loop

Question

I am trying to make an assembly program for finding GCD, takes in two integers, and prints out the GCD. The code assembles fine, however the program gets stuck in an infinite loop after both integers have been submitted:

;Title - GCD

INCLUDE Irvine32.inc


.data

strA BYTE "Enter an integer A: ",0
strB BYTE "Enter an integer B: ",0
temp DWORD ?
finalStr BYTE "GCD of the two integers is: ",0



.code

main PROC

call Clrscr

mainLoop:
mov edx,OFFSET strA
call WriteString
call ReadInt
mov temp, eax
call Crlf

mov edx, OFFSET strB
call WriteString
call ReadInt        
mov ebx, eax
mov eax, temp
call Crlf

call GCD

mov edx, OFFSET finalStr
call WriteString
call WriteInt

call WaitMsg

jmp mainLoop

main ENDP

;-----------------------------------------------
abs PROC
; Computes the absolute value of a number.
; Receives: eax = the number
; Returns:  eax = absolute value of the number
;-----------------------------------------------
   cmp eax, 0                    ; see if we have a negative number
   jge done
   neg eax

done:
   ret
abs ENDP

;-----------------------------------------------
gcd PROC
; Finds Greatest Common Divisor of two integers
; Recieves: eax= int A , ebx = int B
; Returns eax = GCD
;-----------------------------------------------
call abs        ;takes absolute value of both registers
mov temp, eax
mov eax, ebx
call abs
mov eax, temp

cmp eax, ebx    ; making sure we divide the bigger number by the smaller
jz DONE     ; if numbers are equal, GCD is eax either way
jc SWITCH   ;swaps if ebx is larger then eax

mov edx, 0

SWITCH:         ;swaps values so eax is larger then ebx
mov temp, eax
mov eax, ebx
mov ebx, temp
mov edx, 0
jmp L1

L1:     ;divides until remainder is 0, then eax is GCD
div ebx
cmp edx, 0
jz DONE
mov eax, edx
jmp L1


DONE:
gcd ENDP

END main

How can I get out of this loop?

Answer 1

I see one problem straight up:

call abs        ;takes absolute value of both registers
mov  temp, eax
mov  eax, ebx
call abs
mov  eax, temp

There's nothing there moving the abs-ed ebx value back into ebx, it should be:

call abs        ;takes absolute value of both registers
mov  temp, eax
mov  eax, ebx
call abs
mov  ebx, eax
mov  eax, temp

---

Another issue, though not directly related to your problem, is that the x86 architecture has long had an xchg instruction that would greatly clean up your code, specifically the use of temp.

---

And think about this sequence:

cmp eax, ebx    ; making sure we divide the bigger number by the smaller
jz DONE     ; if numbers are equal, GCD is eax either way
jc SWITCH   ;swaps if ebx is larger then eax

mov edx, 0

SWITCH:         ;swaps values so eax is larger then ebx

You'll find that the code at SWITCH: is going to be run regardless of which value is larger.

You can fix that by changing:

jc   SWITCH   ; swaps if ebx is larger then eax

mov  edx, 0

into:

jc   SWITCH   ; swaps if ebx is larger then eax

mov  edx, 0
jmp  L1

---

Beyond that, I'm going to suggest actually running that code in your head to get an understanding of how it works but, more importantly, how to think like a machine in such a way that you become a better developer.

Start with the following table:

[temp]   eax      ebx      edx      <stack>
-------- -------- -------- -------- --------
?        ?        ?        ?        ?,

Then execute each line in turn, filling in the columns as data changes.

You'll eventually figure out where the problem is coming from and you'll also understand why sometimes the older guys that only ever had this means to debug are much better at it :-)

I'll give you a clue. Keep a particular eye on edx and see how that's changing, and how it's likely to affect certain other instructions, like div, that may depend on it being zero.