0

[Solved] My code should find the minimum value in an array. In a procedure shown below, the assembly code compares 2 values in an dword array and jumps to setMin part when the second value is below the first. For some reason my program does not jump to setMin even when I can clearly see in the registers during debugging that it should jump. In result - the min value that is printed out is completely off and I don't know where its from (but that's entirely a different issue).

Here is the code:

; Find the minimum value in the array
;  in  : arr of 10 dword values
;        minIs  = "Minimum value is "
;  out : eax 
;----------------------------------

    mov esi, OFFSET arr     ; arr adress
    mov eax, [esi]          ; current min is 1st value
    mov ecx, 10             ; loop counter
Lcompare:
    add esi, 4              ; next value
    mov ebx, [esi]
    cmp eax, ebx            ; compare 2 values
    jb  setMin              ; Jump if not Below to next
    jmp next
setMin:
    mov eax, [esi]          ; sets new minimum value
next:
    loop Lcompare

    mov edx, OFFSET minIs   
    call WriteString        ; "Minimum value is "
    call WriteInt           ; display min value

    call Crlf

The arr input: 194, 102, 167, 157, 107, 140, 158, 148, 173, 194

The output:

Minimum value is +1701602643

I would really appreciate if someone could point out why the program does not enter setMin: after jb setMin but instead goes straight to next:.

And as a bonus could someone explain why I am getting that output instead of one of the values from the array?

urmur
  • 41
  • 7
  • 1
    `jb` is an `unsigned` comparison in x86. You want `jl` which is a `signed` comparison. See [How do conditional jumps work, based on the values of the flags in x86 assembly?](https://stackoverflow.com/a/45262313/3422102) – David C. Rankin Apr 20 '21 at 03:44
  • Are you sure `arr` was declared with 32-bit dword elements? You didn't show that part. `1701602643` in hex is `0x656c6553`, which as ASCII codes is `"Sele"`, which seems suspicious. Your `jb` looks fine for unsigned min, although inefficient compared to `jae next` to directly *skip* the mov without also needing a `jmp` – Peter Cordes Apr 20 '21 at 03:57
  • Is your assignment the same as [how can i print the numbers first then print my output](https://stackoverflow.com/q/67171688)? That has an array of `word` elements. Running off the end of it might have given you that dword of ASCII bytes. Anyway, doesn't seem to be a [mcve]. – Peter Cordes Apr 20 '21 at 04:08
  • The array is declared as dword. @PeterCordes – urmur Apr 20 '21 at 04:31
  • Thank you for pointing out what the ASCII code is. In my program I print the following string in main after the procedure in question: "Select #: " I don't understand how eax for WriteIn could have been set to that. @PeterCordes – urmur Apr 20 '21 at 04:37
  • 1
    You have an off-by-one error. Your loop iterates 10 times, reading 10 elements which does not count the first one, so the last iteration you read garbage from past the end of the array. – Nate Eldredge Apr 20 '21 at 04:38
  • @NateEldredge: Oh right, well spotted. The `add` is before the load, and the `arr[0]` load is already peeled out of the loop. Yet another reason to just use a pointer compare as the loop condition, instead of [the `loop` instruction, which is slow anyway](https://stackoverflow.com/questions/35742570/why-is-the-loop-instruction-slow-couldnt-intel-have-implemented-it-efficiently). – Peter Cordes Apr 20 '21 at 04:40
  • @NateEldredge thank you for pointing it out. Setting ecx to 9 results in WriteInt printing a value from the array. – urmur Apr 20 '21 at 04:43
  • @PeterCordes I do not know what pointer compare is. I am just getting comfortable with loops at the moment – urmur Apr 20 '21 at 04:47
  • 1
    Oh, and either your `cmp` test or your jump is backwards. `eax` is meant to contain the minimum so far and `ebx` is the new value. If `eax` is below `ebx` then the new value is *not* the minimum; your program is actually finding the maximum. – Nate Eldredge Apr 20 '21 at 04:49
  • @NateEldredge switching to `cmp ebx, eax` resulted in the program correctly finding the minimum rather than maximum. Thank you for all your help. – urmur Apr 20 '21 at 04:52
  • @urmur: A pointer compare would make your loop look like `do{ p++; ...; } while( p < array+length );` in C. In asm, that means `cmp esi, OFFSET array+40` / `jb Lcompare`. You're already incrementing ESI every iteration, so you can use that as a loop counter instead of also using ECX. – Peter Cordes Apr 20 '21 at 04:54
  • @PeterCordes I apreciate you explaining that. We have not learned about this in class yet. – urmur Apr 20 '21 at 04:57
  • If you're going to answer your own question, please post your answer as an *answer*, not an edit to the question. (That's allowed and even encouraged by Stack Overflow.) – Peter Cordes Apr 20 '21 at 05:10
  • @PeterCordes this was my first ever question. Thanks for letting me know. – urmur Apr 20 '21 at 05:12

1 Answers1

2

The comments under the question pointed out my mistakes.

What was wrong:

  1. iterating in the loop 10 times rather than 9. The extra iteration grabbed a string from "the future"
  2. I was finding the maximum rather than the minimum. Switching to cmp ebx, eax solved that.

The FULL working corrected code:

INCLUDE Irvine32.inc

.data
arr     dword   10 dup(0)
prompt  byte    "Select #: "
values  byte    "The values are: ", 0
comma   byte    ", ", 0
minIs   byte    "Minimum value is ", 0

.code

 part3 PROC
; Find the minimum value in the array
;  in  : arr of 10 values
;        minIs  = "Minimum value is "
;  out : eax 
;----------------------------------
    ; use jump if below JB

    mov esi, OFFSET arr     ; arr adress
    mov eax, [esi]          ; current min is 1st value
    mov ecx, 9              ; loop counter
Lcompare:
    add esi, 4              ; next value
    mov ebx, [esi]
    cmp ebx, eax            ; compare 2 values
    jnl next                ; Jump if not less to next
    mov eax, [esi]          ; sets new minimum value
next:
    loop Lcompare

    mov edx, OFFSET minIs   
    call WriteString        ; "Minimum value is "
    call WriteInt           ; display min value

    call Crlf
    ret
part3 ENDP


main PROC
    mov edx, OFFSET prompt
    call WriteString

    Call part3
main ENDP
END main
urmur
  • 41
  • 7