Program breaks and I don't understand why

Question

This is the latest piece of code I have written. I'm trying to figure out several things.

Issue: I think I almost have it, but it is saying that every PIN is valid, which it isn't. This means that the ValidatePIN proc is wrong, but I can't seem to figure it out. Any suggestions?

Thanks much for your help!!!!!

; 32-bit assembly language template

; INCLUDE Irvine32.inc

INCLUDELIB C:\Irvine\Kernel32.lib
INCLUDELIB C:\Irvine\Irvine32.lib
INCLUDE C:\Irvine\Irvine32.inc

pbyte typedef ptr byte                          ; pointer to bytes

.data
VALID_PIN = 0
PIN_SIZE = 5
minVals byte 5,2,4,1,3                  ; globally visible
maxVals byte 9,5,8,4,6                  ; globally visible
samplePin_1 byte 6,3,4,4,3              ; valid PIN
samplePin_2 byte 5,2,3,2,4              ; digit 3 is invalid
samplePin_3 byte 5,2,4,5,3              ; digit 4 is invalid
samplePin_4 byte 1,3,4,4,3              ; digit 1 is invalid

ptr1 pbyte samplePin_1                          ; points to array samplePin_1
ptr2 pbyte samplePin_2                          ; points to array samplePin_2
ptr3 pbyte samplePin_3                          ; points to array samplePin_3
ptr4 pbyte samplePin_4                          ; points to array samplePin_4
ptr5 pbyte minVals                              ; points to array minVals
ptr6 pbyte maxVals                              ; points to array maxVals

ValidPINMsg byte "The PIN is valid     ", 0    ;
InvalidPINMsg byte "The PIN is invalid.  The invalid digit is     ", 0 ;

.code
main proc

mov eax,VALID_PIN                  ; 

mov edi,ptr5
mov ebp,ptr6

mov esi,ptr1
call ValidatePIN                   ; determine whether or not the PIN is valid
.IF eax == ecx
 mov edx,OFFSET ValidPINMsg
 call WriteString
 call WaitMsg
 call Crlf
.ELSE
 mov edx,OFFSET InvalidPINMsg
 call WriteString
 call WriteDec
 call WaitMsg
 call Crlf
.ENDIF

mov esi,ptr2
call ValidatePIN                   ; determine whether or not the PIN is valid                       
.IF eax == ecx
 mov edx,OFFSET ValidPINMsg
 call WriteString
 call WaitMsg
 call Crlf
.ELSE
 mov edx,OFFSET InvalidPINMsg
 call WriteString
 call WriteDec
 call WaitMsg
 call Crlf
.ENDIF

mov esi,ptr3
call ValidatePIN                   ; determine whether or not the PIN is valid                       
.IF eax == ecx
 mov edx,OFFSET ValidPINMsg
 call WaitMsg
 call Crlf
.ELSE
 mov edx,OFFSET InvalidPINMsg
 call WriteString
 call WriteDec
 call WaitMsg
 call Crlf
.ENDIF

mov esi,ptr4
call ValidatePIN                   ; determine whether or not the pin is valid                       
.IF eax == ecx
 mov edx,OFFSET ValidPINMsg
 call WriteString
 call WaitMsg
 call Crlf
.ELSE
 mov edx,OFFSET InvalidPINMsg
 call WriteString
 call WriteDec
 call WaitMsg
 call Crlf
.ENDIF

main endp

ValidatePIN PROC
 mov ecx,PIN_SIZE

 .REPEAT

.IF (esi < edi) || (esi > ebp)
    call L1
.ELSE
 add esi,1
 add edi,1
 add ebp,1
.ENDIF

dec ecx    

.UNTIL ecx == 0

L1: 
 mov eax,ecx
 ret

ret
ValidatePIN ENDP
END main

Answer 1

Issue #1: You forgot exit at the end of main and ret at the end of ValidatePIN

Issue #2: Irvine32.lib has a CRLF-function. Insert call CrLf, where you wish to have a new line.

Issue #3: Typically a function returns a result in EAX. This value in EAX can be evaluated by the caller. In assembly you are free to return return other values in other registers. With Irvine32.lib you can print the value of EAX decimal

There are other useful functions. Take a look at the whole help for Irvine32.inc (with frameset)

Addendum:

There are further issues:

call ValidatePIN                   ; determine whether or not the PIN is valid
.IF eax == ecx
 mov edx,OFFSET ValidPINMsg
...
.ELSE
...

When should EAX become unequal to ECX? The last instruction of ValidatePIN is mov eax,ecx, so EAX and ECX are always equal at this point. You meant .IF eax == 0.

mov esi,ptr2
call ValidatePIN                   ; determine whether or not the PIN is valid
...

EDI and EBP were modified by the last procedures (call ...). You have to initialiaze them again.

.IF (esi < edi) || (esi > ebp)
...

ESI, EDI and EBP are here pointers (i.e. memory addresses) not values. To get the values you have to decorate the pointers with square brackets. But you can't directly compare memory with memory in x86-assembly. The next problem is to guarantee, that only a byte and not a dword will be compared. TL;DR:

mov al, byte ptr [esi]
.IF (al < [edi]) || (al > [ebp])
...

AL is the lowest byte of EAX. MASM recognizes now, that you want only process bytes.

call L1

A call will return. I guess you want to abort the loop at this point. Thus:

jmp L1

You won't like the number of the invalid digit. ECX (which determines the returning EAX) counts down in the loop: 5-4-3-2-1-0. To get a "count-up" you have to calculate (6 - ECX), but leave ECX=0 unchanged. You can transform that term to (-ECX + 6). So, change

.IF (al < [edi]) || (al > [ebp])
    jmp L1
.ELSE
...

to

.IF (al < [edi]) || (al > [ebp])
    neg ecx
    add ecx, 6
    jmp L1
.ELSE
...