assembly strcmp function acting weirdly

[Kirdow]

Ok. I have a strcmp function that I wrote in assembly that looklikes this:

Code: Select all

strcmp:
    .loop:
        mov al, [si]
        mov bl, [di]
        cmp al, bl
        jne .notequal

        cmp al, 0
        je .done

        inc si
        inc di
        jmp .loop

    .notequal:
        clc
        ret
    .done:
        stc
        ret

And then somewhere else I have these 4 declared:

Code: Select all

msg_one                db "test", 0
msg_two                db "test", 0

msg_cmp_success    db "success", 0
msg_cmp_end          db "end", 0

And at the final step, in the main loop, (which has an input further down so it doesn't run in an endless loop), I have this:

Code: Select all

mov si, msg_one
mov di, msg_two
call strcmp
jnc .skip

mov si, msg_cmp_success
call print_string

.skip:
mov si, msg_cmp_end
call print_string

So, what I'm expecting to happen is that when I do strcmp, the carry flag will be set. which will cause .skip not to be called right after. so what I want to happen is that is print msg_cmp_success then msg_cmp_end.
But for some reason, the carry flag doesn't get set, even though they are both equal, which makes it just show the msg_cmp_end

So what is the problem here? why doesn't the carry flag get sent, marking them as equal, and how do I fix it?

[BrightLight]

I've tested your exact copy of the code and it works fine. Perhaps you have a problem in your segment registers?
Where is your program loaded? What's in CS, DS and ES?

[glauxosdever]

Hi,

Perhaps you have a problem in your segment registers? Where is your program loaded? What's in CS, DS and ES?

This is exactly the reason I dislike real mode segmentation...

Switch to protected mode or long mode as fast as possible. Get prepared for writing many many drivers for different kinds of hardware, because BIOS interrupts will not be available. But I'm sure you can write your own drivers better than the firmware programmers did.

Another cause could be SI and DI overflowing. You never can be sure.

Code: Select all

    .notequal:
        clc
        ret
    .done:
        stc
        ret

CMP instruction sets equality flag. Setting carry flag is unnecessary.


Regards,
glauxosdever

[mikegonta]

And at the final step, in the main loop, (which has an input further down so it doesn't run in an endless loop), I have this:

Code: Select all

mov si, msg_one
mov di, msg_two
call strcmp
jnc .skip

mov si, msg_cmp_success
call print_string

.skip:
mov si, msg_cmp_end
call print_string

The "end" message is always displayed no matter what the carry is, the "success" message simply falls through.

[BrightLight]

This is exactly the reason I dislike real mode segmentation...

I dislike segmentation at all. My current 32-bit OS doesn't use any segmentation, and that's what I wanted.

[iansjack]

Instead of all the messing about why aren't you using the CMPS instruction, designed for the job?

[BASICFreak]

I'm unsure if this is correct as I have not assembled your code, but
mov al, [si]
mov bl, [di]

translates to:
MOV AL, BYTE [DS:SI]
MOV BL, BYTE [ES:DI]
(maybe backwards on DS/ES)

I'd recommend hard coding which data segment to use, or make sure DS == ES.

NOTE: This is off memory from doing my (first) OSLoader, where I had 2 different data segments. (DS = 0000 & ES = 0BE0)

ALSO: Have you stepped the code to make sure it does what you think it should?

@omarrx024: Yes, segmentation is terrible, but in real mode it is nice to be able to access more than 64KB. And I truly hope OP is writing a boot/OS Loader and not a 16-bit kernel.

@OP: off what iansjack said, cmps:
cmpsb - compares BYTES ES:DI to DS:SI
can be used with rep
More info on cmps: http://www.fermimn.gov.it/linux/quarta/x86/cmps.htm

[Unsigned]

I'm unsure if this is correct as I have not assembled your code, but
mov al, [si]
mov bl, [di]

translates to:
MOV AL, BYTE [DS:SI]
MOV BL, BYTE [ES:DI]
(maybe backwards on DS/ES)

No. When you use (E)SP or (E)BP for a memory access, SS is used as the default segment register. In any other case, DS is used.

[Kirdow]

Instead of all the messing about why aren't you using the CMPS instruction, designed for the job?

Hmm, I may have forgot to tell that I'm writing 16-bit. So I don't think the cmps is available (atleast by the research I did).

The "end" message is always displayed no matter what the carry is, the "success" message simply falls through.

The "end" message is always displayed, yes, and the success, only when the 2 strings are equal (msg_one and msg_two). but the problem I get is, even though the two strings are equal, it doesn't get displayed, meaning they are not equal (which is wrong, they are equal).

I've tested your exact copy of the code and it works fine.

Yeah, I had a previous version (before I did a recode) which used this function too, and that worked fine.

Perhaps you have a problem in your segment registers?
Where is your program loaded? What's in CS, DS and ES?

As I said earlier in this reply, I have 16-bit, and I load from a floppy (which I didn't say). As the bootloader gets loaded at 0x7c00, I checked a pdf file and saw I have some space after that, from 0x7e00, which I can fit my 4096 byte bootloader. and yes, I know that it is all loaded because the previous version, with another design, had the same load position and same size. also, the CS, DS and ES is put to the same as the previous too.

[Octocontrabass]

Hmm, I may have forgot to tell that I'm writing 16-bit. So I don't think the cmps is available (atleast by the research I did).

All 32-bit instructions are available in 16-bit mode. That doesn't matter because cmpsb and cmpsw are 8086 instructions, so they're available on all x86 CPUs.

Yeah, I had a previous version (before I did a recode) which used this function too, and that worked fine.

I think your strcmp function is working correctly, but it's not doing what you want because you have made a mistake somewhere else.

[Kirdow]

I think your strcmp function is working correctly, but it's not doing what you want because you have made a mistake somewhere else.

Here is the first, working code, that calls the strcmp (just copy pasting):

Code: Select all

		mov si, buffer
		cmp byte [si], 0
		je .k_main_loop
		
		mov si, buffer
		mov di, cmd_help
		call strcmp
		jc .help

and this is the non-working code:

Code: Select all

		mov si, buffer
		cmp byte [si], 0
		je .done
		
		mov si, buffer
		mov di, os_cmd_help
		call strcmp
		jc .os_crun_help

FYI, I tried making a "strcmp_v" function where the v means Verbose, like logging everything to the user. Here's that code:

Code: Select all

strcmp_v:
	pusha
		mov si, ._v_clear_str
		call screen_print_string
	popa
	.loop:
		mov al, [si]
		mov bl, [di]
		cmp al, bl
		jne .notequal
		call ._v_cmp_s
		call ._v_cmp_d
		cmp al, 0
		je .done
		
		pusha
			mov si, ._v_clear_str
			call screen_print_string
		popa
		
		inc di
		inc si
		jmp .loop
		
	.notequal:
		pusha
			mov si, ._v_ne
			call screen_print_string
		popa
		call ._v_ncmp_s
		call ._v_ncmp_d
		pusha
			mov si, ._v_clear_str
			call screen_print_string
		popa
		pusha
			mov [out_byte], al
			call screen_write_byte
			mov si, os_nl
			call screen_write_string
			mov [out_byte], bl
			call screen_write_byte
			mov si, os_nl
			call screen_write_string
		popa
		pusha
			mov si, ._v_clear_str
			call screen_print_string
		popa
		clc
		ret
	.done:
		stc
		ret
		
	._v_cmp_s:
		mov [._v_char], al
		pusha
			mov ah, 0x0E
			mov al, byte [._v_char]
			mov bh, 0x00
			mov bl, 0x0F
			int 0x10
			mov si, os_nl
			call screen_write_string
		popa
		ret
	._v_cmp_d:
		mov [._v_char], bl
		pusha
			mov ah, 0x0E
			mov al, byte [._v_char]
			mov bh, 0x00
			mov bl, 0x0F
			int 0x10
			mov si, os_nl
			call screen_write_string
		popa
		ret
		
	._v_ncmp_s:
		mov [._v_char], al
		pusha
			mov ah, 0x0E
			mov al, byte [._v_char]
			mov bh, 0x00
			mov bl, 0x0F
			int 0x10
			mov si, os_nl
			call screen_write_string
		popa
		ret
	._v_ncmp_d:
		mov [._v_char], bl
		pusha
			mov ah, 0x0E
			mov al, byte [._v_char]
			mov bh, 0x00
			mov bl, 0x0F
			int 0x10
			mov si, os_nl
			call screen_write_string
		popa
		ret
		
._v_char		db 0, 0
._v_clear_str	db "------------", 0
._v_ne			db "--NotEqual--", 0

The thing here, is that everytime you see pusha and popa, between those are something that displays a log statement. so I backup the registers with pusha, change them to display a string/character and then a new line, then I do popa and continue with the normal strcmp. Note that this is a normal strcmp with this pusha-log-popa statements pasted every now and then.
Some other notes is that screen_write_string is writing a string to the screen and screen_print_string is the same but adds \r\n to the end.

The good thing with the strcmp_v, is that it actually says they are equal, even though the original one (which is the same, just without the push-log-pop stuff) says they are not.
So any idea what it could be?

[Octocontrabass]

I still see no differences between the "working" and "not working" code. The problem is still somewhere else.

What values are in the segment registers?

[Kirdow]

What values are in the segment registers?

Well. I use a specific system I made with "delegates", I know it's not called that but it kind of works the same way as delegates in C#. Maybe that is the problem. So to make this easier, I post most of my code. I kept some screen_ methods out tho, they're not the cause anyway. This might get long too. (Note: I use nasm, so some keywords like include and times isn't from assembly, it's from nasm (I think))
in the bootloader I load the kernel to 0x7e00:

Code: Select all

mov dl, 0x0		; drive 0 = floppy 1
mov dh, 0x0		; head (0=base)
mov ch, 0x0		; track/cylindern
mov cl, 0x02	; sector (1=bootloader)
mov bx, 0x7e00	; place in RAM for kernel
mov es, bx		; place BX in pointer ES
mov bx, 0x0		; back to zero

.loop:
mov ah, 0x02
mov al, 8		; sectors to load ; kernel takes up 8 sectors
int 0x13
jc .loop

Then I do this (right after):

Code: Select all

push es
push bx

jmp 0x7e00:0x0

retf

and then in the start of kernel I do:

Code: Select all

push cs
pop ds

jmp os_main

os_main:
	call screen_clear ; function I made to clear screen.
	call os_setup ; this sets some needed things up
	call os_loop_start ; this starts a loop
	mov si, .msg_kirdos_end ; OS end msg, note this is currently a DOS looking OS.
	call screen_print_string ; this prints a message and then makes a new line

	call os_shutdown
	ret

.msg_kirdos_end		db "KirdOS has stopped...", 13, 10, 0

os_setup:
	pusha
		mov al, 1 ; this is the input for the next line
		call os_timecycle_set ; a function to set a freeze period. kind of useless atm but trying to get it working
		mov ax, os_tick ; pointer to a function which runs every loop. kindof like C# Delegate/event
		call os_loop_settick ; sets the event var to ax (from input)
	popa

	ret

os_tick:
	pusha
		mov si, os_str_prompt
		call screen_write_string
		
		mov di, buffer
		call cli_read_line
		
		mov si, buffer
		cmp byte [si], 0
		je .done
		
		mov si, buffer
		mov di, os_cmd_help
		call strcmp
		jc .os_crun_help
		
		[...]

	popa
	ret

; variables
os_str_prompt	db "> ", 0x0
os_str_kirdos	db "KirdOS", 0x0
os_nl			db 13, 10, 0	; new line variable

os_cmd_help	db "help", 0

%include "ksi.asm"

buffer times 64 db 0 ; a buffer for input
times 0x1000-($-$$) db 0 ; I want the kernel to be exactly 4096 bytes, and I know everything gets loaded

And in ksi.asm:

Code: Select all

os_loop_running db 0x0 ; this is like a boolean

; this function starts the loop
os_loop_start:
	pusha
	mov al, [os_loop_running]
	cmp al, 0
	je .start
	popa
	ret
	.start:
	mov al, 1
	mov [os_loop_running], al
	call os_loop_run
	popa
	ret

;this function stops the loop
os_loop_stop:
	pusha
	mov al, [os_loop_running]
	cmp al, 0
	je .done
	mov al, 0
	mov [os_loop_running], al
	mov al, 0
	mov [os_loop_tick], al
	popa
	ret
.done:
	popa
	ret

;this function is the main loop, it calls the os_tick inputted with ax in os_setup.
os_loop_run:
	pusha
.loop:
	call [os_loop_tick] ; here, the os_tick gets called, and yes, it works, maybe the problem is here?
	call os_timecycle_wait ; it sleeps for 1000 microseconds (I kind of wanted to have this for some reason)
	mov al, [os_loop_running] ; gets the running boolean to al
	cmp al, 1 ;checks if it's running
	je .loop ; if it is running, keep looping
	popa ; else, stop
	ret

;this function sets the tick function to a variable, like events/delegates in C#
os_loop_settick:
	pusha
	mov [os_loop_tick], ax
	popa
	ret

os_loop_tick		db 0x0

So, now, anyone has a clue?

[BrightLight]

You loaded your kernel to 0x7E00:0x0000, not 0x7E00. Segmented address 0x7E00:0x0000 corresponds to the linear address 0x7E000. Your segment registers should have 0x7E00 and your whole file should be ORG 0.

[Kirdow]

You loaded your kernel to 0x7E00:0x0000, not 0x7E00. Segmented address 0x7E00:0x0000 corresponds to the linear address 0x7E000. Your segment registers should have 0x7E00 and your whole file should be ORG 0.

Hmm. So what do I have to change? What lines do I need to change and to what?