Nested for loop comprehension + push/pop

Question

I'm trying to create a procedure that generates a random string of length L, containing all capital letters. This procedure receives the length of the string (L) in EAX, and the pointer to a byte array in ESI where the random string will be saved. Returns the pointer to a byte array in ESI held the random string.

;.386
;.model flat,stdcall
;.stack 4096
;ExitProcess PROTO, dwExitCode:DWORD
INCLUDE Irvine32.inc

.data
;bArray BYTE 100 DUP(0), 0
bArray BYTE ?
count DWORD ?
ranNum DWORD ?

.code
main proc
    call Clrscr
    mov esi, OFFSET bArray
    call Randomize
    mov ecx, 20
    
L1:
    ;call Random32
    mov count, ecx
    mov eax, 100
    call RandomRange
    inc eax
    mov ranNum, eax
    call CreateRandomString
    loop L1

    invoke ExitProcess,0
main endp

CreateRandomString PROC
; Receives: Length of the string (L) in EAX, the pointer
; to a byte array in ESI where the random string will be saved
; Returns: Pointer to a byte array in ESI held the random string
;-----------------------------------------------------
mov ecx, ranNum
L2:
    mov eax, 26
    call RandomRange
    add eax, 65
    mov[esi], eax
    call WriteChar
    loop L2
    mov ecx, count
call Crlf
ret
CreateRandomString ENDP

end main

This is my code so far and it generates the random length strings, but the loop is infinite when I only want it to iterate 20 times. I know that the 20 I put in ecx gets lost but I'm not sure where in the code that happens and what I should do to change it. I'm thinking something to do with push and pop but I'm still shaky and confused on that concept. Thank you in advance!

You are changing it yourself. `mov ecx, count`. Step through the code in a debugger to see what is happening. — Raymond Chen, Mar 22 '23 at 00:20
I have been and I know that it gets changed in the procedure when I write "mov ecx, ranNum", but I'm still confused on the nested for loop — clq, Mar 22 '23 at 00:25
I thought that I'm setting the outer loop to be 20, then the inner loop to be the random number in the procedure. I thought that I needed "mov ecx, count" to restore the outer loop count but I guess I'm not decrementing the number of iterations? — clq, Mar 22 '23 at 00:33
`mov ecx, count` loads `ecx` from `count`. The value of `count` was set at `mov count, ecx` and not changed thereafter, so the value is always 20. — Raymond Chen, Mar 22 '23 at 00:40
When stepping through the debugger, count gets changed from 20 and count and ecx change with the same values which is why I'm lost on what to fix — clq, Mar 22 '23 at 00:46
Don't use the same register for both loop counters. The `loop` instruction is slow anyway, just use `dec ebx` / `jnz` or something for one of the loops. — Peter Cordes, Mar 22 '23 at 05:15

score 1 · Answer 1 · answered Mar 26 '23 at 13:10

Is it intentional that you are storing all of the string's characters on top of each other?
With bArray BYTE ? and mov esi, OFFSET bArray, later followed by not incrementing ESI in the CreateRandomString procedure, this is exactly what will happen.

Why you get an infinite loop

bArray BYTE ?
count DWORD ?

With bArray defined as a mere byte, you should not be writing a full dword to it, as in mov [esi], eax but rather write mov [esi], al. You have effectively destroyed the value 20 that you stored in the count variable (that you introduced as a convoluted way to preserve ECX). As a result, the loop instruction in the main L1 loop will always be acting upon ECX=0, looping back indefinitely!

I'm thinking something to do with push and pop

If you want to use ECX a second time in your CreateRandomString procedure, then simply preserve it on the stack:

CreateRandomString PROC
    push ecx             ; Preserve ECX
    mov  ecx, ranNum
L2: mov  eax, 26
    call RandomRange     ; -> EAX=[0,25]
    add  eax, 65
    mov  [esi], al       ; Currently storing on top of each other due to
    call WriteChar       ;       lack of `inc esi`, for testing no doubt
    loop L2
    call Crlf
    pop  ecx             ; Restore ECX
    ret
CreateRandomString ENDP

As others have suggested, preserving and restoring ECX in the CreateRandomString procedure would not be necessary if you would use another counting register in the main L1 loop. And as a bonus, you will have avoided using the slow loop instruction:

    mov  ebx, 20
L1: mov  eax, 100
    call RandomRange     ; -> EAX=[0,99]
    inc  eax
    mov  ranNum, eax
    call CreateRandomString
    dec  ebx
    jnz  L1

Nested for loop comprehension + push/pop

1 Answers1

Why you get an infinite loop